HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Determine playing card

Submitted by: @import:stackexchange-codereview··
0
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

#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();

};
#endif


Implementation 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 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;
};
#endif


I'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;
};
#endif

Context

StackExchange Code Review Q#124897, answer score: 3

Revisions (0)

No revisions yet.