patterncppMinor
Roulette game in C++
Viewed 0 times
gameroulettestackoverflow
Problem
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
Source code
Here is some of the code:
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 > betNumber;
//Checks if number is valid (between 1 and 36)
while (betNumber 36) {
cout > betNumber;
}
//Asks for amount to bet on that number
cout > betAmount;
//Checks if minimum amount is $1 and if the player has enough money in their account
while (betAmount bankAccount) {
cout > 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;Solution
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
When you later check your bounds with
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,
Avoid Obfuscation
In the code commented
with
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:
Find some other code that doesn't change much and extract that into functions as well. For example, the code commented
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
You can also avoid comments by naming constants. Instead of star
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 11) {, you can instead use a calculated size (_countof(betType) or sizeof(betType)/sizeof(betType[0])), or even use a std::vector 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 bankAccount)
{
cout > 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 star
Code Snippets
cout << endl << " " << (char)36 << (char)36 << (char)36 << (char)36 << (char)36;cout << endl << " $$$$$"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);
: : :
}case 1:
: : :
bankAccout = awardWinnings(betNumber == randomNumber, betAmount, betOdds, bankAccount);
break;
case 2:
: : :
bankAccount = awardWinnings(betNumber == randomNumber || betNumber + 3 == randomNumber, betAmount, betOdds, bankAccount);
break;Context
StackExchange Code Review Q#35248, answer score: 5
Revisions (0)
No revisions yet.