Card class for card games

Question

I'm attempting to improve my coding style by concentrating on simple and focused OOP client-server interfaces and error handling - atm in the context of designing a Blackjack game. I'm starting with the Card class that represents an individual card. I'm looking for any improvements on or flaws with my approach.

#include <sstream>
#include <stdexcept>
#include <string>

class Card
{
public:
    enum Ranks { Ace = 1, Ten = 10, Jack, Queen, King };
    enum Suits { Spades, Hearts, Diamonds, Clubs };

    Card( int rank, int suit ) throw(std::range_error)
    {
            if( rank < 1 || rank > 13 ) {
                    std::stringstream errMsg;
                    errMsg << "Card(): rank '" << rank << "' out of range";
                    throw std::range_error( errMsg.str() );
            } else if( suit < 0 || suit > 3 ) {
                    std::stringstream errMsg;
                    errMsg << "Card(): suit '" << suit << "' out of range";
                    throw std::range_error( errMsg.str() );
            }

            m_rank = rank;
            m_suit = suit;
    }

    int rank() const { return m_rank; }
    int suit() const { return m_suit; }
    std::string str( bool useStrings = false ) const
    {
            std::stringstream ret;
            if( useStrings )
                    ret << sRanks[m_rank] << " of " << sSuits[m_suit];
            else
                    ret << cRanks[m_rank] << cSuits[m_suit];
            return ret.str();
    }

private:
    static const char cRanks[14];
    static const char cSuits[4]
    static const std::string sRanks[14];
    static const std::string sSuits[4];

    int m_rank;
    int m_suit;
};

const std::string Card::sSuits[4] = {"Spades", "Hearts", "Diamonds", "Clubs"};
const std::string Card::sRanks[14] = { "\0", "Ace", "Two", "Three", "Four", "Five",
                    "Six", "Seven", "Eight", "Nine", "Ten", "Jack", "Queen", "King" };
const char Card::cSuits[4] = { 's', 'h', 'd', 'c' };
const char Card::cRanks[14] = { '\0', 'A', '2', '3', '4', '5', '6', '7',
                                '8', '9', 'T', 'J', 'Q', 'K' };

The client can create a card either using the Card enum constants or integers, like

Card card(Card::Ace, Card::Diamonds);
Card card2(0, 3); // throws an exception

I'm dubious on how useful the public enum constants are for client code. Also, I'm very new to exception programming and I'm not sure if this is a good approach or whether the constructor should throw an exception. Any improvements or suggestions?

Answer 1

If you are going to go to the trouble of creating Rank and Suit enum then use them.

Card( int rank, int suit ) throw(std::range_error)

I would have done

Card(Rank rank,Suit suit )

That way you can't accidentally go: Card(Card::Hearts,2) The compiler type checking will catch this at compiletime. Just use the enums all through your code. C++ is a strongly typed language use the compiler to catch your errors.

The exception specifications have been deprecated. They were mostly useless anyway (if you fail and throw an illegal exception it causes your application to terminate without unwinding the stack). So in general don't use them. For the one case where they are usfull (no throw specifications) they are good but I use them more as documentation to remind me that I should make sure the method does not throw.

But yes the constructor should throw an exception if it gets a value outside the expected range. There is no point in allowing an invalid object to propagate threw your code it will just cause problems with your invariants.

Prefer to use the initializer list rather than initializing variables in the constructor.

Card(Rank rank, Suit suit)
    : m_rank(rank)
    , m_suit(suit)

Again another place that it would have been nice to use the enum to make the code more eadable:

        if( rank < 1 || rank > 13 ) {

Try:

        if( rank < Ace || rank > King ) {

Hate getters/setter they spoil encapsulation. Do you really need to get the values of the card? Any operation that manipulates the card is usually a method on the card or a friendly class.

int rank() const { return m_rank; }
int suit() const { return m_suit; }

Don't try and write C++ like Java. We don;t need a string function. It is much more preferable (and flexable) to write an output (and probably input) stream operator.

std::string str() const
{
        return std::string( Ranks[m_rank] + Suits[m_suit] );
}

I would have done:

friend std::ostream& operator<<(std::ostream& stream, Card const& card)
{
    return stream << card.Ranks[card.m_rank] << card.Suits[card.m_suit];
}

Adding Cards to a deck. (Based on comment below)

#include <vector>

class Card
{
    public:
        enum Ranks { Ace = 1, Ten = 10, Jack, Queen, King };
        enum Suits { Spades, Hearts, Diamonds, Clubs };

        Card(Ranks rank, Suits suit){}
};

// Need to add the ++ operators for Ranks/Suits
Card::Ranks& operator++(Card::Ranks& r)
{
    return r = Card::Ranks(static_cast<int>(r)+1);
}
Card::Suits& operator++(Card::Suits& s)
{
    return s = Card::Suits(static_cast<int>(s)+1);
}



int main()
{
    std::vector<Card>   deck;

    // Does not look that verbose to me.
    for(Card::Ranks rank=Card::Ace;rank <= Card::King;++rank)
    {
        for(Card::Suits suit=Card::Spades;suit <= Card::Clubs;++suit)
        {
            deck.push_back(Card(rank, suit));
        }
    }

    // Compared to 
    for(int rank=Card::Ace;rank <= Card::King;++rank)
    {
        for(int suit=Card::Spades;suit <= Card::Clubs;++suit)
        {
            deck.push_back(Card(rank, suit));
        }
    }
}

Answer 2

I cleaned it up a bit taking most of Loki's advice, though I'm not sure about the public getter functions not being a good approach here. In the Blackjack program I'm making, and in most card games, your program logic will want to get the rank or suit of the card in question, not just print a string representation of the card. And the client probably shouldn't be able to change the value of the card, so there's no setter function. If a client has say a Hand class that represents a Blackjack hand, and needs to keep a cumulative point total of card ranks, I don't see another way to do this other than declaring Hand a friend inside Card and removing the getter functions, which seems like a poor approach.

One last question I have would be how I could create my own manipulator so that my overloaded stream operator could format its output in a variable way, like

std::cout << print_as_phrase << card << std::endl;

where print_as_string is a manipulator that causes my overloaded operator to return the commented out option in my code below. Right now I can only think of adding a function to card like void print_as_phrase( bool printPhrase = true ); to set a Card data member and have my overloaded friend operator function check that, but that seems more cumbersome than the original str() method I was using.

#include <iostream>
#include <string>
#include <stdexcept>
#include <sstream>
#include <vector>

class Card
{
public:
    enum Rank {
        Ace = 1, Two, Three, Four, Five, Six, Seven,
        Eight, Nine, Ten, Jack, Queen, King, RankEnd
    };
    enum Suit { Spades, Hearts, Diamonds, Clubs, SuitEnd };

    Card( Rank rank, Suit suit )
        : m_rank(rank), m_suit(suit)
    {
        if( rank < Ace || rank > King ) {
            std::stringstream errMsg;
            errMsg << "Card(): rank '" << rank << "' out of range";
            throw std::range_error( errMsg.str() );
        } else if( suit < Spades || suit > Clubs ) {
            std::stringstream errMsg;
            errMsg << "Card(): suit '" << suit << "' out of range";
            throw std::range_error( errMsg.str() );
        }
    }

    Rank rank() const { return m_rank; }
    Suit suit() const { return m_suit; }
    std::string str() const
    {
        std::stringstream ret;
        ret << sRanks[m_rank] << " of " << sSuits[m_suit];
        return ret.str();
    }

    friend std::ostream& operator<<( std::ostream &os, const Card &card )
    {
        return os << card.cRanks[card.m_rank] << card.cSuits[card.m_suit];
    //  return os << card.sRanks[card.m_rank] << " of " << card.sSuits[card.m_suit];
    }

private:
    static const std::string sRanks[14];
    static const std::string sSuits[4];
    static const char cRanks[14];
    static const char cSuits[4];
    Rank m_rank;
    Suit m_suit;
};

const std::string Card::sRanks[14] = {
    "\0", "Ace", "Two", "Three", "Four", "Five", "Six", "Seven",
    "Eight", "Nine", "Ten", "Jack", "Queen", "King"
};
const std::string Card::sSuits[4] = { "Spades", "Hearts", "Diamonds", "Clubs" };
const char Card::cRanks[14] = {
    '\0', 'A', '2', '3', '4', '5', '6', '7',
    '8', '9', 'T', 'J', 'Q', 'K'
};
const char Card::cSuits[4] = { 's', 'h', 'd', 'c' };


Card::Rank& operator++( Card::Rank &rank )
{
    return rank = Card::Rank( static_cast<int>(rank) + 1 );
}

Card::Suit& operator++( Card::Suit &suit )
{
    return suit = Card::Suit( static_cast<int>(suit) + 1 );
}


int main()
{
    std::vector<Card> deck;
    for( Card::Suit suit = Card::Spades; suit < Card::SuitEnd; ++suit )
        for( Card::Rank rank = Card::Ace; rank < Card::RankEnd; ++rank ) {
            deck.push_back( Card(rank, suit) );
            std::cout << deck.back() << "\n";
        }

    return 0;
}