Fruitmachine game

Question

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 <stdio.h>
#include <unistd.h>


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 <= credit && 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){
    sleep(1);

    // Generating three random numbers
    srand(time(NULL));
    int rndOne = rand () %4;
    int rndTwo = rand () %4;
    int rndThree = rand () %4;

    // Assigning random numbers to clearer var names
    firstReel = rndOne;
    secondReel = rndTwo;
    thirdReel = rndThree;

    // Switch statements for each reel
    switch(firstReel){
      case 0:
        printf("Bell ");
        break;
      case 1:
        printf("Cherry ");
        break;
      case 2:
        printf("Orange ");
        break;
      case 3:
        printf("Horseshoe ");
        break;
    }

    switch(secondReel){
      case 0:
        printf("Bell ");
        break;
      case 1:
        printf("Cherry ");
        break;
      case 2:
        printf("Orange ");
        break;
      case 3:
        printf("Horseshoe ");
        break;
    }

    switch(thirdReel){
      case 0:
        printf("Bell\n");
        break;
      case 1:
        printf("Cherry\n");
        break;
      case 2:
        printf("Orange\n");
        break;
      case 3:
        printf("Horseshoe\n");
        break;
    }
    // End of switch statements for each reel

    loop++;

    // Clears the previous printed combination
    if (loop < 10)
      printf("\e[1;1H\e[2J");
    else
    {
      break;
    }
  }

  // Win/lose conditions
  if (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. Game over.\n");
      return 0;
    }
    else
    {
    printf("Your credit is now %d.\n", credit);
    }
  }

  // Option for user to terminate the game
  printf("Would you like to cash out? (Y/N)\n");

  // Takes user input and cancels if Y
  scanf(" %c", &end);
  if (end == 'Y'){
    printf("\e[1;1H\e[2J");
    printf("Your final credit score is: %d.\n", credit);
    sleep(3);
    return 0;
  }
  else
  {
     end = 'N';
  }
  cont = 0;
} while (end == 'N');
return 0;
}

Edward · Accepted Answer · 2014-11-17 20:03:11Z

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 \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` files

The code calls srand and time but doesn't include the corresponding files. The code should have these two lines added:

#include <stdlib.h>
#include <time.h>

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 `main`

The compiler will automatically generate a return 0; at the end of main so it is not necessary to supply your own.

I upvoted this nice answer, but I have a question. If you don't return 0 from main, will it really return 0? Or is that true only for C99? — JS1, Nov 17 '14 at 22:22
It will really return 0, but only if the compiler is compliant with C99. — Edward, Nov 18 '14 at 3:41

vnp · Accepted Answer · 2014-11-17 18:06:59Z

I have no idea what the escape sequence actually does (I bet, not many people know that off the top of the head). Wrap printf("\e[1;1H\e[2J") into a function with a meaningful name.
Don't be shy on functions. Taking user input, for example; printing reel names, etc.
Since you are explicitly break out of the loop, a cont variable is not redundant.
Why bother with rndOne, if you immediately reassign random numbers to clearer var names? A direct int firstReel = rand() % 4; is much more clear.
srand() is an initialization of a RNG, and should be called once per program.
The construct
```
while(loop < 10){
    ...
    loop++;
    if (loop < 10)
        printf("\e[1;1H\e[2J");
    else
    {
        break;
    }
}
```
looks strange at best. Same goes for the subsequent test of if (loop > 10 ...): loop is necessarily equal to 10 when you are out of the loop. Please double check the logic.
You should combine the win/lose test with the payout calculations. victory is redundant.

Community · Accepted Answer · 2017-05-23 12:40:54Z

You can start by splitting this program into separate functions. This should help with spotting problems as well as improving efficiency. Everything is currently in one large do-while loop, so one would have to go to the very bottom to see the loop statement. Having additional functions will at least decrease the size of this loop greatly.

Also, as a possible micro-optimization, use puts() instead of printf() when printing unformatted hard-coded output (no variables).

200_success · Accepted Answer · 2014-11-18 02:50:44Z

Compiler warnings and a logic bug

Always compile with warnings enabled. My compiler produces four warnings:

$ clang -Wall -c cr70102.c
cr70102.c:66:5: warning: implicit declaration of function 'srand' is invalid in C99 [-Wimplicit-function-declaration]
    srand(time(NULL));
    ^
cr70102.c:66:11: warning: implicit declaration of function 'time' is invalid in C99 [-Wimplicit-function-declaration]
    srand(time(NULL));
          ^
cr70102.c:67:18: warning: implicit declaration of function 'rand' is invalid in C99 [-Wimplicit-function-declaration]
    int rndOne = rand () %4;
                 ^
cr70102.c:135:17: warning: '&&' within '||' [-Wlogical-op-parentheses]
  if (loop > 10 && firstReel == secondReel || secondReel == thirdReel){
      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~ ~~
cr70102.c:135:17: note: place parentheses around the '&&' expression to silence this warning
  if (loop > 10 && firstReel == secondReel || secondReel == thirdReel){
                ^
      (                                   )
4 warnings generated.

Of those, the first three are complaining about your failure to #include <stdlib.h> and <time.h>.

The last warning is more serious — it indicates a real logic bug. The superficial problem is that the order of precedence is wrong. You also happen to have a second issue: loop always equals 10 at that point in the program. The net effect is that the only way to win is if the second and third reel are the same.

I also wonder whether the first and third reels coming up the same should result in a win.

Minor bugs and annoyances

If you try to bet a negative amount, it rejects the amount as for "exceeding your credit value" rather than for being under the "minimum of 1 credit."

When spinning, the words hop around horizontally, ruining the illusion. The words should be displayed in fixed-width fields.

To cash out, you must enter a capital Y; a lowercase y is treated as "no".

Code organization

Everything is in one huge main() function. There is repeated code (for clearing the screen¹ and spinning a reel) that should definitely be moved into functions to reduce repetition, because repetitiveness is bad for maintainability. In addition, prompting for the bet amount and asking the user whether to cash out are self-contained tasks that can be moved out of the way of the main loop.

void clscr()
{
    fputs("\x1b[1;1H\x1b[2J", stdout);
}

int get_bet(int credit)
{
    do
    {
        int bet;

        // Introduction of credit system to user
        clscr();
        printf("Your current credits are %d.\n", credit);
        printf("How many credits would you like to bet?\n");
        scanf("%d", &bet);
        clscr();

        // Checking for valid credit input and adjusting credit/bet value according to input
        if (bet < 1)
        {
          printf("Invalid, bet must be a minimum of 1 credit.\n");
          sleep(2);
        }
        else if (bet > credit)
        {
          printf("Invalid, input exceeded your credit value of %d.\n", credit);
          sleep(2);
        }
        else
        {
          printf("You have bet %d credits. Good luck!\n", bet);
          sleep(2);
          return bet;
        }
    } while (1);
}

int cash_out()
{
    char end;

    // Option for user to terminate the game
    printf("Would you like to cash out? (Y/N)\n");

    // Takes user input and cancels if Y
    scanf(" %c", &end);
    return (end == 'Y' || end == 'y');
}

const char *spin()
{
    static const char *FRUITS[] = {
        "   Bell   ",
        "  Cherry  ",
        "  Orange  ",
        "Horseshoe "
    };
    return FRUITS[rand() % (sizeof(FRUITS) / sizeof(FRUITS[0]))];
}

Defining those supporting functions makes it possible to have a main() that is a more reasonable size, and that has fewer variables to keep track of.

int main()
{
    int credit = 5;
    srand(time(NULL));

    do
    {
        int bet = get_bet(credit);
        credit -= bet;

        // Introduction countdown to execute after valid bet is placed
        clscr();    puts("Ready?");     sleep(1);
        clscr();    puts("3...");       sleep(1);
        clscr();    puts("2...");       sleep(1);
        clscr();    puts("1...");       sleep(1);
        clscr();    puts("Spin!");

        const char *reels[3];

        // Loop to print cycle reel 10 times
        for (int loop = 0; loop < 10; loop++) {
            sleep(1);
            clscr();

            for (int reel = 0; reel < sizeof(reels) / sizeof(reels[0]); reel++) {
                fputs(reels[reel] = spin(), stdout);
            }
            fputs("\n", stdout);
        }

        // Win/lose conditions
        if (reels[0] == reels[1] || reels[1] == reels[2]) {
            printf("Congratulations! You win! ");
            credit += 2 * bet;
        }
        else
        {
            printf("Sorry, you lose. ");
            if (credit == 0) {
                printf("Your credit is now 0. Game over.\n");
                return 0;
            }
        }

        printf("Your credit is now %d.\n", credit);
    } while (!cash_out());

    clscr();
    printf("Your final credit score is: %d.\n", credit);
    sleep(3);

    return 0;
}

¹ Is there a reason why the escape sequence after "Spin!\n" is a bit different from the others? If it's deliberate, then it deserves a comment.

Stack Exchange Network

current community

your communities

more stack exchange communities

Fruitmachine game

4 Answers 4

Don't use non-ISO escape sequences

Isolate ANSI escape sequences into functions

Use all the required `#include` files

Eliminate needless repetition

Don't reseed the random number generator more than once

Separate input, output and calculation

Be careful with signed versus unsigned numbers

Prefer control constructs to state variables

Avoid checking state multiple times

Try to consolidate loop exit conditions at the top

Eliminate spurious variables

Omit `return 0` at the end of `main`

Compiler warnings and a logic bug

Minor bugs and annoyances

Code organization

Your Answer

Not the answer you're looking for? Browse other questions tagged c game console or ask your own question.

Hot Network Questions

Fruitmachine game

4 Answers 4

Don't use non-ISO escape sequences

Isolate ANSI escape sequences into functions

Use all the required #include files

Eliminate needless repetition

Don't reseed the random number generator more than once

Separate input, output and calculation

Be careful with signed versus unsigned numbers

Prefer control constructs to state variables

Avoid checking state multiple times

Try to consolidate loop exit conditions at the top

Eliminate spurious variables

Omit return 0 at the end of main

Compiler warnings and a logic bug

Minor bugs and annoyances

Code organization

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged c game console or ask your own question.

Related

Hot Network Questions

Use all the required `#include` files

Omit `return 0` at the end of `main`