Thread: Need Review for a Program I just made.

Hello everyone! This is my first time posting on this forum, and I'd like for some people to review my code for a small game I made. It's a typical Random Number Guessing Game, and I was wondering how I could improve this code. I am still new to C, although not new to programming. I started with Python.

Code:

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


int main(void) {
    int running = 1;
    while (running = 1) {
        char tryAgain = 'n';
        int random;
        static int userIn;
        int counter = 0;
        srand(time(NULL));
        random = rand();
        random = random % 100;
        printf("Welcome to Random Gems!\n");
        printf("Your computer has already randomly generated a number between 1 and 100.\n");
        printf("It is your job to find what number that was in (7) tries.\n");
        for (int i = 0; i < 7; i++) {
            printf("Your guess: ");
            scanf("%d", &userIn);
            if (userIn > random) {
                printf("Too high, try again.\n");
            }
            else if (userIn < random) {
                printf("Too low, try again.\n");
            }
            else if (userIn == random) {
                printf("Congratulations! You got it in %d tries!\n", counter);
                break;
            }
            else {
                printf("INPUT error! EXITING program!");
                break;
            }
            counter++;
        }
        printf("Try again?[y/n]: ");
        scanf("%s", &tryAgain);
        if (tryAgain == 'y') {
            printf("Restarting the game!");
        }
        else if (tryAgain == 'n') {
            break;
        }
        else {
            printf("INPUT error! EXITING program!");
            break;
        }
    }
    printf("Thank you for playing!");
    getch();
    return 0;
}

Look at the following snippet:

Code:

    while (running = 1) {

In C/C++ the operator= is the assignment operator, and operator== is the comparison operator. When you use the assignment operator in your while loop you have an infinite loop, which is equilavent to while(1).

Also the srand() function should be called only once.

And you need to study up on the scanf() format specifiers, the "%s" specifier is for a C-string, not a single character, tryAgain is a single character.

Lastly getch() is a non-standard function that you should avoid. You should really prefer the getchar() function instead. And if you must use getch() you need to #include the proper include file for this non-standard function.

Jim

I couldn't really find improvements to the code that other's haven't caught, but I did notice this line:

Code:

        printf("Try again?[y/n]: ");
        scanf("%s", &tryAgain);
        if (tryAgain == 'y') {
            printf("Restarting the game!");
        }

In order to implement this properly, a good method might be to have the user choice to play the game in a while loop that surrounds the game (which I would move to its own function). It's always a good idea to break unique behaviors into their own functions, it might not matter much in a small game like this, but it is vital once you get larger code going.

I wrote a little game in the same manner as the one you have (number guessing) to show you what I mean:

Code:

#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <ctype.h>
#define MAXGUESS 7
#define MAXRAND 201

void guessing_game(void);
int compare_guess(int,int);
void nlkill(void);

int main()
{
    srand(time(NULL));
    char playchoice = 'b';
    int numbergames = 0;

    while(playchoice != 'n'){
        printf("Would you like to play %s?\n\n",
               (numbergames > 0) ? "again" : "");
        printf("[Y] / [N] >");

        if(scanf(" %c", &playchoice) != 1){
            printf("Please enter numbers only\n\n");
        }

        playchoice = tolower(playchoice);

        if(playchoice == 'y'){
            guessing_game();
            ++numbergames;
        }

        if(playchoice != 'n' || playchoice != 'y'){
            nlkill();
        }
    }

    printf("Thank U 4 being such a great playa!\n\n");

    return 0;
}

void nlkill(void)
{
    while(getchar() != '\n');
}

void guessing_game(void)
{
    int random;
    int guesses = 0;
    int playerguess;
    random = rand()%MAXRAND+1;

    while(guesses <= MAXGUESS){

        printf("\nGuess a number between 1 and %d\n\n>", MAXRAND);

        if(scanf(" %d", &playerguess) != 1){
            printf("Please enter numbers only\n");
        }

        if(compare_guess(playerguess, random) == 1){
            printf("Congratulations, you're a big wiener!\n\n");
            return;
        }

        else ++guesses;
    }

    printf("You a LOSER!\n\n");
}

int compare_guess(int player, int random)
{
    if(player < random){
        printf("\nThat's the lowest of the low.\n\n");
        return 0;
    }

    else if(player > random){
        printf("\nYou are so high right now!\n\n");
        return 0;
    }

    return 1;
}

In the main function you have the outer loop, which asks if the player wants to play. If the player is on their 2nd game or later, a ternary operator will add "again" to "Do you want to play?". You have a bit of checking the return of scanf, and also putting the player choice variable through a function called tolower() which will make it lowercase (so we can compare it). If input is invalid, I flush the input buffer with nlkill() (getchar !='\n').

Then the guessing_game() function is the body of the actual game, it's in its own function so that I can easily see where it is, and know what behaviors it should display. This function then uses the compare function to determine if the player has won.

On a final note, one thing you might want to think about (not really a coding thing), is the conceptual side of design. For instance, in a game like this, if the player uses the most efficient manner of guessing, is there a statistical likelihood they could still lose (would you want them too?). As it is, using a simple binary search, the player is almost certainly going to win.

Why is userIn static ?