patterncppMinor
Determine playing card
Viewed 0 times
playingcarddetermine
Problem
I would like a code review on the coding I did for my assignment. The reason I have asked here is because my lecturer has no intention of going through (Don't ask me why, I asked before and he said no..) and I have no idea if I am doing it right or not.. All my other classmates seems to have different answers too, some have deck class, shuffle function etc..
Anyway the question is asking the user to design and implement a class that represent a playing card in which it can be used to play card games. Consider what information you needs to be stored in a card and what you man want to do with a card (accessors/mutators).
In this assignment, only 2 files are required - 1 header and 1 implementation file.
Kindly see the following:
Header file
Implementation File
As I have only started out in C++, do pardon me if my coding format is wrong etc.
Anyway the question is asking the user to design and implement a class that represent a playing card in which it can be used to play card games. Consider what information you needs to be stored in a card and what you man want to do with a card (accessors/mutators).
In this assignment, only 2 files are required - 1 header and 1 implementation file.
Kindly see the following:
Header file
#ifndef CARD_H
#define CARD_H
#include
using namespace std;
class card
{
private:
string cardSuit;
string cardValue;
public:
// Constructor
string displayCard(string, string);
// Member Functions
string getSuit();
string getValue();
// Default
card();
};
#endifImplementation File
#include
#include
#include
#include
#include "card.h"
using namespace std;
card::card()
{
srand(time(0));
cardSuit = getSuit();
cardValue = getValue();
}
string card::displayCard(string suit, string value)
{
return cardSuit + " of " + cardValue;
}
string card::getSuit()
{
string arrSuits[] = {"HEARTS", "DIAMONDS", "CLUBS", "SPADES"};
// 1 out of 4 suits is choose randomly
int randSuits = rand() % 4;
return arrSuits[randSuits];
}
string card::getValue()
{
string arrValues[] = {"ACE", "ONE", "TWO", "THREE", "FOUR", "FIVE", "SIX", "SEVEN", "EIGHT", "NINE", "TEN", "JACK", "QUEEN", "KING"};
// 1 out of 13 values is choose randomly
int randValues = rand() % 13;
return arrValues[randValues];
}As I have only started out in C++, do pardon me if my coding format is wrong etc.
Solution
In object-oriented programming, a
The card's identity, therefore, must be determined in the constructor, and it should never change thereafter. All three methods should be declared
Furthermore, it's a bad idea to have the card's identity be randomly assigned in the constructor. How would you create a deck of 52 different cards, if you couldn't instantiate each one deterministically?
The
Your comments are wrong:
So, the header file should look more like this:
I'll leave it to you to figure out what the implementation should look like.
card object is supposed to represent a card. Your card, though… I'm not sure what it represents! If anything, it's the ultimate chameleon card: every time you call .getSuit() or .getValue() on it, it reports a different value!The card's identity, therefore, must be determined in the constructor, and it should never change thereafter. All three methods should be declared
const.Furthermore, it's a bad idea to have the card's identity be randomly assigned in the constructor. How would you create a deck of 52 different cards, if you couldn't instantiate each one deterministically?
The
displayCard() method makes no sense — why should you pass in suit and value as parameters, which will then be ignored?using namespace std; defeats the purpose of the std namespace. It's sloppy practice in general. In particular, you should never do that in a header file, because any .cpp file that includes that header will have its namespace contaminated as well.Your comments are wrong:
displayCard() is not a constructor.So, the header file should look more like this:
#ifndef CARD_H
#define CARD_H
#include
class Card
{
public:
// Constructor
Card(const std::string &suit, const std::string &value);
// Member Functions
std::string getSuit() const;
std::string getValue() const;
std::string display() const;
private:
std::string suit, value;
};
#endifI'll leave it to you to figure out what the implementation should look like.
Code Snippets
#ifndef CARD_H
#define CARD_H
#include <string>
class Card
{
public:
// Constructor
Card(const std::string &suit, const std::string &value);
// Member Functions
std::string getSuit() const;
std::string getValue() const;
std::string display() const;
private:
std::string suit, value;
};
#endifContext
StackExchange Code Review Q#124897, answer score: 3
Revisions (0)
No revisions yet.