patterncModerate
Fruitmachine game
Viewed 0 times
fruitmachinegamestackoverflow
Problem
I'm quite happy with my code so far but I often make efficiency errors that I don't see when writing code. Have I missed anything?
```
#include
#include
int main ()
{
char end = 'N';
int credit = 5, bet = 0, userInput = 0, cont = 0, victory;
do{
while (cont == 0){
// Introduction of credit system to user
printf("\e[1;1H\e[2J");
printf("Your current credits are %d.\n", credit);
printf("How many credits would you like to bet?\n");
scanf("%d", &userInput);
printf("\e[1;1H\e[2J");
// Checking for valid credit input and adjusting credit/bet value according to input
if (userInput = 1){
bet = userInput;
credit = credit - bet;
cont = 1;
printf("You have bet %d credits. Good luck!\n", bet);
sleep(2);
break;
}
if (userInput == 0){
printf("Invalid, bet must be a minimum of 1 credit.\n");
sleep(2);
cont = 0;
}
else{
printf("Invalid, input exceeded your credit value of %d.\n", credit);
sleep(2);
cont = 0;
}
// End of check
}
// Introduction countdown to execute after valid bet is placed
printf("\e[1;1H\e[2J");
printf("Ready?\n");
sleep(1);
printf("\e[1;1H\e[2J");
printf("3...\n");
sleep(1);
printf("\e[1;1H\e[2J");
printf("2...\n");
sleep(1);
printf("\e[1;1H\e[2J");
printf("1...\n");
sleep(1);
printf("\e[1;1H\e[2J");
printf("Spin!\n");
printf("\e[1;H\e[2J");
// Declaring var names for reel and loop
int firstReel, secondReel, thirdReel, loop = 0;
// Loop to print cycle reel 10 times
while(loop 10 && firstReel == secondReel || secondReel == thirdReel){
printf("Congratulations! You win! ");
victory = 1;
}
else
{
printf("Sorry, you lose. ");
victory = 0;
}
// Payout calculations according to victory value
if (victory == 1){
credit = credit + (bet * 2);
printf("Your credit is now %d.\n", credit);
}
else
{
if (credit == 0){
printf("Your credit is now 0. Ga
```
#include
#include
int main ()
{
char end = 'N';
int credit = 5, bet = 0, userInput = 0, cont = 0, victory;
do{
while (cont == 0){
// Introduction of credit system to user
printf("\e[1;1H\e[2J");
printf("Your current credits are %d.\n", credit);
printf("How many credits would you like to bet?\n");
scanf("%d", &userInput);
printf("\e[1;1H\e[2J");
// Checking for valid credit input and adjusting credit/bet value according to input
if (userInput = 1){
bet = userInput;
credit = credit - bet;
cont = 1;
printf("You have bet %d credits. Good luck!\n", bet);
sleep(2);
break;
}
if (userInput == 0){
printf("Invalid, bet must be a minimum of 1 credit.\n");
sleep(2);
cont = 0;
}
else{
printf("Invalid, input exceeded your credit value of %d.\n", credit);
sleep(2);
cont = 0;
}
// End of check
}
// Introduction countdown to execute after valid bet is placed
printf("\e[1;1H\e[2J");
printf("Ready?\n");
sleep(1);
printf("\e[1;1H\e[2J");
printf("3...\n");
sleep(1);
printf("\e[1;1H\e[2J");
printf("2...\n");
sleep(1);
printf("\e[1;1H\e[2J");
printf("1...\n");
sleep(1);
printf("\e[1;1H\e[2J");
printf("Spin!\n");
printf("\e[1;H\e[2J");
// Declaring var names for reel and loop
int firstReel, secondReel, thirdReel, loop = 0;
// Loop to print cycle reel 10 times
while(loop 10 && firstReel == secondReel || secondReel == thirdReel){
printf("Congratulations! You win! ");
victory = 1;
}
else
{
printf("Sorry, you lose. ");
victory = 0;
}
// Payout calculations according to victory value
if (victory == 1){
credit = credit + (bet * 2);
printf("Your credit is now %d.\n", credit);
}
else
{
if (credit == 0){
printf("Your credit is now 0. Ga
Solution
I see some things that may help you improve your code.
Don't use non-ISO escape sequences
I assume from the context that you are intending
Isolate ANSI escape sequences into functions
It's often good practice to split out your escape sequences into a separate function. It's easier to read and understand
Use all the required
The code calls
Eliminate needless repetition
The switch statements that print the names of the icons could be much simplified by adding this at file scope.
And then replacing all three switch statements with this one line:
Don't reseed the random number generator more than once
The program currently calls
Separate input, output and calculation
To the degree practical it's usually good practice to separate input, output and calculation for programs like this. By putting them in separate functions, it isolates the particular I/O for your platform (which is likely to be unique to that platform or operating system) from the logic of the game (which does not depend on the underlying OS).
Be careful with signed versus unsigned numbers
If you only want to allow positive numbers for
Prefer control constructs to state variables
Using state variables such as
Then the first two lines of your big loop could be this:
Avoid checking state multiple times
The logic of the game currently checks the state of the
Try to consolidate loop exit conditions at the top
Prefer a
This also eliminates a
Eliminate spurious variables
There are several variables, including
Omit
The compiler will automatically ge
Don't use non-ISO escape sequences
I assume from the context that you are intending
\e to mean the ESC character, '\x1b'. That might be supported by your compiler, but it's non-standard. Better would be to use '\x1b'. Isolate ANSI escape sequences into functions
It's often good practice to split out your escape sequences into a separate function. It's easier to read and understand
clearscreen() than printf("\e[1;1H\e[2J"); for most people.Use all the required
#include filesThe code calls
srand and time but doesn't include the corresponding files. The code should have these two lines added:#include
#include Eliminate needless repetition
The switch statements that print the names of the icons could be much simplified by adding this at file scope.
static const char* icon[4] = {"Bell", "Cherry", "Orange", "Horseshoe"};And then replacing all three switch statements with this one line:
printf("%s %s %s\n", icon[firstReel], icon[secondReel], icon[thirdReel]);Don't reseed the random number generator more than once
The program currently calls
srand at the top of each loop, but this is really neither necessary nor advisable. Instead, just call it once when the program begins and then continue to use rand() to get random numbers.Separate input, output and calculation
To the degree practical it's usually good practice to separate input, output and calculation for programs like this. By putting them in separate functions, it isolates the particular I/O for your platform (which is likely to be unique to that platform or operating system) from the logic of the game (which does not depend on the underlying OS).
Be careful with signed versus unsigned numbers
If you only want to allow positive numbers for
credit and bet which makes sense in this context, they should be declared as unsigned rather than int. Similarly, you can use %u in printf and scanf statements rather than %d. Prefer control constructs to state variables
Using state variables such as
cont tends to lead to fragile code that's hard to debug and maintain. Better would be to make it dependent on a necessary variable such as bet. So for instance, you could define a function as follows:static unsigned getBetAmount(unsigned credit)
{
unsigned bet = 0;
while (bet == 0) {
// Introduction of credit system to user
printf("\x1b[1;1H\x1b[2J");
printf("Your current credits are %u.\n", credit);
printf("How many credits would you like to bet?\n");
scanf("%u", &bet);
printf("\x1b[1;1H\x1b[2J");
// validate bet amount
if (bet > credit) {
bet = 0;
printf("Invalid, input exceeded your credit value of %d.\n",
credit);
}
else if (bet == 0) {
printf("Invalid, bet must be a minimum of 1 credit.\n");
}
else {
printf("You have bet %d credits. Good luck!\n", bet);
}
sleep(2);
// End of check
}
return bet;
}Then the first two lines of your big loop could be this:
bet = getBetAmount(credit);
credit -= bet;Avoid checking state multiple times
The logic of the game currently checks the state of the
victory variable multiple times, when really all that's needed is to either register a win or a loss. If you were to separate out a spin function which does the looping and displaying of the reels and simply returns true if the user has won, the win/loss calculations are much simplified:// Win/lose conditions
if (spin()) {
printf("Congratulations! You win! ");
credit = credit + (bet * 2);
printf("Your credit is now %d.\n", credit);
}
else
{
printf("Sorry, you lose. ");
if (credit == 0) {
printf("Your credit is now 0. Game over.\n");
return 0;
}
}Try to consolidate loop exit conditions at the top
Prefer a
for or while loop to do {...} while() loop. Because the reader is forced to search for the loop terminating condition, it's more effort to understand the latter construct. This code, for example could be structured like this:for (char end = 'N'; end == 'N'; )
{
bet = getBetAmount(credit);
credit -= bet;
countdown();
// all of the rest of the code
}This also eliminates a
break statement. A break can also contribute to difficulties understanding a loop because again, the programmer reading the code has to search for them.Eliminate spurious variables
There are several variables, including
cont, victory, userInput, rndOne, rndTwo and rndThree which can be elmininated from the code. Reducing the number of variables, especially when they're not really needed can make your code easier to understand and easier to maintain and debug.Omit
return 0 at the end of mainThe compiler will automatically ge
Code Snippets
#include <stdlib.h>
#include <time.h>static const char* icon[4] = {"Bell", "Cherry", "Orange", "Horseshoe"};printf("%s %s %s\n", icon[firstReel], icon[secondReel], icon[thirdReel]);static unsigned getBetAmount(unsigned credit)
{
unsigned bet = 0;
while (bet == 0) {
// Introduction of credit system to user
printf("\x1b[1;1H\x1b[2J");
printf("Your current credits are %u.\n", credit);
printf("How many credits would you like to bet?\n");
scanf("%u", &bet);
printf("\x1b[1;1H\x1b[2J");
// validate bet amount
if (bet > credit) {
bet = 0;
printf("Invalid, input exceeded your credit value of %d.\n",
credit);
}
else if (bet == 0) {
printf("Invalid, bet must be a minimum of 1 credit.\n");
}
else {
printf("You have bet %d credits. Good luck!\n", bet);
}
sleep(2);
// End of check
}
return bet;
}bet = getBetAmount(credit);
credit -= bet;Context
StackExchange Code Review Q#70102, answer score: 10
Revisions (0)
No revisions yet.