Macau Card Game

Question

I've implemented a card game called Macau. This is a follow-up from this topic, where I tried to get the game's design right before actually implementing it.

Even if the game is presented on that topic, I will give a brief explanation as it follows. The game consists of a standard-deck that contains 52 cards. At the very beginning, every player receives 5 cards from the deck. (The first player that runs out of cards is the winner). Then, a card is put on the top of the pile. A player can put on the pile any card that is compatible with the top one. Two cards are compatible if they have the same suit or the same rank. There also exist several special cards:

• Rank: 2 or 3. This will require the next player to receive 2/3 cards from the deck if he doesn't have a 4 card with the same suit.
• Rank: A (Ace). This will simply skip the next player's turn.

Please note that, even if I mentioned 7 and the Joker as special cards in that topic, in the current state of the game they are not implemented. More precisely, the Jokers are disabled and the 7 is considered a normal card.

Here is the diagram of my classes.

And here is the code:

CardProperties.hpp

#pragma once
#include <array>
#include <string>

constexpr unsigned int numberOfSuits{ 6 };
constexpr unsigned int numberOfRanks{ 14 };

const std::array<std::string, numberOfSuits> suits{"Diamonds","Clubs"
                                                                , "Hearts","Spades" , "Red", "Black"};

const std::array<std::string, numberOfRanks> ranks{ "A", "2","3","4","5","6","7","8","9","10", "J","Q","K","Joker" };

enum class Suit
{
    Diamonds,
    Clubs,
    Hearts,
    Spades,
    Red,
    Black,
};

enum class Rank
{
    Ace = 0,
    Two,
    Three,
    Four,
    Five,
    Six,
    Seven,
    Eight,
    Nine,
    Ten,
    Jack,
    Queen,
    King,
    Joker,
};

Card.hpp

#pragma once
#include <ostream>
#include "CardProperties.hpp"

class Game;

class Card
{
public:
    Card(const Suit& suit, const Rank& rank);
    Card(const Rank& rank, const Suit& suit);
    bool isCompatibleWith(const Card& other) const;
    bool operator != (const Card& other) const;
    friend std::ostream& operator<<(std::ostream& os, const Card& card);

    //needed for std::set
    bool operator == (const Card& other) const;
    bool operator < (const Card& other) const;

    friend class Game;
private:
    bool invalidCardProperties(const Suit& suit, const Rank& rank) const;
    Suit _suit;
    Rank _rank;
};

Exceptions.hpp

#pragma once
#include <exception>
#include <string>

class InvalidCardException : public std::exception
{
public:
    InvalidCardException(const std::string& errorMessage);
    const char* what() const override;

private:
    std::string _errorMessage;
};

class EmptyDeckException : public std::exception
{
public:
    EmptyDeckException(const std::string& errorMessage);
    const char* what() const override;

private:
    std::string _errorMessage;
};

class NotEnoughCardsException : public std::exception
{
public:
    NotEnoughCardsException(const std::string& errorMessage);
    const char* what() const override;

private:
    std::string _errorMessage;
};

class InvalidNumberOfPlayersException : public std::exception
{
public:
    InvalidNumberOfPlayersException(const std::string& errorMessage);
    const char* what() const override;

private:
    std::string _errorMessage;
};

Card.cpp

#include "Card.hpp"
#include "Exceptions.hpp"

bool Card::invalidCardProperties(const Suit& suit, const Rank& rank) const
{
    if (rank == Rank::Joker && (suit != Suit::Black && suit != Suit::Red))
        return true;
    if (rank != Rank::Joker && (suit == Suit::Black || suit == Suit::Red))
        return true;
    return false;
}

Card::Card(const Suit& suit, const Rank& rank)
    : _suit(suit),
    _rank(rank)
{
    if (invalidCardProperties(suit, rank))
        throw InvalidCardException("The Joker can only be red or black.\n");
}

Card::Card(const Rank& rank, const Suit& suit)
    : Card(suit,rank)
{}

bool Card::isCompatibleWith(const Card& other) const
{
    return _suit == other._suit || _rank == other._rank;
}

bool Card::operator< (const Card& other) const
{
    return (static_cast<int>(_rank) < static_cast<int>(other._rank));
}

bool Card::operator == (const Card& other) const
{
    return _rank == other._rank && _suit == other._suit;
}

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

std::ostream& operator << (std::ostream& os, const Card& card)
{
    return os << '[' << suits[static_cast<int>(card._suit)] << " " << ranks[static_cast<int>(card._rank)] << ']';
}

Deck.hpp

#pragma once
#include <vector>
#include "Card.hpp"

class Deck
{
public:
    Deck();
    const Card& dealCard();
    std::vector<Card> dealCards(const unsigned int& numberOfCards);
    void refill(const std::vector<Card>& cards);
    void refill(const Card& card);
private:
    void fill();
    void shuffle();
    std::vector<Card> _cards;
};  

Deck.cpp

#include "Deck.hpp"
#include "Exceptions.hpp"
#include <random>

Deck::Deck()
{
    fill();
    shuffle();
}

const Card& Deck::dealCard()
{
    if (_cards.empty())
        throw EmptyDeckException("");

    Card& topCard(_cards.back());
    _cards.pop_back();
    return topCard;
}

std::vector<Card> Deck::dealCards(const unsigned int& numberOfCards)
{
    if (_cards.size() < numberOfCards)
        throw NotEnoughCardsException("");

    std::vector<Card> requestedCards;
    auto it{ std::prev(_cards.end(), numberOfCards) };

    std::move(it, _cards.end(), std::back_inserter(requestedCards));
    _cards.erase(it, _cards.end());

    return requestedCards;
}


void Deck::fill()
{
    constexpr unsigned int numberOfSuitsWithoutJokerSpecificOnes{ 4 };
    constexpr unsigned int numberOfRanksWithoutJokers{ 13 };

    for (unsigned int index1 = 0; index1 < numberOfRanksWithoutJokers; ++index1)
    {
        for (unsigned int index2 = 0; index2 < numberOfSuitsWithoutJokerSpecificOnes; ++index2)
        {
            Card card(static_cast<Suit>(index2), static_cast<Rank>(index1));
            _cards.push_back(card);
        }
    }

    /*Card redJoker(Suit::Red, Rank::Joker);
    _cards.push_back(redJoker);
    Card blackJoker(Suit::Black, Rank::Joker);
    _cards.push_back(blackJoker);*/
}

void Deck::shuffle()
{
    std::mt19937 randomNumberGenerator{ std::random_device{}() };
    std::shuffle(_cards.begin(), _cards.end(), randomNumberGenerator);
}

void Deck::refill(const std::vector<Card>& cards)
{
    _cards.insert(_cards.end(), cards.begin(), cards.end());
    shuffle();
}

void Deck::refill(const Card& card)
{
    _cards.push_back(card);
    shuffle();
}

Pile.hpp

#pragma once
#include "Card.hpp"
#include <vector>

class Pile
{
public:
    void addCard(const Card& card);
    const Card& eraseTopCard();
    const Card& getTopCard();
    friend std::ostream& operator << (std::ostream& os, const Pile& pile);
    std::vector<Card> releaseAllCardsButFirst();
private:
    std::vector<Card> _alreadyPlayedCards;
};

Pile.cpp

#include "Pile.hpp"
#include "Exceptions.hpp"
#include <algorithm>

constexpr unsigned int minimumValueOfPlayedCards{ 1 };
constexpr unsigned int lastElementOffset{ 1 };

void Pile::addCard(const Card& card)
{
    _alreadyPlayedCards.push_back(card);
}

std::vector<Card> Pile::releaseAllCardsButFirst()
{

    std::vector<Card> releasedCards;

    auto it{ std::next(_alreadyPlayedCards.begin(), _alreadyPlayedCards.size() - lastElementOffset) };

    std::move(_alreadyPlayedCards.begin(), it, std::back_inserter(releasedCards));
    _alreadyPlayedCards.erase(_alreadyPlayedCards.begin(), it);

    return releasedCards;
}

const Card& Pile::getTopCard()
{
    return _alreadyPlayedCards.back();
}

const Card& Pile::eraseTopCard()
{
    const Card& requestedCard{ _alreadyPlayedCards.back() };
    _alreadyPlayedCards.pop_back();
    return requestedCard;
}

std::ostream & operator<<(std::ostream & os, const Pile & pile)
{
    return os << pile._alreadyPlayedCards.back();
}

Hand.hpp

#pragma once
#include <vector>
#include "Card.hpp"

class Hand
{
public:
    void addCard(const Card& card);
    void addCards(const std::vector<Card>& cards);
    void removeCard(const Card& card);
    void removeCard(const unsigned int& cardNumber);
    std::vector<Card>::iterator findCard(const Card& card);
    const Card& getCard(const Card& card);
    const Card& getCard(const unsigned int& cardNumber);
    const unsigned int numberOfCards() const;
    friend std::ostream& operator << (std::ostream& os, const Hand& hand);

private:
    std::vector<Card> _cards;
};

Hand.cpp

#include "Hand.hpp"
#include <algorithm>

constexpr unsigned int indexOffset{ 1 };

void Hand::addCard(const Card& card)
{
    _cards.push_back(card);
}

void Hand::removeCard(const Card& card)
{
    auto it{ findCard(card) };
    _cards.erase(it);
}

void Hand::addCards(const std::vector<Card>& cards)
{
    _cards.insert(_cards.end(), cards.begin(), cards.end());
}

void Hand::removeCard(const unsigned int& cardNumber)
{
    _cards.erase(std::next(_cards.begin(), cardNumber - indexOffset));
}

std::vector<Card>::iterator Hand::findCard(const Card& card)
{
    return std::find(_cards.begin(), _cards.end(), card);
}

const Card& Hand::getCard(const Card& card)
{
    auto it{ findCard(card) };
    return *it;
}

const Card& Hand::getCard(const unsigned int& cardNumber)
{
    return _cards[cardNumber - indexOffset];
}

const unsigned int Hand::numberOfCards() const
{
    return _cards.size();
}

std::ostream& operator << (std::ostream& os, const Hand& hand)
{
    for (unsigned int index = 0; index < hand._cards.size(); ++index)
        os << "Card number: " << index + indexOffset <<" "<< hand._cards[index] << '\n';
    return os << '\n';
}

Player.hpp

#pragma once
#include "Hand.hpp"
#include <vector>

class Player
{
public:
    Player(const std::string& name = "undefined_name");
    const Card& putCard(const Card& card);
    Card putCard(const unsigned int& cardNumber);
    const Card& getCard(const unsigned int& cardNumber);
    void receive(const Card& card);
    void receive(const std::vector<Card>& cards);
    const unsigned int numberOfCards() const;
    friend std::ostream& operator << (std::ostream& os, const Player& player);
    const std::string name() const;

private:
    Hand _handOfCards;
    std::string _name;
};

Player.cpp

#include "Player.hpp"

Player::Player(const std::string& name)
    : _name(name)
{}

const Card& Player::putCard(const Card& card)
{
    const Card& requestedCard{ _handOfCards.getCard(card) };
    _handOfCards.removeCard(card);

    return requestedCard;
}

Card Player::putCard(const unsigned int& cardNumber)
{
    Card requestedCard{ _handOfCards.getCard(cardNumber) };
    _handOfCards.removeCard(cardNumber);

    return requestedCard;
}

const Card& Player::getCard(const unsigned int& cardNumber)
{
    return _handOfCards.getCard(cardNumber);
}

void Player::receive(const Card& card)
{
    _handOfCards.addCard(card);
}

void Player::receive(const std::vector<Card>& cards)
{
    _handOfCards.addCards(cards);
}

std::ostream& operator << (std::ostream& os, const Player& player)
{
    return os << player._name << "'s cards.\n" << player._handOfCards;
}

const unsigned int Player::numberOfCards() const
{
    return _handOfCards.numberOfCards();
}

const std::string Player::name() const
{
    return _name;
}

Game.hpp

#pragma once
#include "Deck.hpp"
#include "Pile.hpp"
#include "Exceptions.hpp"
#include "Player.hpp"
#include <set>

    class Game
    {
    public:
        Game(const unsigned int& numberOfPlayers = 2);
        void run();
    private:
        void dealInitialCardsToEachPlayer();
        void initializePlayerNames();

        void receiveCardsFromDeck(Player& player, const unsigned int& numberOfCards);
        void putCardToPile(Player& player, unsigned int& cardNumber);
        void normalCardPlayerTurn(Player& player);
        void specialCardPlayerTurn(Player& player);

        void printInformationAboutThePlayerAndTheTopCardFromPile(const Player& player) const;
        void printInformationAboutThePlayerAndTheTopCardFromPile(const Player& player, const Rank& rank) const;
        const Player& findWinner();

        bool isSpecialCard(const Card& card) const;
        void validatePlayerCardCompatibilityWithPileTopCard(Player& player, unsigned int& cardNumber);

        std::vector<Player> _players;
        unsigned int _numberOfPlayers;
        Deck _deck;
        Pile _pile;
        bool _isRunning;

        const std::set<Card> _specialCards{ { Suit::Clubs, Rank::Two },{ Suit::Diamonds, Rank::Two },{ Suit::Hearts, Rank::Two },{ Suit::Spades, Rank::Two }
            ,{ Suit::Clubs, Rank::Three },{ Suit::Diamonds, Rank::Three },{ Suit::Hearts, Rank::Three },{ Suit::Spades, Rank::Three },
            /*,{ Suit::Clubs, Rank::Seven },{ Suit::Diamonds, Rank::Seven },{ Suit::Hearts, Rank::Seven },{ Suit::Spades, Rank::Seven },*/
        { Suit::Clubs, Rank::Ace },{ Suit::Diamonds, Rank::Ace },{ Suit::Hearts, Rank::Ace },{ Suit::Spades, Rank::Ace } };
    };

Game.cpp

#include "Game.hpp"
#include "GameConstants.hpp"
#include <set>
#include <iostream>
#include <algorithm>

void Game::initializePlayerNames()
{
    std::string currentPlayerName;

    for (unsigned int index = 0; index < _numberOfPlayers; ++index)
    {
        std::cout << "Name: "; 
        std::getline(std::cin, currentPlayerName);

        Player player(currentPlayerName);
        _players.push_back(player);
    }
}

Game::Game(const unsigned int& numberOfPlayers)
    : _numberOfPlayers(numberOfPlayers),
    _isRunning(true)
{
    if (numberOfPlayers > maximumNumberOfPlayers || numberOfPlayers < minimumNumberOfPlayers)
        throw InvalidNumberOfPlayersException("The minimum number of players is 2, while the maximum number of players is 9.");

    initializePlayerNames();
}

void Game::printInformationAboutThePlayerAndTheTopCardFromPile(const Player& player) const
{
    std::cout << "\nTop card from pile: " << _pile << "\n";
    std::cout << player << "\n";
    std::cout << "What card would you like to put? If you don't have any compatible card or you wish to skip the turn, enter 0.\n";
}

void Game::printInformationAboutThePlayerAndTheTopCardFromPile(const Player& player, const Rank& rank) const
{
    switch (rank)
    {
    case Rank::Two:
    {
        std::cout << "\nTop card from pile: " << _pile << "\nThis is 2 card and you will need to receive 2 cards from the deck.\n"
            << "If you have a 4 card that has the same suit, then you can stop it.\n";
        std::cout << "What card would you like to put? If don't have a compatible 4 card, then enter 0.\n" << player << "\n";
        break;
    }
    case Rank::Three:
    {
        std::cout << "\nTop card from pile: " << _pile << "\nThis is 3 card and you will need to receive 3 cards from the deck.\n"
            << "If you have a 4 card that has the same suit, then you can stop it.\n";
        std::cout << "What card would you like to put? If don't have a compatible 4 card, then enter 0.\n" << player << "\n";
        break;
    }
    case Rank::Seven:
    {
        std::cout << "\nTop card from pile: " << _pile << "\nThis is a 7 card and you will need to put down a card that has the specified suit. If you have a Joker, then"
            << " you can put it. Enter 0 if you don't have such a compatible card.\n" << player << "\n";
        break;
    }
    case Rank::Ace:
    {
        std::cout << "\nTop card from pile: " << _pile << "\nThis is an A card."<< player.name() <<"'s turn was skipped.\n";
        break;
    }
    }
}

void Game::receiveCardsFromDeck(Player& player, const unsigned int& numberOfCards)
{
    try
    {
        player.receive(_deck.dealCards(numberOfCards));
    }
    catch (const NotEnoughCardsException& error)
    {
        _deck.refill(_pile.releaseAllCardsButFirst());
        player.receive(_deck.dealCards(numberOfCards));
    }
    catch (const EmptyDeckException& error)
    {
        _deck.refill(_pile.releaseAllCardsButFirst());
        player.receive(_deck.dealCards(numberOfCards));
    }
}

void Game::putCardToPile(Player& player, unsigned int& cardNumber)
{
    _pile.addCard(player.putCard(cardNumber));
}

bool Game::isSpecialCard(const Card& card) const
{
    return (_specialCards.find(card) != _specialCards.end());
}


const Player& Game::findWinner()
{
    auto it{ std::find_if(_players.begin(), _players.end(),
        [&](const Player& player) { return !player.numberOfCards(); }) };

    return *it;
}

void Game::dealInitialCardsToEachPlayer()
{
    std::for_each(_players.begin(), _players.end(),
        [this](Player& player) {player.receive(_deck.dealCards(initialNumberOfCardsPerPlayer)); });
}

void Game::validatePlayerCardCompatibilityWithPileTopCard(Player& player, unsigned int& cardNumber)
{
    while (!player.getCard(cardNumber).isCompatibleWith(_pile.getTopCard()))
    {
        std::cout << "This card is incompatible.\nEnter another card or enter 0 to skip your turn.\n";
        std::cin >> cardNumber;

        if (!cardNumber)
            break;
    }
}

void Game::normalCardPlayerTurn(Player& player)
{
    unsigned int cardNumber{ 0 };

    printInformationAboutThePlayerAndTheTopCardFromPile(player);
    std::cin >> cardNumber;

    if (!cardNumber)
        receiveCardsFromDeck(player, defaultNumberOfCardsToPick);
    else
    {       
        validatePlayerCardCompatibilityWithPileTopCard(player, cardNumber);
        if (!cardNumber)
            receiveCardsFromDeck(player, defaultNumberOfCardsToPick);
        else
            putCardToPile(player, cardNumber);
    }

    if (!player.numberOfCards())
        _isRunning = false;
}

void Game::specialCardPlayerTurn(Player& player)
{
    unsigned int cardNumber{ 0 };

    switch (_pile.getTopCard()._rank)
    {
    case Rank::Two:
    {
        printInformationAboutThePlayerAndTheTopCardFromPile(player, Rank::Two);
        std::cin >> cardNumber;

        if (!cardNumber)
            receiveCardsFromDeck(player, neededCardsToPickForTwo);
        else if (player.getCard(cardNumber)._rank == Rank::Four && player.getCard(cardNumber).isCompatibleWith(_pile.getTopCard()))
            putCardToPile(player, cardNumber);
        else
        {
            validatePlayerCardCompatibilityWithPileTopCard(player, cardNumber);
            if (!cardNumber)
                receiveCardsFromDeck(player, neededCardsToPickForTwo);
            else
                putCardToPile(player, cardNumber);
        }
        break;
    }

    case Rank::Three:
    {
        printInformationAboutThePlayerAndTheTopCardFromPile(player, Rank::Three);
        std::cin >> cardNumber;

        if (!cardNumber)
            receiveCardsFromDeck(player, neededCardsToPickForThree);
        else if (player.getCard(cardNumber)._rank == Rank::Four && player.getCard(cardNumber).isCompatibleWith(_pile.getTopCard()))
            putCardToPile(player, cardNumber);
        else
        {
            validatePlayerCardCompatibilityWithPileTopCard(player, cardNumber);
            if (!cardNumber)
                receiveCardsFromDeck(player, neededCardsToPickForThree);
            else
                putCardToPile(player, cardNumber);
        }
        break;
    }
    case Rank::Ace:
    {
        printInformationAboutThePlayerAndTheTopCardFromPile(player, Rank::Ace);
        break;
    }
    }

    if (!player.numberOfCards())
        _isRunning = false;
}

void Game::run()
{
    dealInitialCardsToEachPlayer();
    _pile.addCard(_deck.dealCard());

    while (isSpecialCard(_pile.getTopCard()))
    {
        _deck.refill(_pile.eraseTopCard());
        _pile.addCard(_deck.dealCard());
    }

    Card lastCard{ _pile.getTopCard() };
    bool specialCardHadEffect{ false };
    while (_isRunning)
    {
        for (auto& currentPlayer : _players)
        {
            if (!isSpecialCard(_pile.getTopCard()))
                normalCardPlayerTurn(currentPlayer);
            else
            {
                if (isSpecialCard(_pile.getTopCard()) && !specialCardHadEffect)
                {
                    specialCardPlayerTurn(currentPlayer);
                    specialCardHadEffect = true;
                }
                else
                    normalCardPlayerTurn(currentPlayer);
            }

            if (_pile.getTopCard() != lastCard)
            {
                specialCardHadEffect = false;
                lastCard = _pile.getTopCard();
            }

            if (!_isRunning)
                break;
        }
    }

    std::cout << "\tThe winner is " << findWinner().name() << ".\n";
}

main.cpp

#include "Game.hpp"
#include <iostream>

int main()
{
    unsigned int numberOfPlayers{ 0 };
    std::cout << "Number of players.\n";
    std::cin >> numberOfPlayers;
    std::cin.ignore();
    try
    {
        Game game(numberOfPlayers);
        game.run();
    }
    catch (const InvalidNumberOfPlayersException& error)
    {
        std::cerr << error.what() << "\n";
        std::cin >> numberOfPlayers;
        Game game(numberOfPlayers);
        game.run();
    }
    return 0;
}

Answer 1

CardProperties.hpp

#pragma once

Don't use #pragma once. It's not standard, it's not portable, and it doesn't always work. Use include guards.

const std::array<std::string, numberOfSuits> suits{"Diamonds","Clubs" // ...

From C++17 on, you can make this constexpr with std::string_view and inline:

inline constexpr auto suits = std::array{ std::string_view{"Diamonds"}, std::string_view{"Clubs"}, // ...

But if I may make another suggestion... this design is very brittle. It will "work" so long as the Suit enumeration is not toyed with. But if someone does:

enum class Suit
{
    Diamonds = 1, // I prefer suits have non-zero values so I can use 0 for error/unknown suits
    Clubs,
    // ...

or:

enum class Suit
{
    Diamonds,
    Clubs,
    Hearts,
    Spades,
    Red   = 998, // these are not real suits, so
    Black = 999, // I want them to have very different values
};

or:

enum class Suit
{
    Diamonds,
    Clubs,
    Spades,
    Hearts,
    Red,
    Black
};
// did you even notice what I did there?

then the code breaks, possibly catastrophically.

If you made suits like this:

inline constexpr auto suits = std::array{
    std::tuple{Suit::Diamonds, std::string_view{"Diamonds"},
    std::tuple{Suit::Clubs, std::string_view{"Clubs"},
    // ...

and you did suit-to-string conversions like:

auto const p = std::find_if(suits.cbegin(), suits.cend(), [s=card._suit](auto&& i) { return std::get<Suit>(i) == s; });
if (p == suits.cend()) ... // or assert(p != suits.cend())
auto const suit_str = std::get<1>(*p);

then your code would be safer, more maintainable, and more flexible.

Same idea goes for ranks.

((And I forgot to mention earlier....)) Once you define suits and ranks like this, then you can define numberOfSuits and numberOfRanks like this:

constexpr auto numberOfSuits = suits.size();
constexpr auto numberOfRanks = ranks.size();

Or even better, get rid of both of those constants because you don't need them anymore.

friend std::ostream& operator<<(std::ostream& os, const Card& card);

You have std::ostream in the header, but never include anything to get it. You should #include <iosfwd> in the header, and #include <ostream> in the cpp file.

The only reason you need this function to be a friend is because you have no accessor functions for the rank or suit. You should have those, and if you do, this function doesn't need to be a friend.

Card.hpp

class Game;

Making Game a friend of Card doesn't seem to make sense. All Game appears to need friendship for is to check the rank/suit of the card... but that's something that would be useful in any case with a card. What point is there is having cards where the rank and suit are invisible?

Instead, you should just give the Card class a set of observer functions:

constexpr auto suit() const noexcept { return _suit; }
constexpr auto rank() const noexcept { return _rank; }

Now instead of card._rank, Game can just do card.rank(), and there's no need for friendship. Plus your card class is now reusable for other games, and other stuff (like maybe a card-counting program).

Card(const Suit& suit, const Rank& rank);

Your code is a little const&-happy. You don't need const& everywhere; that was taught as the near-universal default in C++98, but it hasn't been so since C++11.

In particular:

• You don't need const& parameters for tiny, fundamental types like int and enumerations made up of int (as yours are). In fact, const& will probably make code slower in that case.
• You don't need const& parameters for sink functions - functions that are taking the argument and storing it somewhere. For example, your constructors both take the suit and rank and store it in the class's data-members.

So your two constructors can be:

constexpr Card(Suit, Rank);
constexpr Card(Rank rank, Suit suit) : Card{std::move(suit), std::move(rank)} {}

note they can also be constexpr, and defining the flipped-argument version in the header makes it inline. The moves are unnecessary for enums, but they're also completely free, so I added them to show general best practices.

bool isCompatibleWith(const Card& other) const;

First, this function can be constexpr and noxexcept.

More importantly, though: this function should not be in the card class. Whether two cards are "compatible" is not a property of cards... it's a property of the game. This function should be in the Game class.

bool operator != (const Card& other) const;
bool operator == (const Card& other) const;
bool operator < (const Card& other) const;

As a general rule, you shouldn't define comparison operators in the class. They should be free functions, for a number of reasons, not least being to allow conversions for both operands, not just the right-hand side.

If you have proper observer functions - which you should anyway - these functions won't even need to be friends of the class. You could just do:

inline constexpr bool operator==(Card const& lhs, Card const& rhs) const noexcept
{
    return lhs.rank() == rhs.rank() && lhs.suit() == rhs.suit();
}

inline constexpr bool operator!=(Card const& lhs, Card const& rhs) const noexcept
{
    return !(lhs == rhs);
}

Now as for ordering, that's something that probably shouldn't be a property of the cards. In some games, an ace is higher than a king, and in others it's lower than two (and in some games it's both). As with card "compatibility", ordering is a property of the game, not the cards. So the ordering functions should be in the game.

But you need it for std::set, you say. std::set has you covered. In your Game class, you would define a nested type CardOrdering, defined something like this:

struct CardOrdering
{
    constexpr bool operator()(Card const& lhs, Card const& rhs) const noexcept
    {
        // return true if lhs is "less than" rhs
    }
};

And then your set would be:

const std::set<Card, CardOrdering> _specialCards{ { Suit::Clubs, Rank::Two }, // ...

Card.cpp

bool Card::invalidCardProperties(const Suit& suit, const Rank& rank) const
{
    if (rank == Rank::Joker && (suit != Suit::Black && suit != Suit::Red))
        return true;
    if (rank != Rank::Joker && (suit == Suit::Black || suit == Suit::Red))
        return true;
    return false;
}

The purpose of this function is to validate cards, but by returning bool you throw a lot of information away. In fact, in your constructor you do:

if (invalidCardProperties(suit, rank))
    throw InvalidCardException("The Joker can only be red or black.\n");

except that all you know is that the card is "invalid". You don't know it's a Joker, or if the problem is that it's a Joker with a suit.

You have all the information you need in the function, so there's no point in tossing it all away and returning true or false. Instead, you can do something like this:

constexpr void Card::validate() const
{
    if (_rank == Rank::Joker && (_suit != Suit::Black && _suit != Suit::Red))
        throw InvalidJokerException{_suit};
    if (rank != Rank::Joker && (suit == Suit::Black || suit == Suit::Red))
        throw InvalidCardException{_rank, _suit};
    // ... any other checks you want to do...
}

Then your constructor can just be:

constexpr Card::Card(Suit suit, Rank rank) :
    _suit{suit},
    _rank{rank}
{
    validate();
}

Exceptions.hpp

It's good that you're using custom exceptions inherited from std::exception, but there are a few issues with the way you're doing it.

First, unless you're trying to create your own exceptions hierarchy, inheriting directly from std::exception isn't a good idea. Normally you would inherit at least from logic_error or runtime_error.

So, for example, your invalid card exception is clearly a logic error, so you'd just need to do:

struct InvalidCardException : std::logic_error
{
    using std::logic_error::logic_error;
    // or, even better, create your own constructor that takes a
    // rank and suit, and generates a message.
};

Not only that, you should look for inheritance opportunities in your exceptions. For example, an "empty deck" seems like a special case of "not enough cards", so EmptyDeckException should inherity from NotEnoughCardsException. Note that if you do that, some things automatically get easier elsewhere in your code. For example:

void Game::receiveCardsFromDeck(Player& player, const unsigned int& numberOfCards)
{
    try
    {
        player.receive(_deck.dealCards(numberOfCards));
    }
    catch (const NotEnoughCardsException& error)
    {
        _deck.refill(_pile.releaseAllCardsButFirst());
        player.receive(_deck.dealCards(numberOfCards));
    }
    catch (const EmptyDeckException& error)
    {
        _deck.refill(_pile.releaseAllCardsButFirst());
        player.receive(_deck.dealCards(numberOfCards));
    }
}

simplifies to:

void Game::receiveCardsFromDeck(Player& player, const unsigned int& numberOfCards)
{
    try
    {
        player.receive(_deck.dealCards(numberOfCards));
    }
    catch (const NotEnoughCardsException& error)
    {
        // will automatically handle the EmptyDeckException too
        _deck.refill(_pile.releaseAllCardsButFirst());
        player.receive(_deck.dealCards(numberOfCards));
    }
}

However, that's another issue I'll be getting to later.

There is another problem with your exceptions: all your what() functions should be noexcept.

On with the review!

Deck.hpp

Now you have two classes Deck and Pile... but riddle me this: what exactly is the difference between a deck and a pile?

The way I see it, there is literally no difference between a deck of cards and a pile of cards, except that a deck should default construct to 54 unique cards (the 52 regular cards plus the two jokers), whereas a pile should default construct to empty (no cards).

Other than that... aren't they exactly the same? You can take cards from the deck... you can take cards from a pile. You can add cards to the deck... you can add cards to the pile. In both cases, they'll act like a stack - whatever has been most recently added will be what you get when you draw. You can shuffle both.

It seems to me that you can simplify your design a lot if you just create a Pile class, and then make Deck a derived class from that that initializes with the full set of cards.

For example, suppose you defined Pile like this:

class Pile
{
public:
    Pile() noexcept = default;

    auto add(Card) -> void; // adds a single card to the top of the pile
    template <typename InputIterator>
    auto add(InputIterator first, InputIterator last) -> void; // adds multiple cards in order to the top

    auto draw() -> Card; // draws a card from the top of the pile
    auto draw(std::size_t) -> std::vector<Card>; // draws multiple cards

    auto discard() -> void; // draws a card from the top of the pile and just discards it
    auto discard(std::size_t) -> void; // discards multiple cards

    template <typename PRNG>
    auto shuffle(PRNG&& prng)
    {
        std::shuffle(_cards.begin(), _cards.end(), std::forward<PRNG>(prng));
    }

    auto empty() const noexcept -> bool;
    auto size() const noexcept -> std::size_t;
    // other stuff to satisfy Container requirements

private:
    std::vector<Card> _cards;
};

An interface like that seems both totally generic, and yet capable of satisfying all your needs. It would work for pretty much any card game, not just the one you're making. That's what writing reusable code is all about.

And then Deck would just be:

class Deck : public Pile
{
public:
    static auto standard() -> Deck; // returns a standard card deck
    static auto standard_with_jokers() -> Deck; // returns standard deck plus Jokers
    // possibly other common standard decks...

    Deck(); // initializes with the standard() deck

    // you could maybe add some handy extra functions, like:
    auto deal(std::size_t num_players, std::size_t num_cards) -> std::vector<std::vector<Card>>;
    // the above deals cards from the pile/deck using the dealing
    // algorithm - each player gets one card, then each player
    // gets a second card, and so on until all players have the
    // right number of cards.
};

and that's all you'd need for that!

Anywho, on with the review....

std::vector<Card> dealCards(const unsigned int& numberOfCards);

unsigned int is not really the right type to use here (and, of course, it shouldn't be passed by const&). When you're dealing with sizes or counts, you really only have two practical options: int or std::size_t. int should be your first choice, generally speaking, because unsigned types do some wacky things that can really screw up your code if you're not careful. However, the standard library uses unsigned types for its sizes - many experts think this was a mistake (it's why we have nonsense like different types for size_type and difference_type)... but it's a mistake we're stuck with.

So any time you're dealing with a size, you should choose between int or std::size_t. Pick one and be consistent.

void refill(const std::vector<Card>& cards);
void refill(const Card& card);

Once again, neither of these call for a const& type. But you can do even better with the interface here.

How about a function that let's you refill the deck from a std::set? Why not - you're actually using one in the program, so it's not a crazy idea. How about a function that lets you load a set of cards from a file?

Whenever you're dealing with a bunch of values, the first thing you should reach for should be iterators. Iterators are the building blocks of algorithms - once you can do something with iterators, you can do it with anything.

So you could do:

template <typename InputIterator>
void refill(InputIterator first, InputIterator last)
{
    _cards.insert(_cards.end(), first, last);
}

And once you have that, you can do:

template <typename InputRange>
void refill(InputRange&& range)
{
    using std::begin;
    using std::end;
    refill(begin(range), end(range):
}

and that will not only work for std::vector<Card>, it will work for std::set<Card> and even std::istream_iterator<Card> for reading from a file (assuming you implemented operator>> for Card), and more.

private:
    void shuffle();

Making shuffling a private function hidden from users seems like a poor design decision. Why shouldn't users of a deck of cards be the ones who decide when and how the cards get shuffled?

Deck.cpp

Deck::Deck()
{
    fill();
    shuffle();
}

As a user of the class, I would be annoyed that the deck doesn't let me decide when to shuffle. I might specifically want an unshuffled deck for predictability or testing. Why wouldn't you give me that choice?

const Card& Deck::dealCard()
{
    if (_cards.empty())
        throw EmptyDeckException("");

    Card& topCard(_cards.back());
    _cards.pop_back();
    return topCard;
}

You have a nasty bug here due to your fixation on using references. topCard is a reference to the last card in the _cards vector. Fine... but then you remove that card! So now topCard is a reference to a destroyed card! And, worse, you return that dangling reference!

The only reason this doesn't blow up in your face is because Card is a trivial type, and pop_back() probably doesn't release any memory. So neither the operating system nor the library detects that you're using a destroyed object in (theoretically) non-existent memory.

All this would be fixed if you just stopped with all the reference-craziness and used value semantics the way Lord Stroustrup intended:

Card Deck::dealCard()
{
    if (_cards.empty())
        throw EmptyDeckException{};

    auto topCard = _cards.back();
    _cards.pop_back();
    return topCard;
}

Incidentally, you keep raising custom exceptions like: throw ExceptionType("");. Why? Why not just define the custom exception type with a default message like:

struct EmptyDeckException : std::logic_error
{
    EmptyDeckException() :
        std::logic_error{"the deck is empty"}
    {}
};

And the same for other custom exception types. You could even add handy information for debugging, like:

class NotEnoughCardsException : public std::logic_error
{
public:
    NotEnoughCardsException(std::size_t requested, std::size_t available) :
        std::logic_error{_make_message(requested, available)},
        requested{requested},
        available{available}
    {}

    std::size_t requested = 0;
    std::size_t available = 0;

private:
    static auto _make_message(std::size_t requested, std::size_t available)
    {
        auto oss = std::ostringstream{};
        oss << "not enough cards (" << available <<
            ") to satisfy request (" << request << ')';
        return oss.str();
    }
};

struct EmptyDeckException : NotEnoughCardsException
{
    explicit EmptyDeckException(std::size_t requested) :
        NotEnoughCardsException{requested, 0}
    {}
};

On to dealCards():

std::vector<Card> Deck::dealCards(const unsigned int& numberOfCards)
{
    if (_cards.size() < numberOfCards)
        throw NotEnoughCardsException("");

    std::vector<Card> requestedCards;
    auto it{ std::prev(_cards.end(), numberOfCards) };

    std::move(it, _cards.end(), std::back_inserter(requestedCards));
    _cards.erase(it, _cards.end());

    return requestedCards;
}

This function is particularly convoluted and inefficient.

First, don't define variables until they're needed. You don't need requestedCards to figure out it.

Instead, figure out it, then create requestedCards using it in one go:

auto const it = std::prev(_cards.end(), numberOfCards);

auto requestedCards = std::vector<Card>{it, _cards.end()};
_cards.erase(it, _cards.end());

return requestedCards;

Doing it with std::back_inserter means the cards are added one at a time, with potentially multiple allocations... very inefficient. Doing it right in the constructor gives the library a chance to optimize - it can figure out how many cards are being added, do a single allocation, and then add them all.

If you really want to do moves - which doesn't hurt, but doesn't gain you anything here, you can do:

auto requestedCards = std::vector<Card>{
    std::make_move_iterator(it),
    std::make_move_iterator(_cards.end())
};

and you'll get the same benefits.

void Deck::fill()
{
    constexpr unsigned int numberOfSuitsWithoutJokerSpecificOnes{ 4 };
    constexpr unsigned int numberOfRanksWithoutJokers{ 13 };

    for (unsigned int index1 = 0; index1 < numberOfRanksWithoutJokers; ++index1)
    {
        for (unsigned int index2 = 0; index2 < numberOfSuitsWithoutJokerSpecificOnes; ++index2)
        {
            Card card(static_cast<Suit>(index2), static_cast<Rank>(index1));
            _cards.push_back(card);
        }
    }

    /*Card redJoker(Suit::Red, Rank::Joker);
    _cards.push_back(redJoker);
    Card blackJoker(Suit::Black, Rank::Joker);
    _cards.push_back(blackJoker);*/
}

This is a very brittle and damn-near-unmaintainable way of doing this. The slightest change to either Suit or Rank, and this silently breaks. It's also horrendously inefficient, because of repeated push_back()s, which will likely trigger multiple allocations.

Instead what you should do is have a static constexpr array, like this:

namespace {

constexpr auto default_deck = std::array{
    Card{Rank::Ace, Suit::Diamonds},
    Card{Rank::Two, Suit::Diamonds},
    // etc.
};

} // anonymous namespace

So long as this is confined to a single source file, it would be okay to use macros for this (at least until we get static reflection!). Especially if you want to have multiple default decks - like one with jokers and one without. Or you could try using some clever metaprogramming if you're up to it.

Once you've got that, fill() is trivial:

void Deck::fill()
{
    _cards.assign(default_deck.cbegin(), default_deck.cend());
}

On to shuffle():

void Deck::shuffle()
{
    std::mt19937 randomNumberGenerator{ std::random_device{}() };
    std::shuffle(_cards.begin(), _cards.end(), randomNumberGenerator);
}

This is not the way to use random number generators - you shouldn't be recreating the engine every time the function is called.

But this is a bad place to be doing all of this anyway. Shuffling control should not be in the deck of cards, it should be provided by the caller. The major reason for that is that it would allow a game to save its random generator state, and reproduce the same "random" game. Why is that important? Two reasons:

• REPLAY: If the game can save its RNG state and reproduce the same random numbers, then that along with saving player moves allows it to save entire games and reproduce them, either to show off cool plays or simply for a "demo mode".
• CHEAT DETECTION: If someone is suspected of cheating, being able to regenerate exacty the same deals and such will make it easy to catch them.

Looking at it another way: When have you ever played with a deck of cards that shuffled itself without your control?

Pile.hpp

Nothing to say here, except:

void addCard(const Card& card);

No need for const& here; it's a sink function.

Pile.cpp

constexpr unsigned int minimumValueOfPlayedCards{ 1 };
constexpr unsigned int lastElementOffset{ 1 };

These constants don't really need to be global, do they?

void Pile::addCard(const Card& card)
{
    _alreadyPlayedCards.push_back(card);
}

As mentioned, this is a sink function, so:

void Pile::addCard(Card card)
{
    _alreadyPlayedCards.push_back(std::move(card));
}

Now the next function:

std::vector<Card> Pile::releaseAllCardsButFirst()
{
    std::vector<Card> releasedCards;

    auto it{ std::next(_alreadyPlayedCards.begin(), _alreadyPlayedCards.size() - lastElementOffset) };

    std::move(_alreadyPlayedCards.begin(), it, std::back_inserter(releasedCards));
    _alreadyPlayedCards.erase(_alreadyPlayedCards.begin(), it);

    return releasedCards;
}

Isn't this really just the same as Deck::dealCards()? Except that you're essentially skipping the first card, dealing the whole deck, then putting that first card back?

In any case, the same things with Deck::dealCards() apply here.

const Card& Pile::eraseTopCard()
{
    const Card& requestedCard{ _alreadyPlayedCards.back() };
    _alreadyPlayedCards.pop_back();
    return requestedCard;
}

You've got a dangling reference here again.

std::ostream & operator<<(std::ostream & os, const Pile & pile)
{
    return os << pile._alreadyPlayedCards.back();
}

I'm not sure this is the way an output operation for a pile of cards should work. First, it will trigger UB if the pile is empty. Second, is it generally true that when outputting a pile of cards, you only write the top card?

Hand.hpp

void addCard(const Card& card);
void addCards(const std::vector<Card>& cards);

Same comments as for Deck::refill().

void removeCard(const unsigned int& cardNumber);

No need for const&. Should be int or std::size_t - just pick one and be consistent.

std::vector<Card>::iterator findCard(const Card& card);

This function doesn't really make a lot of sense as part of the public interface.

First, you're giving users of the class access right into the internal guts of the class. Users of Hand don't kneed to know it uses a vector<Card> internally. What if you changed it to use a set<Card> instead someday?

Second... it's pretty useless. What is a user supposed to do with it? If a user calls it and gets a result... what does that mean? Was the card found or not? Normally you'd check by comparing it with the vector's end iterator... but how would a user do that? They can't.

This function should be private, if anything.

const Card& getCard(const unsigned int& cardNumber);

As usual, no const&, and choose either int or size_t.

const unsigned int numberOfCards() const;

This should return int or size_t, and should probably be noexcept.

Hand.cpp

void Hand::removeCard(const unsigned int& cardNumber)
{
    _cards.erase(std::next(_cards.begin(), cardNumber - indexOffset));
}

You never check to see if cardNumber is valid. That could be a legit choice, but if so, you should at least document it.

const Card& Hand::getCard(const Card& card)
{
    auto it{ findCard(card) };
    return *it;
}

const Card& Hand::getCard(const unsigned int& cardNumber)
{
    return _cards[cardNumber - indexOffset];
}

In both of these functions you never bother to check whether the card or card number is actually in the hand.

Player.hpp

Player(const std::string& name = "undefined_name");

You use std::string in the class declaration, but never #include <string>.

This is a pure sink function, so there's no benefit to using const&. In fact, the function will probably be faster if you just take the std::string by value.

Now, is there any reason you have to make the player's hand private? Put another way: Is there any possible hand the player can have that is not legal?

When you're designing a class, you need to think in terms of class invariants. A class invariant is a rule that must always be true with the class. For example, a date class can have a day, month, and year int data members, with the invariant that they always describe a valid date. So you can't have a day greater than 31 or less than 1, you can't have a month outside the range of 1-12, you can't have the 31st of June, you can't have the 29th of February except on leap years, and so on. For that class, you can't give users unrestricted access to any of those three data members, because if they change one, they can break the class invariant.

But consider a 2D point class: it will have two double data members, for x and y. Are there any invalid values for either x or y in a point, either alone or in combination? No. So there's no need to make x and y private and then set up a bunch of setters and getters.

In your player class, it makes sense to make the name private: It should be set when the player is set up, and never allowed to change. But... what sense is there in making the hand private? The Hand class will make sure the hand is never invalid.

If you make the player's hand public, then there's no need for all those functions that do nothing but delegate to basically the same-named functions in the hand. In fact, your Player class could just be:

class Player
{
public:
    explicit Player(std::string name = "undefined_name");

    auto name() const noexcept -> std::string const& { return _name; }

    Hand hand;

private:
    std::string _name;
};

auto operator<<(std::ostream& os, Player const& player) -> std::ostream&
{
    return os << player.name() << "'s cards.\n" << player.hand;
}

That's all you need!

Player.cpp

Nothing to say here.

Game.hpp

Game(const unsigned int& numberOfPlayers = 2);

Once again, no const&, and choose between int or std::size_t.

Whenever a constructor takes a single argument, you should almost always declare it explicit. I know, it's a bummer this isn't the default, but we're stuck with the way it is.

So this constructor should be declared as:

explicit Game(int numberOfPlayers = 2); // or std::size_t instead of int

Moving on:

void putCardToPile(Player& player, unsigned int& cardNumber);

You forgot a const here probably, but I'm pretty sure you don't want to take the card number by reference in any case.

const Player& findWinner();

Even without looking at the implementation of this function, I can tell it's dodgy. Is there always a winner, every time this function is called? Unlikely. During the course of most games, there's usually no winner... not until the end. So what does this function return if there's no winner?

The way your code gets around this issue is by using a second variable - a flag, _isRunning. Apparently, if _isRunning is false, findWinner() must return a winner. That's a little convoluted, and brittle. _isRunning might be better named "game over" (and the logic flipped).

But even then, is it possible for the game to be over and not have a winner? For example, can the game tie (I don't know the rules)? What if someone quits early?

What you probably want to do here is return a (const) pointer. If there is no winner, the pointer will be null, otherwise it will point to the winner.

std::vector<Player> _players;
unsigned int _numberOfPlayers;

Is it really necessary to have two variables for this? Isn't _numberOfPlayers just _players.size()?

const std::set<Card> _specialCards{ // ...

I'm not sure you need this variable.

First of all, is it really necessary to make it a member of the Game class? It doesn't seem to ever change. It seems more like a static property, if anything.

Second, because it never changes, is there really any need to make it a set at all?

Instead of this, what you could do is have a constexpr static variable in the cpp file:

namespace {

constexpr auto special_cards = std::array{
    Card{ Suit::Clubs, Rank::Two },
    Card{ Suit::Diamonds, Rank::Two },
    // and so on
};

} // anonymous namespace

That should be much faster than a set.

Game.cpp

void Game::initializePlayerNames()

Looking at this function, it doesn't just initialize the player names... it actually initializes the players completely. It should have a more honest name.

But let me make a design suggestion. Right now, the only way to play this game is to pick a number of players, and then call Game{n}. The game then asks for each player's name from std::cin.

That's all fine and good... but it's a little limiting. What if I want to load player names from a file? What if I want to make a GUI for this game and get the player names from text boxes?

And that's not all. All players are Player classes... but what about supporting different kinds of players: human (using the console), human (using a GUI), human (using a network so you play online), AI (so you can play against the computer).

The way you'd probably do this is to make an abstract player class, and then different types of player classes on top of it. You might do:

• Player: abstract base class
  • HumanPlayer: class that gets player moves as input and validates them, giving error messages if they're invalid
    • ConsoleHumanPlayer: a HumanPlayer class that gets I/O using std::cin and std::cout
    • NetworkHumanPlayer: a HumanPlayer class that gets I/O using the network
    • ... and so on
  • AIPlayer: class that generates moves using artificial intelligence
    • EasyAIPlayer: easy-mode AI
    • HardAIPlayer: hard-mode AI
    • ... and so on

But how do you support all of these in the game?

Well, rather than just taking a number of players, you could have the game take the players as arguments. You'd probably need to use smart pointers to handle dynamic dispatch, so, very roughly:

class Game
{
public:
    template <typename InputIterator>
    Game(InputIterator first, InputIterator last) :
        _players{first, last}
    {
        // validate number of players
    }

    template <typename InputRange>
    Game(InputRange&& range) :
        Game{helper_begin(range), helper_end(range)}
    {}

private:
    template <typename InputRange>
    auto helper_begin(InputRange&& range)
    {
        using std::begin;
        return std::make_move_iterator(begin(range));
    }

    template <typename InputRange>
    auto helper_end(InputRange&& range)
    {
        using std::end;
        return std::make_move_iterator(end(range));
    }

    std::vector<std::unique_ptr<Player>> _players;

or it might be simpler to just default construct to no players, and have a set_players() function. Either way.

Now you'd set up the players in your main() function or something like that:

auto get_players()
{
    auto players = std::vector<std::unique_ptr<Player>>;

    std::cout << "Enter players.\n"
        "\t* Use <easy> to select easy AI.\n"
        "\t* Use <hard> to select hard AI.\n"
        "\t* Enter an empty name when done adding players.\n"

    for (auto name = std::string{}; std::getline(std::cin, name); )
    {
        if (name == "")
            break;
        else if (name == "<easy>")
            players.push_back(std::make_unique<EasyAIPlayer>());
        else if (name == "<hard>")
            players.push_back(std::make_unique<HardAIPlayer>());
        else
            players.push_back(std::make_unique<ConsoleHumanPlayer>(std::move(name));
    }

    return players;
}


int main()
{
    try
    {
        auto game = Game{get_players()};
        game.run();
        // ...

That will give the game much more flexibility.

void Game::receiveCardsFromDeck(Player& player, const unsigned int& numberOfCards)
{
    try
    {
        player.receive(_deck.dealCards(numberOfCards));
    }
    catch (const NotEnoughCardsException& error)
    {
        _deck.refill(_pile.releaseAllCardsButFirst());
        player.receive(_deck.dealCards(numberOfCards));
    }
    catch (const EmptyDeckException& error)
    {
        _deck.refill(_pile.releaseAllCardsButFirst());
        player.receive(_deck.dealCards(numberOfCards));
    }
}

Never use exceptions for normal control flow. Deck should have a size() function, so you can do:

void Game::receiveCardsFromDeck(Player& player, std::size_t numberOfCards) // or int instead of std::size_t
{
    if (_deck.size() < numberOfCards)
    {
        // check that _pile has enough cards, and if not, *then* you
        // should throw an exception, because that seems to be a
        // real error.
        _deck.refill(_pile.releaseAllCardsButFirst());
    }

    // assert(_deck.size() >= numberOfCards)
    player.receive(_deck.dealCards(numberOfCards));
}

Exceptions are not for normal program flow - they are for exceptional situations; things that should never happen, normally.

const Player& Game::findWinner()
{
    auto it{ std::find_if(_players.begin(), _players.end(),
        [&](const Player& player) { return !player.numberOfCards(); }) };

    return *it;
}

I've already mentioned that this function will fail catastrophically if there's not a player with no cards left.

But there are some other issues here, too. Don't use & in lambda captures generally... and it's especially not needed here.

const Player* Game::findWinner() const noexcept
{
    auto const it = std::find_if(_players.cbegin(), _players.cend(), [](auto&& player) { return !player.numberOfCards(); });
    if (it == _players.cend())
        return nullptr;
    return &(*it);
}

Moving on....

void Game::validatePlayerCardCompatibilityWithPileTopCard(Player& player, unsigned int& cardNumber)
{
    while (!player.getCard(cardNumber).isCompatibleWith(_pile.getTopCard()))
    {
        std::cout << "This card is incompatible.\nEnter another card or enter 0 to skip your turn.\n";
        std::cin >> cardNumber;

        if (!cardNumber)
            break;
    }
}

You don't do any checking here to make sure that the card number is valid... or even a number at all! What happens if a player types "X"?

Always check input!

void Game::normalCardPlayerTurn(Player& player)
{
    unsigned int cardNumber{ 0 };

    printInformationAboutThePlayerAndTheTopCardFromPile(player);
    std::cin >> cardNumber;

Don't declare variables until they're needed. cardNumber isn't needed until after the function call.

Always check input! What happens if the user enters a non-number, or a number that's out of range?

All the same points repeat for the rest of the functions.

main.cpp

I always recommend to beginners that they don't use formatted input with std::cin directly. In other words, don't do:

std::cin >> something;

Why? Because formatted input is problematic. It fails very easily, and when it does it leaves std::cin in a fail state... which can completely screw up the rest of the program.

Also, formatted input leaves stuff behind. Normally when you're getting input from std::cin, you're reading it line-by-line, so when someone enters a number, they type "42[enter]". std::cin >> i; will get the "42"... but leave the "[enter]"... so that if you do std::getline(std::cin, str);, this will get an empty string. That creates all kind of chaos... and you know this - you try to avoid the problem with std::cin.ignore(), but it won't work. You can see for yourself: when the program asks you for the number of players, type "2[space][enter]".

Here's my advice: Always use std::getline() with std::cin.

If you want a number, then do:

auto s = std::string;
std::getline(std::cin, s);
auto number = std::stoi(s);

You can wrap all that up in a function with error checking.

Now, as for the program's structure, you give the user exactly one chance to retry entering the number of players. Instead, you could do something like this:

while (true)
{
    try
    {
        std::cout << "Number of players.\n";

        // You can replace all this with a "read_uint()" function
        auto s = std::string;
        std::getline(std::cin, s);
        auto numberOfPlayers = std::stoul(s);

        Game game(numberOfPlayers);
        game.run();

        return 0;
    }
    catch (InvalidNumberOfPlayersException const& error)
    {
        std::cerr << error.what() << '\n';
    }
}

Now the program will keep asking for the number of players until the user gives a valid number. You could give them the option to enter "0" to just quit without playing.

Answers to questions in comments

std::array versus std::set and big-O

The dirty little secret of big-O notation is that it's mostly useless in practice.

The reason why is because big-O notation describes asymptotic behaviour - the behaviour as the number of elements goes to infinity. \$\mathcal{O}(\log n)\$ is \$N_1 \log n\$ and \$\mathcal{O}(n)\$ is \$N_2 n\$, and it's true that if \$n\$ is \$\infty\$, it doesn't matter what \$N_1\$ and \$N_2\$ are (assuming they're finite); the latter will always be larger.

The trick is that \$n\$ is NEVER \$\infty\$. It can't be - no computer has infinite memory, and none ever will. So which of \$N_1 \log n\$ and \$N_2 n\$ is larger for the maximum value of \$n\$ you're dealing with depends very much on \$N_1\$ and \$N_2\$... which kinda defeats the whole purpose of big-O notation.

And it turns out on modern computers, with all their caching tricks and predictive actions, the \$N_1\$ in std::set's \$N_1 \log n\$ behaviour is so freaking huge compared to the \$N_2\$ in a \$N_2 n\$ linear sort on a vector or array, that for all realistic values of \$n\$, std::set is slower.

If that blows your mind, you're not alone. Bjarne Stroustrup famously had to run the tests himself because he couldn't believe it.

Here's another way of thinking about it. If you compacted your Card type to be a single byte as @Deduplicator recommends, then the entire deck of cards can fit in a single 64-byte L1 cache line. But even if you don't, and in the worst likely case a Card is 16 bytes (2 64-bit ints, 8-bit bytes), then the 12 cards in your special card set will take up 192 bytes total in an array - or 3 cache lines. Meanwhile, in a std::set, assuming each node is a Card and two 64-bit pointers, your special card set will take up at least 384 bytes, and - more importantly - those 384 bytes could be spread all over memory.

So when you do a linear search on a std::array<Card, 12>, the processor will load 1 cache line of data... and - because this is what modern processors do - predictively load the next cache line of data at the same time for free. And once you start iterating through the cached data, the processor will predictively load the next one, so it will be ready before you even need it (hopefully). All-in-all, in the worst case scenario you've paid for 1 cache load (and got 2 for free).

But when you do a search through a std::set<Card> with 12 members, the processor will do 1 cache load for the first set node. If you're lucky the next set node will be in the cache already; if not, that's another cache load. It's the same with the next set node, and the next. In the worst case scenario, you've paid for four cache loads.

Why is this a big deal? Check out the numbers. A cache miss that requires loading data from main memory is two orders of magnitude slower than a cache hit. Even though a std::set's binary search only does 4 checks (worst case) on a 12-element set, while a linear search through a std::array does 12, each one of those 4 checks in the std::set is potentially 100 times slower than a check in the std::array. So with std::set you have \$N_1 \lg n\$, but \$N_1\$ is 100, which for \$n = 12\$ gives 358.5... while with std::array you have \$N_2 n\$ where \$N_2\$ is 1 which gives 12.

So forget big-O notation. Unless you're dealing with absolutely insane numbers of elements (like millions), or you need some of the other properties of other container types (like iterators that won't invalidate if you add/remove stuff), just use std::array if you know number of elements (and it won't change) and std::vector if you don't.

And as a bonus, if you use std::array instead of std::set, that will also make your special cards set constexpr. And constexpr usually means fast, and can even mean totally free in certain situations. If nothing else, constexpr at least means easier to test and catch bugs (because you can't have undefined behaviour at compile time).

Besides, if you really want \$\mathcal{O}(\log n)\$ search, you can always do a compile-time check that your special cards array is sorted, and use std::binary_search() instead of std::find(). std::binary_search() on an array will probably still be much faster than searching a std::set. It's probably not worth it for an array of just 12 elements, though.

The moral of the story:

• Forget big-O. It's pretty much useless in practice.
• Always make std::array or std::vector your first choice.

Answer 2

Well, my first Impression is "Gah! So many classes! Boilerplatomania!".

The fact of the matter is that C++ is multi-paradigm, and you are encouraged to choose the best way to do things, which is rarely straight-up fully object-oriented. Object-obsession is an illness too often seen in recovering Java-programmers and the like.

Your CardProperties.hpp works, but it creates an Independent static copy for each TU including it. You might not actually want that.

I'm thoroughly confused by the order you give the suits, that's very "original". Also, why don't you re-use two of the 4 suits for red/black?

Aside from that, prefer std::string_view over std::string if you have the choice.
Also, using a std::array instead of a native array makes things needlessly complicated.

You might to fix your parameter-passing: Anything up to a few pointers worth, unless copying is expensive, is best passed by value unless it's an out-parameter.

If you slightly change your representation, your Card could be a single byte big, instead of at least two. Admittedly, that means it must be intimately conversant with details of Suit and Rank, but that might still be worthwhile. The consuming code would never know, as you don't hardcode the size.

Consider deriving all your custom exceptions from a single custom exception for ease of use. And consider deriving that from std::runtime_exception instead, both for semantics and for it's proper handling of the message-buffer (see the notes).

By the way, "The Joker can only be red or black.\n" is an interesting response to trying to create a red two. Aside from the fact that ending the exceptions message with a newline is rarely expected.

Deck::dealCard() gets a reference to a card, removes the Card from ist vector thus turning it into a dangling reference, and then returns that reference. Are you sure you did want to use references at all there?

Don't bother using move-semantics for trivially copyable types. It's a complete waste of effort. Also, you can construct a std::vector directly from an iterator-range, that's clearer, more concise and more efficient than going the long way around.

I'm sure there's much more, but slogging through all that code and wishing for at least a little bit less verbosity is tiring.

Answer 3

Writing identifiers with a leading underscore is a bad idea, and is generally dissuaded as a style. Note that it is legal for member names if (and only if) the next character is not a capital letter or another underscore.

---

You used old-style syntax for some member initializers. Use uniform initialization uniformly.

---

unsigned int numberOfPlayers{ 0 };

Don’t use unsigned unless you really need the extra one bit of range or guaranteed semantics on overflow. (if anyone has a good link explaining why at a beginner/overview level, please comment)

---

throw EmptyDeckException("");

Constructing a string from a const char* only to have it turn out to be empty is needlessly inefficient. The uniform way to make empty objects is with the default constructor, so it is idiomatic to write string{} or just plain {} in context.

---

std::cerr << error.what() << "\n";

You could use '\n' instead of "\n" here.

---

Game(const unsigned int& numberOfPlayers = 2);

Huh? Why pass a simple int by const reference? I see you are doing this a lot. I think you must have some mistake in your mental model here.

Other than that, you seem well versed in up-to-date C++ features and standard library usage. So all-in-all, very good!

---

Note that you can put default initializers inline with the declarations of the data members. For those getting a simple value not depending on constructor arguments, you don’t have to even list them in the constructor.

 // class member
 bool is_running = true;

---

catch (const NotEnoughCardsException& error)
{
    _deck.refill(_pile.releaseAllCardsButFirst());
    player.receive(_deck.dealCards(numberOfCards));
}
catch (const EmptyDeckException& error)
{
    _deck.refill(_pile.releaseAllCardsButFirst());
    player.receive(_deck.dealCards(numberOfCards));
}

Both exceptions have the same reaction. If you don’t care what the exception is, don’t overdo the find-graining of the errors as different types. Throwing an error code is easiest: the catch block can look at the error code if it wants to, but catches module internal errors with one type. If exposing these outside the class, see std::error_code.

---

A lot of one-line single-return-statement function bodies could just be in the header, inline with the class definition.