patterncppModerate
C++ Blackjack game
Viewed 0 times
gameblackjackstackoverflow
Problem
I made this small Blackjack game in the past as a way to practice C++ basics and have fun at the same time. I stopped working on the game during the last two years of my computer science degree program and now in my free time I want to improve it. I'm looking for suggestions on how I can improve the game. Not just adding new features, but how I can improve the organization, eliminate unnecessary code, and stick to common (industry standard) programming practice. Feel free to make as many comments as you like!
(I'm aware that I could add more comments)
Blackjack.cpp
```
#include "stdafx.h"
#include "Blackjack.h"
Blackjack::Blackjack()
{
srand(time(0));
dhandSize = 0;
phandSize = 0;
dhandSum = 0;
phandSum = 0;
playerDone = false;
dealerDone = false;
}
void Blackjack::playGame()
{
cout > phit;
if (phit == 1)
{
addPlayerCard();
printHand();
sumHands();
if (phandSum > 21)
{
cout > pstand;
}
if (pstand == 1)
{
playerDone = true;
}
if (dhandSum 21)
{
cout = 17)
{
dealerDone = true;
}
if (phandSum == 21 && dhandSum == 21)
{
cout phandSum)
{
cout << "Sum of your hand is lower than the dealer's sum of " << dhandSum << ". You lose!";
return;
}
}
}
}
int dhand[5];
int phand[5];
int dhandSize;
int phandSize;
int dhandSum;
int phandSum;
int phit;
int pstand;
bool playerDone;
bool dealerDone;
void Blackjack::addPlayerCard()
{
if (phandSize <= 5)
{
phand[phandSize] = 1 + (rand() % 11);
phandSize++;
}
else
{
cout << "Sorry. You have reached the maximum number of cards (5)." << endl;
playerDone = true;
}
}
void Blackjack::addDealerCard()
{
if (dhandSize <= 5
(I'm aware that I could add more comments)
Blackjack.cpp
```
#include "stdafx.h"
#include "Blackjack.h"
Blackjack::Blackjack()
{
srand(time(0));
dhandSize = 0;
phandSize = 0;
dhandSum = 0;
phandSum = 0;
playerDone = false;
dealerDone = false;
}
void Blackjack::playGame()
{
cout > phit;
if (phit == 1)
{
addPlayerCard();
printHand();
sumHands();
if (phandSum > 21)
{
cout > pstand;
}
if (pstand == 1)
{
playerDone = true;
}
if (dhandSum 21)
{
cout = 17)
{
dealerDone = true;
}
if (phandSum == 21 && dhandSum == 21)
{
cout phandSum)
{
cout << "Sum of your hand is lower than the dealer's sum of " << dhandSum << ". You lose!";
return;
}
}
}
}
int dhand[5];
int phand[5];
int dhandSize;
int phandSize;
int dhandSum;
int phandSum;
int phit;
int pstand;
bool playerDone;
bool dealerDone;
void Blackjack::addPlayerCard()
{
if (phandSize <= 5)
{
phand[phandSize] = 1 + (rand() % 11);
phandSize++;
}
else
{
cout << "Sorry. You have reached the maximum number of cards (5)." << endl;
playerDone = true;
}
}
void Blackjack::addDealerCard()
{
if (dhandSize <= 5
Solution
Members Vs Globals
It's odd that you have global variables defined in your Blackjack.cpp file. These:
Look like they should have been declared as class members (which now that you have added the header I can see that they are in there as well), rather than globals. The globals should be removed, they're just going to cause confusion.
What's in a deck
When you're dealing cards, you're deciding what card to add using a random generator.
This is ok as a start, however it's possible that the player could end up with 5 aces etc. Is that really what you want? With a pack of cards, there are many cards that have a value of ten (10,Jack,Queen,King) yet your current random approach thinks all card values are as likely. In reality, the chances of you getting each card decrease as cards of that value are removed from the deck. Consider adding a deck class that you initialise with 1 or more packs of shuffled cards when constructed then remove from the deck as each card is drawn. This would make your draws more realistic and allow you to reuse the deck class in any future card games you may construct (such as poker).
Duplicated Code - Player Vs Dealer
Dealers and players are almost the same. You're performing the same calculations in
Then you have a
The next step being to look at the Player abstraction to see if some of the functionality could be pushed from your BlackJack class into it. For example it could have a method to CalculateScore, based on the cards it is holding.
Adding this type of generalisation would also make it easier for you to extend your program so that it could for example support multiple players against the dealer.
Dealer Bias
After you deal your initial cards, you check if the dealer or player have Black Jack and declare them the winner if they do. If both players have 21, then the dealer is declared the winner. This is a good example of when extracting the functionality into a shared method would have helped you out. The checks you need to do are the same checks that are performed at the end of every round, except the end of round check supports a draw. If you extract the functionality into another method can call it from both places the game will become that little bit fairer for the players.
Constants are your friends
There are several places in your code that could benefit from the removal of 'magic' numbers. Replacing them with constants could help the readability of your code and reduce the chance of bugs. You might want to have a constant for 21 (possibly BlackJack), particularly since you have so many occurrences of it.
A constant for MaxScoreForDealerToHit of 17 would help to make your code self-documenting, the <17 becomes much more readable as:
However, the main constants I would introduce are for Yes(1) and No(2). You ask the user some yes no questions and have the same values for the answer, so using a constant would really aid translation. Consider this (from your main loop):
It becomes much more obvious that your
Letters for variable names don't cost the Earth
In context, you can figure out what
It's odd that you have global variables defined in your Blackjack.cpp file. These:
int dhand[5];
int phand[5];
int dhandSize;
int phandSize;
int dhandSum;
int phandSum;
int phit;
int pstand;
bool playerDone;
bool dealerDone;Look like they should have been declared as class members (which now that you have added the header I can see that they are in there as well), rather than globals. The globals should be removed, they're just going to cause confusion.
What's in a deck
When you're dealing cards, you're deciding what card to add using a random generator.
phand[phandSize] = 1 + (rand() % 11);This is ok as a start, however it's possible that the player could end up with 5 aces etc. Is that really what you want? With a pack of cards, there are many cards that have a value of ten (10,Jack,Queen,King) yet your current random approach thinks all card values are as likely. In reality, the chances of you getting each card decrease as cards of that value are removed from the deck. Consider adding a deck class that you initialise with 1 or more packs of shuffled cards when constructed then remove from the deck as each card is drawn. This would make your draws more realistic and allow you to reuse the deck class in any future card games you may construct (such as poker).
Duplicated Code - Player Vs Dealer
Dealers and players are almost the same. You're performing the same calculations in
AddPlayerCard that you are performing in AddDealerCard. This could be generalised if you were for example to add the concept of a Player (dealers are players to, they're just automated) to your class. You could do something as simple as this:struct Player {
int dhand[5];
int dhandSize;
bool done;
};Then you have a
addCardFunction that looked more like this:void Blackjack::addDealerCard(Player &player)
{
if (player.dhandSize <= 5)
{
player.dhand[dhandSize] = 1 + (rand() % 11);
player.dhandSize++;
}
else
{
player.done = true;
}
}The next step being to look at the Player abstraction to see if some of the functionality could be pushed from your BlackJack class into it. For example it could have a method to CalculateScore, based on the cards it is holding.
Adding this type of generalisation would also make it easier for you to extend your program so that it could for example support multiple players against the dealer.
Dealer Bias
After you deal your initial cards, you check if the dealer or player have Black Jack and declare them the winner if they do. If both players have 21, then the dealer is declared the winner. This is a good example of when extracting the functionality into a shared method would have helped you out. The checks you need to do are the same checks that are performed at the end of every round, except the end of round check supports a draw. If you extract the functionality into another method can call it from both places the game will become that little bit fairer for the players.
Constants are your friends
There are several places in your code that could benefit from the removal of 'magic' numbers. Replacing them with constants could help the readability of your code and reduce the chance of bugs. You might want to have a constant for 21 (possibly BlackJack), particularly since you have so many occurrences of it.
A constant for MaxScoreForDealerToHit of 17 would help to make your code self-documenting, the <17 becomes much more readable as:
if (dhandSum < MaxScoreForDealerToHit && dealerDone != true)However, the main constants I would introduce are for Yes(1) and No(2). You ask the user some yes no questions and have the same values for the answer, so using a constant would really aid translation. Consider this (from your main loop):
int exitGame = Yes;
do
{
Blackjack casino_royale;
casino_royale.playGame();
cout > exitGame;
}while (exitGame == Yes);It becomes much more obvious that your
exitGame variable should be called playAgain.Letters for variable names don't cost the Earth
In context, you can figure out what
dhand, dhandSize, dhandSum mean. However, they would be much more expressive if you pad them out with some missing letters: DealerHand, NumberOfCardsDealerHas, DealersCardTotal.Code Snippets
int dhand[5];
int phand[5];
int dhandSize;
int phandSize;
int dhandSum;
int phandSum;
int phit;
int pstand;
bool playerDone;
bool dealerDone;phand[phandSize] = 1 + (rand() % 11);struct Player {
int dhand[5];
int dhandSize;
bool done;
};void Blackjack::addDealerCard(Player &player)
{
if (player.dhandSize <= 5)
{
player.dhand[dhandSize] = 1 + (rand() % 11);
player.dhandSize++;
}
else
{
player.done = true;
}
}if (dhandSum < MaxScoreForDealerToHit && dealerDone != true)Context
StackExchange Code Review Q#133489, answer score: 15
Revisions (0)
No revisions yet.