Console Blackjack game, without split or betting system for now

I never programmed in my life, and I am currently self teaching myself some C++ by reading books, online free classes and googling. After beginning to read about OOP and classes, and after seeing that a Blackjack game would be something simple to implement using the console, I created the following program to play Blackjack on the console (No graphics, just text narrating what's going on).

Features:

• Aces can be worth 1 or 11, depending on what's better for the score.
• Dealer forced to hit if it has a soft 17 (score of 17 with at least one Ace).

Not implemented:

• Betting system - The player ether lose, tie or win.
• Split system - The player can't split his hand if dealt a pair.

card.h

#ifndef CARD_H
#define CARD_H

#include <iostream>

class Card
{
public:
enum CardSuit
{
CS_S,
CS_D,
CS_C,
CS_H,
CS_MAX
};

enum CardRank
{
CR_2,
CR_3,
CR_4,
CR_5,
CR_6,
CR_7,
CR_8,
CR_9,
CR_T,
CR_J,
CR_Q,
CR_K,
CR_A,
CR_MAX
};

private:
CardSuit m_suit;
CardRank m_rank;

public:
Card(CardSuit suit = CS_S, CardRank rank = CR_A)
:   m_suit {suit},
m_rank {rank}
{ }

void printCard() const;
int getCardValue() const;
};

#endif


card.cpp

#include "card.h"

void Card::printCard() const
{
switch (m_rank)
{
case CR_2: std::cout << '2'; break;
case CR_3: std::cout << '3'; break;
case CR_4: std::cout << '4'; break;
case CR_5: std::cout << '5'; break;
case CR_6: std::cout << '6'; break;
case CR_7: std::cout << '7'; break;
case CR_8: std::cout << '8'; break;
case CR_9: std::cout << '9'; break;
case CR_T: std::cout << 'T'; break;
case CR_J: std::cout << 'J'; break;
case CR_Q: std::cout << 'Q'; break;
case CR_K: std::cout << 'K'; break;
case CR_A: std::cout << 'A'; break;
}

switch (m_suit)
{
case CS_S: std::cout << 'S'; break;
case CS_D: std::cout << 'D'; break;
case CS_C: std::cout << 'C'; break;
case CS_H: std::cout << 'H'; break;
}
}

int Card::getCardValue() const
{
switch (m_rank)
{
case CR_2: return 2;
case CR_3: return 3;
case CR_4: return 4;
case CR_5: return 5;
case CR_6: return 6;
case CR_7: return 7;
case CR_8: return 8;
case CR_9: return 9;
case CR_T: return 10;
case CR_J: return 10;
case CR_Q: return 10;
case CR_K: return 10;
case CR_A: return 11;
}
return 0;
}


deck.h

#ifndef DECK_H
#define DECK_H

#include "card.h"
#include <array>
#include <vector>
#include <iostream>

class Deck
{
private:
std::array<Card, 52> m_card;
int m_cardIndex;

void swapCard(Card &a, Card &b);
inline Card* dealCard();

public:
std::vector<Card*> m_playerHand;
std::vector<Card*> m_dealerHand;

Deck() : m_cardIndex {0}, m_playerHand {}, m_dealerHand {}
{
int index {0};
for (int iii {0}; iii < Card::CS_MAX; ++iii)
{
for (int jjj {0}; jjj < Card::CR_MAX; ++jjj)
{
m_card[index] = Card(static_cast<Card::CardSuit>(iii), static_cast<Card::CardRank>(jjj));
++index;
}
}
}

void printDeck() const;
void shuffleDeck(int xTimes);
void dealPlayer();
void dealDealer();
};

inline Card* Deck::dealCard()
{
return &m_card[m_cardIndex++];
}

#endif


deck.cpp

#include "deck.h"
#include <random>
#include <chrono>

namespace Rng
{
const auto seed {std::chrono::high_resolution_clock::now().time_since_epoch().count()};
std::mt19937 mt {static_cast<unsigned long int>(seed)};

int rng(int min, int max)
{
std::uniform_int_distribution<> rng {min, max};
return rng(mt);
}
}

void Deck::swapCard(Card &a, Card &b)
{
Card temp {a};
a = b;
b = temp;
}

void Deck::printDeck() const
{
for (int iii {0}; iii < 52; ++iii)
{
m_card[iii].printCard();
if (((iii + 1) % 13 == 0) && iii != 0)
std::cout << '\n';
else
std::cout << ' ';
}
}

void Deck::shuffleDeck(int xTimes = 1)
{
for (int iii {0}; iii < xTimes; ++iii)
{
for (int jjj {0}; jjj < 52; ++jjj)
{
swapCard(m_card[jjj], m_card[Rng::rng(0, 51)]);
}
}
m_cardIndex = 0;
m_playerHand.clear();
m_dealerHand.clear();
}

void Deck::dealPlayer()
{
int index {static_cast<int>(m_playerHand.size())};
m_playerHand.resize(index + 1);
m_playerHand[index] = dealCard();
}

void Deck::dealDealer()
{
int index {static_cast<int>(m_dealerHand.size())};
m_dealerHand.resize(index + 1);
m_dealerHand[index] = dealCard();
}


main.cpp

#include "card.h"
#include "deck.h"
#include <iostream>
#include <vector>
#include <string>

int getPoints(std::vector<Card*> &hand)
{
int score {0};
int acesCount {0};
for (auto &card : hand)
{
score += card->getCardValue();
if (card->getCardValue() == 11)
++acesCount;
}
if (score > 21 && acesCount != 0)
{
do
{
score -= 10;
--acesCount;
} while (score > 21 && acesCount > 0);
}
return score;
}

void playGame(Deck &gameDeck)
{
gameDeck.shuffleDeck(20);
gameDeck.dealPlayer();
gameDeck.dealDealer();
gameDeck.dealPlayer();
gameDeck.dealDealer();
std::cout << "You were dealt |";
gameDeck.m_playerHand[0]->printCard();
std::cout << "| |";
gameDeck.m_playerHand[1]->printCard();
std::cout << "|\nDealer was dealt |";
gameDeck.m_dealerHand[0]->printCard();
std::cout << "| and a card facing down\nThe dealer peaks at the hole card.\n";
int playerScore {getPoints(gameDeck.m_playerHand)};
int dealerScore {getPoints(gameDeck.m_dealerHand)};
if (playerScore == 21 && dealerScore != 21)
{
std::cout << "You have a Blackjack!\n"
"You win the game.";
return;
}
else if (dealerScore == 21 && playerScore != 21)
{
std::cout << "The dealer flips the hole card to reveal "
"a Blackjack with cards |";
gameDeck.m_dealerHand[0]->printCard();
std::cout << "| and |";
gameDeck.m_dealerHand[1]->printCard();
std::cout << "|\nYou lose the game.\n";
return;
}
else if (playerScore == 21 && dealerScore == 21)
{
std::cout << "You have a Blackjack.\n"
"The dealer flips the hole card to reveal"
"a Blackjack with cards |";
gameDeck.m_dealerHand[0]->printCard();
std::cout << "| and |";
gameDeck.m_dealerHand[1]->printCard();
std::cout << "|\nThe game is a tie\n.";
return;
}
// Player Hit loop:
bool exitPlayerLoop {false};
while (!exitPlayerLoop)
{
std::cout << "Choose your action: [H]it or [S]tand\n";
std::string action {};
std::cin >> action;
switch (action.front())
{
case 'h':
case 'H':
{
gameDeck.dealPlayer();
std::cout << "You were dealt a |";
gameDeck.m_playerHand[gameDeck.m_playerHand.size() - 1]->printCard();
for (auto &card : gameDeck.m_playerHand)
{
std::cout << " |";
card->printCard();
std::cout << '|';
}
std::cout << '\n';
playerScore = getPoints(gameDeck.m_playerHand);
if (playerScore > 21)
{
std::cout << "You busted. You lose the game.\n\n";
return;
}
}
break;
case 's':
case 'S':
{
std::cout << "You stood. Your hand is";
for (auto &card : gameDeck.m_playerHand)
{
std::cout << " |";
card->printCard();
std::cout << '|';
}
exitPlayerLoop = true;
}
break;
default:
std::cout << "Invalid input\n";
}
}
std::cout << "\nThe dealer flips the hole card. It reveals a |";
gameDeck.m_dealerHand[1]->printCard();
std::cout << "|\n";
// Dealer hit loop:
bool exitDealerLoop {false};
while (!exitDealerLoop)
{
int dealerAcesCount {0};
for (auto &card : gameDeck.m_dealerHand)
{
if (card->getCardValue() == 11)
++dealerAcesCount;
}
dealerScore = getPoints(gameDeck.m_dealerHand);
if (dealerScore < 17 || (dealerScore == 17 && dealerAcesCount > 0))
{
gameDeck.dealDealer();
std::cout << "Dealer hits and was dealt a |";
gameDeck.m_dealerHand[gameDeck.m_dealerHand.size() - 1]->printCard();
std::cout << "|\n";
continue;
}
std::cout << "Dealer hand:";
for (auto &card : gameDeck.m_dealerHand)
{
std::cout << " |";
card->printCard();
std::cout << '|';
}
std::cout << '\n';
exitDealerLoop = true;
}
playerScore = getPoints(gameDeck.m_playerHand);
dealerScore = getPoints(gameDeck.m_dealerHand);
if (dealerScore > 21)
{
std::cout << "The dealer busted. You win the game!\n\n";
}
else if (playerScore > dealerScore)
{
std::cout << "You win the game!\n\n";
}
else if (playerScore < dealerScore)
{
std::cout << "You lose the game.\n\n";
}
else if (playerScore == dealerScore)
{
std::cout << "The game is a draw.\n\n";
}
return;
}

int main()
{
std::cout << "         Welcome to Blackjack!\n"
"* * RULES: Dealer must hit soft 17 * *\n\n";
Deck gameDeck;
bool exitLoop {false};
while (!exitLoop)
{
playGame(gameDeck);
bool validChoice = false;
while (!validChoice)
{
std::cout << "Play again? <Y/N>\n";
std::string choice {};
std::cin >> choice;
switch (choice.front())
{
case 'y':
case 'Y':
validChoice = true;
break;
case 'n':
case 'N':
validChoice = true;
exitLoop = true;
break;
default:
std::cout << "Invalid choice.\n";
}
}
}
std::cout << "\nThank you for playing!\n";
return 0;
}


I tried to put in functions the methods that were used more than once only, hence the long void playGame(Deck &gameDeck) function.

Considering that I am just beginning to learn the language, I want some improvement advice: What are my bad habits? What are my rookie mistakes? Should I separate the huge void playGame(Deck &gameDeck) function into smaller ones? Am I doing the multi file division the right way? I aimed for a file and header per class. Anything you guys would change to this particular program? Any advice to me in general?

PS: Can I move those enums out of the header file? I tried but with no success. They look ugly in a header file...

EDIT: I lied about never programming in my life. I did some RPGs with Rpg Maker 2000 during my youth. It had a basic scripting language.

In C++ &/* belongs with type. E.g. void swapCard(Card& a, Card& b);

The CTOR member init list becomes easier to read (IMO) when formatted like this:

Deck()
: m_cardIndex {0}
, m_playerHand {}
, m_dealerHand {}


You can and should use enum class over the "normal" one.
(Taken from here):

What is the difference between two?

• enum classes - enumerator names are local to the enum and their values do not implicitly convert to other types (like another enum or int)

• Plain enums - where enumerator names are in the same scope as the enum and their values implicitly convert to integers and other types

enum classes should be preferred because they cause fewer surprises that could potentially lead to bugs.

You're already wrapping your random number generation in a separate namespace so why not your own classes in general?
E.g.:

namespace ConsoleBlackjack
{

class Card
{
[...]

} // namespace ConsoleBlackjack


Subjective but iii/jjj seem unusual for a loop variable. Any specific reason for doing it this way?

Perhaps you could make use of std::shuffle instead of having to roll your own.

It would be good to get rid of your magic numbers (e.g. 5, 13, 17, 21, 52, 51 etc.). Instead turn them into named constants.
CS_MAX and CR_MAX should IMO also be turned into separate named constants instead of riding with the enums.

Use compiler warnings. I'm not sure which compiler you use but you should always enable and try to fix the compiler warnings. The way you enable them differs by compiler. It's best to look this up based on your compiler.

You're missing a default statement in your switches. This is a good reason to use compiler warnings. In this case you didn't miss any fields but it still complains because you put constants in there that should be separate (see above).

You should declare the RNG parts static as they are expensive. Have a look at this excellent code to see how it can be done (specifically the RockPaperScissors RockPaperScissors::random() part).

• this is the kind of answer I was expecting! I usually code with my phone using c4droid, because I don't have much home free time, so I use my mobile to learn it. The compiler it uses is g++ +Bionic. Don't know what to do, because the code doesn't throw me any warnings... When I move the code to visual studio it also doesn't show any warnings... Hmmm – msmilkshake Jun 1 '19 at 12:27
• @msmilkshake Usually warnings have to be explicitly enabled. I don't have much experience with visual studio but maybe this can help: learncpp.com/cpp-tutorial/… – yuri Jun 1 '19 at 13:04
• about your answer: You're already wrapping your random number generation in a separate namespace so why not your own classes in general? Whad to you mean? create a namespace for my classes? ; I use iii and jjj because I learned that using i, j, k alone ca be hard to find using ctrl+f. iii, jjj, kkk as counters are easier to find using ctrl+f – msmilkshake Jun 1 '19 at 13:45
• @msmilkshake (1) I meant just using namespaces for your classes, I updated the answer to hopefully make this clearer. (2) I've never heard of that advice but then again I also never tried searching for my loop variables. – yuri Jun 1 '19 at 14:06
• What warning argument should I add to my compiler to detect the missing default case? – msmilkshake Jun 1 '19 at 19:32

I never programmed in my life, and I am currently self teaching myself some C++ by reading books, online free classes and googling.

If that is true, then I applaud you. This code is better than some code I've seen from people who "know what they're doing". For one thing, I understood it and could tell what you were trying to do. Of course, you can still improve a lot. Don't take this as a "okay, I don't need to learn anything more". Everyone can get better, except people who mistakenly think they're already the best. But for a first try, not bad. Anyway, on with the review.

PS: Can I move those enums out of the header file? I tried but with no success. They look ugly in a header file...

Short answer no. It is necessary in the header file because you use the enums almost immediately on the next couple lines:

private:
CardSuit m_suit;
CardRank m_rank;


So you can't just remove them. If you don't like how they look, you will have to come up with another solution for m_suit and m_rank.

Not that you want them out of the header. If they are not in the header, then other things like your main() function can't use them. If they are in the header, it makes using them easier.

I tried to put in functions the methods that were used more than once only, hence the long void playGame(Deck &gameDeck) function.

If what you're saying is that only functions that get used more than once become part of the class, then I say you're on the right track.

Generally, you want functions that are specific to the data in the class to be methods of that class. You don't want anything that has nothing to do with the class, or is program specific being part of that class.

The reason is let's say you decide to make a Solitaire game for example. Well, you've already written a Card class and a Deck class. Those classes you could probably use again in solitaire. Thus, anything that you think might find use in Solitaire, you might want to make part of the class. So Deck::shuffleDeck() for example, might find use in Solitaire, so it's a good fit to make part of the class. playGame(), however, is program specific. It has no use in a Solitaire game, on in Blackjack. Thus, it's a bad candidate to make part of the class.

All of that to say, I guess, is that playGame() doesn't need to be part of Deck. Good choice.

Am I doing the multi file division the right way? I aimed for a file and header per class.

Yup. Your headers are great in my opinion. However, others may say that they are just good.

Should I separate the huge void playGame(Deck &gameDeck) function into smaller ones?

It does seem a little big. Sub-functions would reduce repeated code. Not fully sure how you can reduce at the moment, other than maybe if there was a was a way to not repeat the dealer and player hit process. Again, not sure how exactly you could do that at the moment, but that would help. Any place you've repeated the same basic code, a function will help.

What are my bad habits? What are my rookie mistakes?

I didn't look at the code closely enough to give a complete answer, but one I did catch was this:

public:
std::vector<Card*> m_playerHand;
std::vector<Card*> m_dealerHand;


Generally, it's better to keep these private if you can. Then, make a public interface for them. So, you could do, for instance:

class Deck {
private:
std::vector<Card*> m_playerHand;
public:
std::vector<Card*>& getPlayerHand() { return m_playerHand; }
}


You may have a good reason to make them public, or it may be just easier to make them public. But, if you can make them private, it is usually better to do so.

Anything you guys would change to this particular program? Any advice to me in general?

This one I have several points for improvements:

1. Consider adding a ostream& parameter to Card::PrintCard()

The reason I am suggesting this is because right now there is no way to re-direct the print of card. It only goes to std::cout. If you want to make it go to std::cerr or a fstream, for instance, you can't. Your code would be much more flexible if it accepted a ostream like this:

void Card::printCard(ostream& stream) const
{
switch (m_rank)
{
case CR_2: stream << '2'; break;
case CR_3: stream << '3'; break;
case CR_4: stream << '4'; break;
case CR_5: stream << '5'; break;
case CR_6: stream << '6'; break;
case CR_7: stream << '7'; break;
case CR_8: stream << '8'; break;
case CR_9: stream << '9'; break;
case CR_T: stream << 'T'; break;
case CR_J: stream << 'J'; break;
case CR_Q: stream << 'Q'; break;
case CR_K: stream << 'K'; break;
case CR_A: stream << 'A'; break;
}

switch (m_suit)
{
case CS_S: stream << 'S'; break;
case CS_D: stream << 'D'; break;
case CS_C: stream << 'C'; break;
case CS_H: stream << 'H'; break;
}
}


Of course, this breaks current code, since the current code isn't expecting a parameter, so you can overload the function lie this if you want:

void Card::printCard() const
{
printCard(std:cout);
}


That will make current code continue to work while making your printing far more flexible.

1. Consider adding a stream operator

Now, all I said about #1 is good, but there's another reason to implement a printCard() function that takes a ostream as a parameter. That is because creating a stream operator for our card class really easy:

ostream& operator <<(ostream& stream, Card c) {
c.printCard(stream);

return stream;
}


Now, with that in place, you have a new way to print to std::cout, and it looks like this:

std::cout << myCard;
stg::cout << "We can even put a message here: " << myCard << " and even put a message after, if we want too!\n";


In fact, std::cerr and fstreams work this way too. It makes things a lot easier.

1. Consider making a Hand class

Instead of using std::vector<Card*>, it would be much easier if you made a Hand class, or even a using or typedef name called Hand. It would look something like this:

class Hand {
// Option 1: create a class
};
// Or...
// Option 2: use using.
using Hand = std::vector<Card*>;
// or...
// Option 3: use a typedef
typedef std::vector<Card*> Hand;



Options 1 and 2 are preferred. Use 3 if you have to for some crazy unforseen reason.

This way, you can make a general purpose Deck::deal() function that would replace Deck::dealPlayer() and Deck::dealDealer():

void Deck::deal(Hand& hand) {
// implementation...
}


And turn the dealer and player hands into a Hand:

public:
Hand m_playerHand;
Hand m_dealerHand;


You know, this leads me to my next point:

1. m_playerHand and m_dealerHand seem unneeded as members of Deck

Instead, it feels like you should use them as member variables in playGame() instead:

void playGame(Deck &gameDeck)
{
Hand playerHand;
Hand dealerHand;

// implementation...

// then, if you take suggestion number 3, you can fill it like this:
gameDeck.deal(playerHand);
gameDeck.deal(dealerHand);

}


I'm sure there are lots of other things you could do, but I think this will get you started. Once you take my suggestions and yuri's suggestions, it will probably become more apparent how you could reduce your code even more.

• I should also note that once you take our suggestions, you can ask a new question with the new code if you want more feedback. If not, that's okay too. – Chipster Jun 3 '19 at 0:05
• Hey. Great answer! Never thought anybody would put this much work in reviewing code for rookies! I am mostly learning from learncpp.com and I haven't reached the operator overloading lessons yet, but I saw that those are gonna be discussed after where I am, so I didn't do any overloading because I simply didn't discuss it. There are a lot of good improvements in this answer and I am going to try to put them all in action. Things look way cleaner with them. That hand class looks like a really good adittion! To see if I got it: I would instantiate two hand objects, one for the dealer and one – msmilkshake Jun 4 '19 at 6:26
• ... For the player? Oh, I also don't know how templates work yet. I have a lot to learn and gotta keep reading those learcpp lessons. – msmilkshake Jun 4 '19 at 6:30
• Yes. One for the dealer and player. – Chipster Jun 4 '19 at 6:33
• Also, that's totally fine to not know operator overloading yet. It was a suggestion just in case you knew how to do that. Glad you're continuing to learn. – Chipster Jun 4 '19 at 6:34

Adding onto @Chipster's answer, I'd like to suggest some improvements for the Deck class:

class Deck
{
private:
std::array<Card, 52> m_card;
int m_cardIndex;

void swapCard(Card &a, Card &b);
inline Card* dealCard();

public:
std::vector<Card*> m_playerHand;
std::vector<Card*> m_dealerHand;

...

void printDeck() const;
void shuffleDeck(int xTimes);
void dealPlayer();
void dealDealer();
};

1. As @yuri suggested, make a DECK_SIZE variable that maybe gets set via a constructor to get rid of the magic number of 52 (you could use templates for this), even though that's the standard deck size. Also, m_card in the singular doesn't make sense to me. I'd say m_cards (or simply cards to get rid of the unnecessary Hungarian notation altogether).

2. From an object-oriented perspective, it doesn't make sense for a Deck to have m_playerHand and m_dealerHand. It makes more sense for these to be part of player and dealer classes (but players and dealers share a lot of common functionality, so a class hierarchy may make sense here—maybe an abstract base class of BlackjackEntity).

3. printDeck() is fine, but it can be replaced with the following friend function:

friend std::ostream& operator<<(std::ostream& os, const Deck& deck)

This would allow you to simply std::cout << referenceToDeck.

1. And finally, dealPlayer and dealDealer do exactly the same thing, just to different entities. Moreover, it makes more sense to move the dealTo method to the Dealer class (assuming you go down that route) and change its signature to be:

void Dealer::dealTo(Deck& deck, BlackjackEntity& recipient)

After all, it's the Dealer who deals, not the deck. The deck simply has the capacity to be dealt. This method would then call deck.dealCard() to get the card and give it to recipient, which is either this or a reference to the player. For this to work, both Dealer and Player would have to subclass BlackjackEntity. That base class would provide all methods common to Dealers and Players. The subclasses would add any methods unique to their respective entities (like dealing for the Dealer).

Hope that helps!

• Never heard of the Hungarian notation term before. I'm gonna Google that. The free lessons I'm reading online suggest that member functions should be written with prefix m_ just so people can tell they are member variables. To the rest of your answer, that all makes a lot of sense. I can picture it in my head now what methods belong to classes and what methods don't belong and would better live in their own classes. Thanks! – msmilkshake Jun 4 '19 at 6:39
• No problem! Yeah, the link I sent to the answer discusses some notable problems with Hungarian notation. Mainly, it makes your code very noisy (and, on bigger projects, much harder to maintain and write). It was originally intended for non-statically-typed languages (e.g., Python)—in other words, languages whose type is determined at runtime (as the program is running) as opposed to compile time. But with C++, this isn't a problem because the language is statically typed :) You have to say int x. Besides, if you're using an IDE, it should be easy to verify the type of a variable. – AleksandrH Jun 4 '19 at 13:20