Roulette game in C++

Question

I'm kind of new to C++ and was just wondering if anyone could give me tips on how I could make my code more efficient (I added some comments to the code to help you understand it better).

Is it a good idea to use switch cases? Is there a way I can use more functions/arrays/pointers?

Source code

Here is some of the code:

case 1:
 //Asks for number to place bet on
 cout << endl << "Please choose a number to place your bet on: ";
 cin >> betNumber;
 //Checks if number is valid (between 1 and 36)
 while (betNumber < 1 || betNumber > 36) {
 cout << endl << "You must choose a valid number between 1 and 36, inclusive!" << endl;
 cout << "Please choose a number to place your bet on: ";
 cin >> betNumber;
 }
 //Asks for amount to bet on that number
 cout << endl << "How much would you like to bet on the number " << betNumber << "? $";
 cin >> betAmount;
 //Checks if minimum amount is 1ドル and if the player has enough money in their account
 while (betAmount < 1 || betAmount > bankAccount) {
 cout << endl << "You have $" << bankAccount << " in your bank account: $";
 cin >> betAmount;
 }
 //Seeds random number to the generator
 srand(time(0));
 //Generates a random number
 randomNumber = 1 + (rand() % 36);
 cout << endl << "The ball landed on the number " << randomNumber << ".";
 //Checks if player won or lost their bet
 if (betNumber == randomNumber) {
 bankAccount = win(betAmount, betOdds, bankAccount);
 cout << endl << "Congratulations, you won! You now have $" << bankAccount << " in your account." << endl;
 } else {
 bankAccount = lose(betAmount, bankAccount);
 cout << endl << "Bad luck, you lost! You now have $" << bankAccount << " in your account." << endl;
 }
 break;

• Jamal: Right off the bat, srand() should go in main(). If you call it repeatedly (such as where you have it now), you'll receive the same random values each time.
• HazElMagic: Thanks for that, Jamal! Doesn't the time(0) ensure that it is random, though?
• Jamal: It just sets the seed to 0, which is how it's usually done. The problem is with multiple calls to std::srand(), which will keep setting it to zero. You don't want that. You want to let std::rand() give a "true" random value each time, which is also why you use std::time().
• Jamal: You could also use NULL instead of 0, preferably nullptr if you're using C++11.
• Jamal: As this was provided by a comment, you may make those changes in your original code. For future reference, do not make any changes you receive from answers, as that will invalidate them.

Answer 1

Since there aren't any loops in your code that aren't waiting for the user to provide some input, it's probably way premature to talk about efficiency. Is your code measurably slow? Does it leave you hanging? If not, unless you have an unusual need, save questions on efficiency for later.

So let's talk about some more important things: readability and maintainability. Here are some tips on ways to improve the readability and maintainability of your code. These aren't hard rules that you should never break (especially the one on comments); they are guidelines on ways to make your life easier that you should learn to bend when the guideline makes things awkward instead.

I hope this helps, even though it may be a lot to take in all at once. Feel free to ask follow-up questions and get other opinions.

Avoid Redundancy

Redundancy can show up in many forms. One example of it is in your declaration of arrays, using code like int betType[11] = {35, 17, 8, 11, 5, 2, 1, 1, 2, 1, 1};. Unless there's something very special about the number 11, there's no reason to call it out. Instead just say int betType[] = {35, 17, 8, 11, 5, 2, 1, 1, 2, 1, 1}; which will automatically determine the size of the array for you.

When you later check your bounds with while (betChoice < 1 || betChoice > 11) {, you can instead use a calculated size (_countof(betType) or sizeof(betType)/sizeof(betType[0])), or even use a std::vector<int> instead of an int[], and check against the vector's size().

This will help you avoid magic numbers that don't mean much later. After all, if someone asks you what's special about 11, would you think it's the number of available bet types? But if they asked what betTypes.size() meant, it would be easy to answer.

Another way redundancy shows up is in large blocks of repeating code. For instance, case 1 and case 2 have almost the same code. In fact I had to read it a couple times to find the part that was different. Sometimes this can be best handled by refactoring similar parts of code into functions, and passing parameters to them that control how they differ. Sometimes it's better just to extract the parts that are identical into simpler functions, and use them. I'll touch on this more below, but I certainly don't have the answer.

Avoid Obfuscation

In the code commented Displays a large dollar sign, there are a lot of casts from int to char so that cout prints the value as a character. But the characters in question are not that unusual. Just use the actual character you want to show, for example replacing

cout << endl << " " << (char)36 << (char)36 << (char)36 << (char)36 << (char)36;

with

cout << endl << " $$$$$"

This will not only be easier to type or update, it will be easier to read.

Avoid Comments

This recommendation is somewhat controversial, but it begins to target your question about functions. Instead of commenting what a line of code does, comment how a block of code does something unusual. When you're first starting out, everything seems unusual, but eventually you will see patterns and only need to comment on things that are not common patterns.

But then, instead of commenting what a block of code does, give it a name instead by putting it in a function. For example, you have several cases where you ask how much the user wants to bet on a number, then loop until they enter a valid number. You could extract this loop into a helper function like this:

int getBetAmount(int bankAccount)
{
 int betAmount;
 cin >> betAmount;
 while (betAmount < 1 || betAmount > bankAccount)
 {
 cout << endl << "You have $" << bankAccount << " in your bank account: $";
 cin >> betAmount;
 }
 return betAmount;
}
int _tmain() {
 : : :
 case 1:
 : : :
 cout << endl << "How much would you like to bet on the number" << betNumber << "? $";
 betAmount = getBetAmount(bankAccount);
 : : :
 : : :
 case 2:
 : : :
 cout << endl << "How much would you like to bet on the numbers" << betNumber << " and " << betNumber + 3 << "? $";
 betAmount = getBetAmount(bankAccount);
 : : :
}

Find some other code that doesn't change much and extract that into functions as well. For example, the code commented Checks if player won or lost their bet, I see creating a function you'd call like this:

case 1:
 : : :
 bankAccout = awardWinnings(betNumber == randomNumber, betAmount, betOdds, bankAccount);
 break;
case 2:
 : : :
 bankAccount = awardWinnings(betNumber == randomNumber || betNumber + 3 == randomNumber, betAmount, betOdds, bankAccount);
 break;

After you make these changes, ideally the parts that are different will start to stand out, and the parts that are the same will have good names that tell you what they do even if they don't have a comment. And then you can more easily avoid incorrect comments like case 2's Check if number is valid (between 1 and 36) that actually checks for 33.

You can also avoid comments by naming constants. Instead of starting with int bankAccount = 500 and then 500 lines later referencing 500 to figure out your overall winnings, perhaps declare const int StartingBankAccount = 500; and use the name instead of the number in both places. If you decide to change the initial account wealth, this also helps ensure your ending summary remains correct.

Avoid Bad Dice

While this is a toy program, and a person is unlikely to play long enough for it to matter, rand() % max is a flawed approach to generating random numbers. It's flawed in ways too subtle for me to explain (I understand it, but not well enough to explain it). However Stephan T. Lavavej knows it much better and explains it in a video called rand() Considered Harmful; watch it and use the approach he recommends if you want a more uniformly distributed random number.

• HazElMagic: Thank you so much for all of those amazing tips! I extremely appreciate all of your time and effort! Here is my current code: codepad.org/UVgOUz7u
• HazElMagic: I also created a function just then to reduce the code even more! Thanks a lot, Michael!
• Michael Urman: You're welcome! I see you've already started to apply the advice. Always be on the look out for more code you can factor into existing or new functions; for example after each call to getBetAmount you then call pause; should that then also be part of getBetAmount?

Answer 2

I have found a couple of things that could help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. I don't know that you've actually done that, but it's an alarmingly common thing for new C++ programmers to do.

An alternative could be to do the following, preferably within the calling function.

using std::cout;

Separate I/O from game logic

The program currently has game logic and I/O all intermixed. It would be better to separate the two. Specifically I'd recommend that the player, each bet and the roulette wheel each be made separate objects. That way, if you decided to have multiple players with multiple types of bets at the same wheel, it could be done very simply by applying the state of the wheel to each bet and then updating the player's status (account balance) accordingly.

Eliminate "magic numbers"

There are a few numbers in the code, such as 1 and 36 that have a specific meaning in their particular context. By using named constants such as MINIMUM_BET_AMOUNT or WHEEL_SLOTS, the program becomes easier to read and maintain. For cases in which the constant only has sense with respect to a particular object, consider making that constant part of the object.

Reduce the scope of variables

Within the current code essentially all variables are all in the same scope. Better would be to minimize the scope of variables so that a variable such as betAmount are only in scope for the duration they're actually needed. Object orientation will help a great deal with that.

Add realism to the game

Most real roulette tables have at least one zero slot (many have both 0 and 00). Adding those to the game would make things a little more realistic. I've also seen video screens in casinos which display the last dozen or so results. On a fair wheel this, of course, has no useful value to bettors, but people often have fun looking at the previous results and making some bet based on that, thereby illustrating why they call it the Gambler's fallacy.

Use a menu object or at least a common menu function

In a number of places in your code, you have something like a menu. Your code presents a couple of options and then asks the user to pick one based on an input number. Rather than repeating that code in many places, it would make sense to make it generic. Only the prompt strings actually change, but the underlying logic of presenting the choices and asking for input are all the same. It looks like you're a beginning programmer, and so perhaps you haven't learned about objects yet, but this kind of repeated task with associated data is really well-suited to object-oriented programming and that's something that C++ is very good at expressing.

Consider using a better random number generator

You are currently using this code to generate random numbers:

srand(time(0));
randomNumber = 1 + (rand() % 36);

There are two problems with this approach. One is that the low order bits of the random number generator are not particularly random, so neither with random1 be. On my machine, there's a slight but measurable bias toward 0 with that. The second problem is that you should only seed the random number generator once and not every time it's used.

A better solution, if your compiler and library supports it, would be to use the C++11 `std::uniform_int_distribution. It looks complex, but it's actually pretty easy to use:

// random number generator from Stroustrup: 
// http://www.stroustrup.com/C++11FAQ.html#std-random
int rand_int(int low, int high)
{
 static std::default_random_engine re {};
 using Dist = std::uniform_int_distribution<int>;
 static Dist uid {};
 return uid(re, Dist::param_type{low,high});
}

For a roulette table with possible values 0 to 36, call it with rand_int(0,36);.