Suit class for card game

Question

I'm trying to make a card game, using OOP style. I decided to make suit a class rather than an enum as I would first have done. How am I doing so far? Am I over-complicating things by making it a class?

namespace Cards
{
class Suit
{
public:
    Suit();
    ~Suit();

    std::string GetName(bool plural, bool capital) const;
    bool IsRed() const;
    bool IsBlack() const;
    bool IsDifferentColour(const Suit& other) const;
    bool IsSameColour(const Suit& other) const;
    bool operator == (const Suit& other) const;
    bool operator != (const Suit& other) const;
    bool operator < (const Suit& other) const;

    static const Suit hearts, diamonds, clubs, spades;

private:
    enum Type
    {
        Type_Null,
        Type_Hearts,
        Type_Diamonds,
        Type_Clubs,
        Type_Spades
    } type;

    explicit Suit(Type type);
};
}

const Cards::Suit Cards::Suit::hearts(Type_Hearts);
const Cards::Suit Cards::Suit::diamonds(Type_Diamonds);
const Cards::Suit Cards::Suit::clubs(Type_Clubs);
const Cards::Suit Cards::Suit::spades(Type_Spades);

Cards::Suit::Suit()
    : type(Type_Null)
{}

Cards::Suit::Suit(Type type)
    : type(type)
{}

Cards::Suit::~Suit()
{}

std::string Cards::Suit::GetName(bool plural, bool capital) const
{
    std::string name;

    switch (type)
    {
    case Type_Hearts:
        {
             name = "Heart";
        }
        break;

    case Type_Diamonds:
        {
            name = "Diamond";
        }
        break;

    case Type_Clubs:
        {
            name = "Club";
        }
        break;

    case Type_Spades:
        {
            name = "Spade";
        }
        break;

    default:
        {
            assert(false);
        }
        break;
    }

    if (! capital)
    {
        name[0] = static_cast<std::string::value_type>(::tolower(name[0]));
    }

    if (plural)
    {
        name += "s";
    }

    return name;
}

bool Cards::Suit::IsRed() const
{
    bool red;

    switch (type)
    {
        case Type_Hearts:
        {
             red = true;
        }
        break;

    case Type_Diamonds:
        {
            red = true;
        }
        break;

    case Type_Clubs:
        {
            red = false;
        }
        break;

    case Type_Spades:
        {
            red = false;
        }
        break;

    default:
        {
            assert(false);
            // prevent compiler warning
            red = false;
        }
    }

    return red;
}

bool Cards::Suit::IsBlack() const
{
    return ! IsRed();
}

bool Cards::Suit::IsDifferentColour(const Suit& other) const
{
    return this->IsRed() != other.IsRed();
}

bool Cards::Suit::IsSameColour(const Suit& other) const
{
    return this->IsRed() == other.IsRed();
}

bool Cards::Suit::operator == (const Cards::Suit& other) const
{
    return this->type == other.type;
}

bool Cards::Suit::operator != (const Cards::Suit& other) const
{
    return ! (*this == other);
}

bool Cards::Suit::operator < (const Cards::Suit& other) const
{
    return this->type < other.type;
}

Answer 1

If the destructor does nothing remove it:

Cards::Suit::~Suit()
{}

We can shorten that switch statement to make it readable.

    switch (type)
    {
        case Type_Hearts:   name = "Heart";  break;
        case Type_Diamonds: name = "Diamond";break;
        case Type_Clubs:    name = "Club";   break;
        case Type_Spades:   name = "Spade";  break;
        default:            assert(false);   break;
    }

Or we can replace it with a look up:

    static std::string  names[] = {"NULL", "Heart", "Diamond", "Club", "Spade"};
    if (type == Type_Null)
    {    assert(false); // Or throw.
    }
    name =  names[type];

If you want more checking:

    static std::vector<std::string>  names = {"Heart", "Diamond", "Club", "Spade"};
    name =  names.at(type-1); // will throw on bad value.

This seems a bit over keen.

        name[0] = static_cast<std::string::value_type>(::tolower(name[0]));

std::string::value_type by definition is char.
tolower() may return an int unless you pass in an EOF you will get a character back.

I would have just done:

        name[0] = ::tolower(name[0]);

In this switch use fall throug:

switch (type)
{
    case Type_Hearts:    /* fall through */
    case Type_Diamonds:  return true;

    case Type_Clubs:     /* fall through */
    case Type_Spades:    return false;
    default:  assert(false);
        // prevent compiler warning
        return true;
}

Is A Null_Suit the same as another Null_Suit?
in double a NaN is not equal to another NaN you may want that kind of functionality.

bool Cards::Suit::operator == (const Cards::Suit& other) const
{
    return this->type == other.type;
}

Answer 2

Yes I think you are over complicating things. In C++11 I would use a strongly typed enum:

enum class Suite {Hearts,
                  Diamonds,
                  Clubs,
                  Spades};

I might also have considered a simple class hierarchy

namespace suite
{
  class SuiteBase{...};
  class Hearts : public SuiteBase {...};
  class Diamonds : public SuiteBase {...};
  class Clubs : public SuiteBase {...};
  class Spades : public SuiteBase {...};
}

Then create needed operations between suites. Preferably using some double dispatch technique to be able to compare SuiteBase pointers.