OOP Blackjack in C++

I finished my Blackjack OO game and I'd like to get my OO skills reviewed.

```
#include
#include
#include
#include
#include

class Card
{
private:
char m_card; // 'A' ,'2', '3', '4', '5', '6', '7', '8', '9', 'T', 'J', 'Q'
int m_value, //1 or 11 , 2 , 3 , 4, 5 , 6 , 7 , 8 , 9, 10, 10, 10
m_suite; //0 spades, 1 clubs, 2 hearts, 3 diamonds
std::string m_nameS;
bool m_given; //Check if it's been wether given or not. works as an index

int assignValue();

public:
Card(char, int);
Card(const Card&);

int getValue() const;
int getSuite() const;
std::string getName() const;
bool getIfGiven() const;
char getCard() const;

void setGiven(bool x);

void nameCard();
};

int Card::assignValue()
{
if(m_card == 'A') return 11;
else if (m_card == 'T' ||
m_card == 'J' ||
m_card == 'Q') return 10;
else return (m_card - '0');
}

Card::Card(char cardV, int suite)
{
m_card = cardV;
m_value = assignValue();
m_suite = suite;
m_given = false;
}

Card::Card(const Card& card)
{
m_card = card.getCard() ;
m_value = card.getValue();
m_suite = card.getSuite() ;
m_nameS = card.getName() ;
m_given = card.getIfGiven() ;
}

int Card::getValue() const
{ return m_value; }

int Card::getSuite() const
{ return m_suite; }

std::string Card::getName() const
{ return m_nameS; }

bool Card::getIfGiven() const
{ return m_given; }

char Card::getCard() const
{ return m_card; }

void Card::setGiven(bool x)
{ m_given = x; }

void Card::nameCard()
{
switch(m_suite)
{
case 0: m_nameS = "spades" ; break;
case 1: m_nameS = "clubs" ; break;
case 2: m_nameS = "hearts" ; break;
case 3: m_nameS = "diamonds";
}
if(m_card == 'A') std::cout m_deck;

public:
Deck();

void fillD

Lot of code but here are some comments.

class Card

Personally I put private parts of a class at the end of the class, those are implementation details. The public parts OTOH are more important for the user of the class. (IMHO)

Use scoped enum for the card suit, as it is now you use char so any char can be accidentally assigned.

assignValue

assignValue depends on the order of when the member variable m_card was defined, it would be better to make it static and take as argument the card otherwise this can be an error source in the future.

A std::map could have been used to hold the values of the cards e.g. map

constructor

you do not initialize all declared variables, std::string is initialized automatically to empty string but it is good nevertheless to show that you initialize it for clarity. one day you may change the type of the variable and forget to add it to the constructor

nameCard

you seem to have omitted the King.

your switch sets a string for 0-3 but forgetting to call this function will forget to define that variable, you could instead make sure the card has a textual suit by defining it in the constructor same time you assign m_suite.

again the mapping from value to some string could be held in a std::map both for suits and for the textual values like "Ace". that would make things like Deck::fillDeck smaller.

Deck

Use a smart pointer instead of a raw pointer to the cards, that you do not have to worry about memory leaks.

std::vector> m_deck;

when you initialize the deck just do

m_deck.push_back( make_unique( c, n ) );

after that you can treat the pointers as any other pointer and you do not need to delete them.

giveCard

This function copies the card from the deck, which somehow seems a bit unintuitive, but ok that works i guess as well. Alternatively you could move the card to the players hand, then when you initialize a new deck just create 52 new cards in a fresh deck.

Hand

you may want to keep the hand sorted to make it easier for user.

display

There is no need to have the extra ( ) around ++iter

Player

You are inheriting from Player (Dealer) but Player has no virtual destructor, add one.

It is not necessary to write using Player::Player; in Dealer, when you inherit you can access the base class members that are protected/public. If you want to access private members then maybe you should reconsider why you made them private in the first place.

It seems wrong to declare the Deck to be a member variable in the Player class - do each player have their own marked deck? I would think it would be better to declare it outside Player/Dealer since they only have a handful of the cards at any time.

StackExchange Code Review Q#78710, answer score: 2