HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Multiplayer BlackJack using Vector

Submitted by: @import:stackexchange-codereview··
0
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
<< "

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:

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/32768


The Bug:

Start = 8;
 End   = 10;
 int RNG = (rand() % End) + Start;  // Number in the range.  8 -> 17


The 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/32768

Context

StackExchange Code Review Q#113210, answer score: 4

Revisions (0)

No revisions yet.