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;
};

• dbasnett: The shooter loses his turn if a 7 is rolled after there is a point.

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).

• Corbin: Good points, but two nit picks: The parameters to main have nothing to do with if you're compiling from the command line, but rather how you're running (creating) it. And for if using C++11, use nullptr instead... You should also mention that if you're using C++11, run the hell away from srand()/rand(). rand() has terrible range, is a pain in the ass to clamp to a range without bias, and a 32 bit seed limit is rather meh (half of std::time() gets truncated for example). Doesn't matter for a basic card game program, but CR brings out my inner pedant :).
• Jamal: @Corbin: Good points. I'll put those in with my current edits.
• Jamal: @Corbin: Also, I had no idea about std::srand() in this case. I did happen to come across std::uniform_int_distribution not too long ago. Would this be a valid solution for here?
• Corbin: Yeah. You would use a std::uniform_int_distribution<int>(1, 6) to achieve the same functionality as what he's done. A full (conviently dice oriented) example is here: en.cppreference.com/w/cpp/numeric/random/…
• Jamal: @Corbin: Awesome! It appears to work with my old compiler, too. Looks like I can toss out rand() from my own stuff.

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.