patterncppMinor
Multiplayer BlackJack using Vector
Viewed 0 times
multiplayervectorusingblackjack
Problem
This is a small program I wrote for GCSE computing, it operates using Vectors and that's pretty much it, wondering if you could tell me if there is anything I could do better.
BlackJackv2.cpp
```
#include "BlackJack v2.h"
int main() {
// Create Vector with the players, using vec because of an undiefined amount of players
std::vector VecPlayers;
// StrLine for entering players
std::string StrLine = "";
// Used as an index for VecPlayers
unsigned int CountPlayer = 0;
// Give players that are playing then add them to the vector
std::cout > StrLine) {
if (StrLine == ".") break;
VecPlayers.push_back(Players());
VecPlayers[CountPlayer].Name = StrLine;
CountPlayer++;
std::cout > Answer;
// If they want another card
while (Answer == 'Y') {
// Give the player one card
VecPlayers[CountPlayer] = AddCard(VecPlayers[CountPlayer], 1);
// Show all the playerscards
DisplayCard(VecPlayers[CountPlayer]);
// Add the cards toghether
VecPlayers[CountPlayer] = SumCard(VecPlayers[CountPlayer]);
// If your cards sum is less than or equal to 21
if (VecPlayers[CountPlayer].Sum > Answer;
// Capitalise all input so no need for or operator
Answer = toupper(Answer);
}
// If they have too many cards
else {
std::cout GreatestSum && VecPlayers[Count].Sum < 22) {
// Assign the Sum of the current player to GreatestSum
GreatestSum = VecPlayers[Count].Sum;
// Assign the Index of the current player to WinnersIndex
WinnersIndex = Count;
}
}
// Print out the winners name and sum of his cards
std::cout << std::string(100, '\n') << "The winner is " << VecPlayers[WinnersIndex].Name
<< "
BlackJackv2.cpp
```
#include "BlackJack v2.h"
int main() {
// Create Vector with the players, using vec because of an undiefined amount of players
std::vector VecPlayers;
// StrLine for entering players
std::string StrLine = "";
// Used as an index for VecPlayers
unsigned int CountPlayer = 0;
// Give players that are playing then add them to the vector
std::cout > StrLine) {
if (StrLine == ".") break;
VecPlayers.push_back(Players());
VecPlayers[CountPlayer].Name = StrLine;
CountPlayer++;
std::cout > Answer;
// If they want another card
while (Answer == 'Y') {
// Give the player one card
VecPlayers[CountPlayer] = AddCard(VecPlayers[CountPlayer], 1);
// Show all the playerscards
DisplayCard(VecPlayers[CountPlayer]);
// Add the cards toghether
VecPlayers[CountPlayer] = SumCard(VecPlayers[CountPlayer]);
// If your cards sum is less than or equal to 21
if (VecPlayers[CountPlayer].Sum > Answer;
// Capitalise all input so no need for or operator
Answer = toupper(Answer);
}
// If they have too many cards
else {
std::cout GreatestSum && VecPlayers[Count].Sum < 22) {
// Assign the Sum of the current player to GreatestSum
GreatestSum = VecPlayers[Count].Sum;
// Assign the Index of the current player to WinnersIndex
WinnersIndex = Count;
}
}
// Print out the winners name and sum of his cards
std::cout << std::string(100, '\n') << "The winner is " << VecPlayers[WinnersIndex].Name
<< "
Solution
You should write self documenting code.
This basically means breaking your code up into logical well named units. Units are functions and classes. Classes should represents objects in your system.
I would have layed out main something like this:
Couple of classes I would build:
Seeding Random Number
You should seed the random number generator only once during an application run. By seeding it multiple times you ruin the distribution. So move this out of the loop and put it just after main starts.
Random Number generation.
This is probably the anti pattern for getting a rand number (and it has a bug). This is because the space is for random numbers is not even so the chance for any particular number is not the same.
The
This means.
The Bug:
The better way to write this is:
Also
http://en.cppreference.com/w/cpp/numeric/random
Generating a pack of cards
The problem here is that you are likely to get a couple of cards that are the same (thus stacking your deck slightly).
What you should do is generate all the cards you expect to find in the deck then shuffle the deck to get a random order.
This basically means breaking your code up into logical well named units. Units are functions and classes. Classes should represents objects in your system.
I would have layed out main something like this:
int main()
{
std::vector players = getPlayers();
Shoe cards(6); // initialize with 6 decks.
for(;;) {
Game game(players, cards);
game.dealCards();
for(player: players) {
game.getExtraCards(player);
}
game.getExtraCards(); // for the dealer.
game.payWinners();
}
}Couple of classes I would build:
Game: A class to hold state about the current game.
Player: A class to hold state about players
Cards: A class to hold the different types of cards.
Mainly so they are easy to print.
Shoe: A class to hold all the cards from several decks of cards.
So you can deal from the deck. Then shuffle when you have
used all the cards.Seeding Random Number
srand(int(time(NULL)));You should seed the random number generator only once during an application run. By seeding it multiple times you ruin the distribution. So move this out of the loop and put it just after main starts.
Random Number generation.
int RandomNumber(int Start, int End) {
int RNG = (rand() % End) + Start;
return RNG;
}This is probably the anti pattern for getting a rand number (and it has a bug). This is because the space is for random numbers is not even so the chance for any particular number is not the same.
The
rand() function generates a number in the range 0-RANDMAX. For arguments sake lets say RANDMAX 32768. Your want a number in the range 1-6.This means.
1: Probability: 5462/32768 // Notice this has a slightly higher probability
2: Probability: 5462/32768 // Notice this has a slightly higher probability
3: Probability: 5461/32768
4: Probability: 5461/32768
5: Probability: 5461/32768
6: Probability: 5461/32768The Bug:
Start = 8;
End = 10;
int RNG = (rand() % End) + Start; // Number in the range. 8 -> 17The better way to write this is:
int RandomNumber(int start, int end) {
int rng = end - start + 1;
int max = RANDMAX / rng * rng; // Integer arithmetic so will be
// less than RANDMAX if it does
// not divide exactly.
int rnd;
// Get a value from [0, max)
// Discards numbers outside the range.
// So that each number has an equal probability.
do {
rnd = rand();
}
while(rnd >= max);
// Generate the result.
return (rnd % rng) + start;
}Also
rand() is old and is known to be pretty bad. You should be using the new random number generator that was provided in C++11 to solve this specific problem.http://en.cppreference.com/w/cpp/numeric/random
Generating a pack of cards
Players AddCard(Players Player, unsigned int NumberCards) {
for (unsigned int Count = 0; Count < NumberCards; Count++) {
Player.IndexSuit[Player.Cards] = RandomNumber(0, 3);
Player.IndexCard[Player.Cards] = RandomNumber(0, 12);
Player.Cards++;
}
return Player;
}The problem here is that you are likely to get a couple of cards that are the same (thus stacking your deck slightly).
What you should do is generate all the cards you expect to find in the deck then shuffle the deck to get a random order.
class Shoe
{
std::vector cards;
std::vector::iterator nextCard;
std::random_device rd;
std::mt19937 randomGenerator;
public:
Shoe(int numberOfDecks)
: randomGenerator(rd)
{
// Create all the cards.
for(int deck = 0;deck < numberOfDecks; ++deck) {
for(int card=0; card < 52; ++card) {
cards.push_back(Card(card));
}
}
shuffle();
}
void shuffle()
{
std::shuffle(std::begin(cards), std::end(cards), randomGenerator);
nextCard = std::begin(cards);
}
Card deal()
{
if (nextCard == std::end(cards)) {
shuffle();
}
return *nextCard++
}
}Code Snippets
int main()
{
std::vector<Player> players = getPlayers();
Shoe cards(6); // initialize with 6 decks.
for(;;) {
Game game(players, cards);
game.dealCards();
for(player: players) {
game.getExtraCards(player);
}
game.getExtraCards(); // for the dealer.
game.payWinners();
}
}Game: A class to hold state about the current game.
Player: A class to hold state about players
Cards: A class to hold the different types of cards.
Mainly so they are easy to print.
Shoe: A class to hold all the cards from several decks of cards.
So you can deal from the deck. Then shuffle when you have
used all the cards.srand(int(time(NULL)));int RandomNumber(int Start, int End) {
int RNG = (rand() % End) + Start;
return RNG;
}1: Probability: 5462/32768 // Notice this has a slightly higher probability
2: Probability: 5462/32768 // Notice this has a slightly higher probability
3: Probability: 5461/32768
4: Probability: 5461/32768
5: Probability: 5461/32768
6: Probability: 5461/32768Context
StackExchange Code Review Q#113210, answer score: 4
Revisions (0)
No revisions yet.