Craps game rules and code

Question

I'm developing a small craps game in C++, and my C++ skills are a bit rusty. I really need someone to review my code to ensure that I have correctly implemented the game according to the rules.

Game rules:

1. The player or shooter rolls a pair of standard dice
   1. If the sum is 7 or 11 the game is won
   2. If the sum is 2, 3 or 12 the game is lost
   3. If the sum is any other value, this value is called the shooter’s point and he continues rolling until he rolls a 7 and loses or he rolls the point again in which case he wins
2. If a game is won the shooter plays another game and continues playing until he loses a game, at which time the next player around the Craps table becomes the shooter

My code:

#include <iostream>
#include <ctime>

using namespace std;

bool checkWinning(int roll);

int main(int argc, const char * argv[])
{
    //create player aka shooter

    //create pair of dice
    unsigned int dice1=0;
    unsigned int  dice2 = 0;

    //create roll
    unsigned int roll = 0;

    //create game loop
    while(checkWinning(roll) == true)
    {
        dice1 =  rand() % 6 + 1;
        dice2  = rand() % 6 + 1;
        roll = dice1 + dice2;

        cout<< dice1<<" +"<< dice2<<" = "<<  roll << endl;
        //cout<< checkWinning(2) <<endl;
    }

    return 0;
}

bool checkWinning(int roll)
{
    bool winner  = true;
    if( roll == 2 || roll == 3 || roll == 12)
        return winner= false;
    else
        if(roll == 7 || roll == 11 )
        return winner;
    else
        return winner;
};

Answer 1

• Try not to use using namespace std.

• The parameters in main() are only necessary if you're executing from the command line.

• You're not calling std::srand() nor including <cstdlib>. However, if you're using C++11, both std::srand and std::rand are not recommended due to certain computational complications (the C++11 pseudo-random number generators can be found under <random>). But, for a simple program, it may not matter.

  In general, here's how to call std::srand():

  // casting may just be necessary if warnings are generated
  //     that will alert you if there's a possible loss of data
  // prefer nullptr to NULL if using C++11
  std::srand(static_cast<unsigned int>(std::time(NULL)));
  

  Only include this once, preferably at the top of main(). This is preferred because

  1. It'll help you keep track of it, especially if it'll need to be removed at some point.
  2. If called repeatedly, you'll receive the "same random number" each time.

• It's best to keep variables as close in scope as possible. Here, dice1 and dice2 can be initialized in the while-loop:

  unsigned int dice1 = rand() % 6 + 1;
  unsigned int dice2 = rand() % 6 + 1;
  

  roll, however, will need to stay where it is so that the loop will work.

• The bool-checking can be shortened:

  // these are similar
  while (checkWinning(roll) == true)
  while (checkWinning(roll))
  

  // these are also similar
  while (checkWinning(roll) == false)
  while (!checkWinning(roll))
  

• checkWinning() takes int roll, but roll is already unsigned int. They should match.

• checkWinning()'s closing curly brace shouldn't have a ;. It's not a type.

• bool winner seems redundant; just return true or false. Also, the conditions seem a little unclear. If the sum constitutes a win or a re-roll, how do you specifically distinguish the two? They both return true. I'd at least rename the function for clarification. There's also a Boolean enum, but that may be overkill here (or even unnecessary as there are only two ending outcomes).

• There should be a final outcome message, indicating a win or a loss. Also, you're not giving the player the option to play another game if victorious (and until loss).

Answer 2

Using % with rand is wrong.

Assuming your RAND_MAX is the same as mine 2147483647 then the probabilities for each number are:

dice1 =  rand() % 6 + 1;

1:  357913942/2147483647   Notice a slightly higher probability for a 1.
2:  357913941/2147483647
3:  357913941/2147483647
4:  357913941/2147483647
5:  357913941/2147483647
6:  357913941/2147483647

The solution use C++11 random functionality.
Correct for the skew in C++03's rand()

Unfortunately I can't find a correct answer on SO for using rand().

int dieRoll() // return a number between 1 and 6
{
    static int maxRange = RAND_MAX / 6 * 6; // note static so calculated once.

    int result;
    do
    {
        result = rand();
    }
    while(result > maxRange);    // Anything outside the range will skew the result
    return result % 6 + 1;       // So throw away the answer and try again.
}

Note:

int result = rand() * 1.0 / range; // does not help with distribution

Answer 3

In addition to what @Jamal said…

The singular for "dice" is "die", so name your variables accordingly. It's C++, so Die deserves to be a class, with a Die.roll() method. The constructor could call std::srand().

You should use a do-while loop. Then you could avoid having to artificially initialize all of your values to illegal 0 values.

Your checkWinning() function could just be a switch statement. It doesn't need a winner variable, and can just return the result immediately. I see where you implemented rules 1.1 and 1.2, but I don't see any of your other game rules expressed anywhere in your code.