1
\$\begingroup\$

I have written a complex number guess game. The output should be like this:

Enter initial guess between 0 and 99:

45

Initial guess is 45

45 is too high. Make a guess between 0 and 45

10

10 is too high. Make a guess between 0 and 10

50

50 out of range. Make a guess between 0 and 10

2

2 is too low. Make a guess between 2 and 10

8 is the correct guess! Game over!

What I wrote is this:

int main(){

srand(time(NULL));
int rannum;
int guess;
int max1=0,min2=0;
rannum = rand() % 99 + 1;

printf("Enter guess");
scanf("%d", &guess);

printf("%d", guess);


while (guess<99 && guess>0){
    if (rannum < guess){
        guess = max1;
        printf("%d is too high,0 and %d", max1, max1);
    }
    scanf("%d", &guess);

     if (guess > max1){

        printf("%d is out of range,enter between 0 and %d", guess, max1);
    }
     scanf("%d", &guess);
     if (guess < rannum){
        guess = min2;
        printf("%d is too low,guess between %d and %d", min2, max1, max1);
    }
     scanf("%d", &guess);
     if (guess == rannum){
         printf("You win");
     }
}

return 0;
}

It works somehow, but I would like some input on how well it's written.

\$\endgroup\$
  • 1
    \$\begingroup\$ When you paste your code, please use the {} button afterwards on all of the code or and 4 spaces at the beginning of every line. Please be more explicit about the help you are looking for. \$\endgroup\$ – pacmaninbw Dec 31 '17 at 17:20
  • \$\begingroup\$ Don't write, never present uncommented/undocumented code. Let us in on what is complex about this. \$\endgroup\$ – greybeard Dec 31 '17 at 18:04
2
\$\begingroup\$

This seems pretty straightforward and is readable. The variable names are pretty good. I'd change the following things.

Use Functions for Clarity

Speaking of functions, I recommend thinking about the problem you're solving as a set of steps, and then putting each step into its own function. In this case, there are several steps that need to occur:

  1. Generate the number to be guessed
  2. Prompt the user for their guess
  3. Check their guess
  4. If it was correct, tell them and exit
  5. Otherwise, tell them what was wrong
  6. Repeat steps 2-6

So you could write a function for most of those things:

int generateRandomNumber (int low, int high)
{
    int range = high - low + 1;
    int randomNum = low + rand() % range;
    return randomNum;
}

int promptUserForGuess(const char* prompt)
{
    printf(prompt);
    int guess = 0;
    scanf("%d", &guess);
    return guess;
}

typedef enum {
    ERR_NONE,
    ERR_TOO_LOW,
    ERR_TOO_HIGH,
    ERR_OUT_OF_RANGE
} GuessError;

GuessError checkGuess(int target, int low, int high, int guess)
{
    GuessError result = ERR_NONE;

    if ((guess < low) || (guess > high))
    {
        result = ERR_OUT_OF_RANGE;
    }
    else if (guess < target)
    {
        result = ERR_TOO_LOW;
    }
    else if (guess > target)
    {
        result = ERR_TO_HIGH;
    }

    return result;
}

void displayErrorOrSuccess(GuessError err, int low, int high, int guess)
{
    switch (err)
    {
        case ERR_NONE:
            printf ("You win\n");
        break;

        case ERR_TOO_LOW:
            printf ("%d is too low. Guess between %d and %d\n", guess, guess, high);
        break;

        case ERR_TOO_HIGH:
            printf ("%d is too high. Guess between %d and %d\n", guess, low, guess);
        break;

        case ERR_OUT_OF_RANGE:
            printf ("%d is out of range. Enter a guess between %d and %d\n", guess, low, high);
        break;
    }
}

Then your main() function becomes really simple and easy to read:

int main () {
    srand((unsigned)time((time_t*)NULL));
    int rannum = generateRandomNumber(1, 99);

    GuessError err = ERR_NONE;
    do {
        int guess = promptUserForGuess("Enter a guess:\n");
        err = checkGuess(rannum, 1, 99, guess);
        displayErrorOrSuccess(err, 1, 99, guess);
    } while (err != ERR_NONE);
}

Avoid Magic Numbers

I notice in your code that you have the value 99 written several times. What happens if you want to change the range of the random number? You'll have to update it each place it occurs in the code. We call bare numbers like these "magic numbers" because it's impossible to tell what they mean when reading the code. I recommend making some constants like this:

const int MIN_RANDOM_NUM = 1;
const int MAX_RANDOM_NUM = 99;

Then in your code you would use the named constants instead of the bare numbers. It immediately becomes obvious what they mean.

Bugs

There are a number of bugs in your original version. You're deciding whether to continue or not based on whether the guess is out of range or not. If the user enters a guess out of range, the program stops before it can tell the user what the problem is. You should be basing whether to stop or not on whether the user has guessed correctly. If the value is out of range, tell them that and have them enter a new guess.

Also, if they enter a value in range, it stops and waits for input a second time with no prompt. I think you meant for the first call to scanf() inside the while loop to be inside the first if. In fact, all of the calls to scanf() in the while loop should be inside the corresponding if statements.

You never set max1 and min2 to any useful values. So it always tells the user to enter a guess between 0 and 0. It would be best if they started out as max1 set to 99 and min2 set to 1, and then you updated them based on the value of the guess. For example if the user enters 5 and it's too low, the new minimum should be 5 for the next go round.

\$\endgroup\$
  • \$\begingroup\$ The indentation problem is primarily a cut and paste issue when creating the question. The main() { and the ending } were originally outside the code. You may want to remove that portion of your answer. \$\endgroup\$ – pacmaninbw Dec 31 '17 at 23:16
  • \$\begingroup\$ OK, thanks. I didn't think to look through the edits. I'll try to remember that next time. \$\endgroup\$ – user1118321 Dec 31 '17 at 23:24
  • \$\begingroup\$ Minor point: Better to check the return value from scanf() then just code int guess = 0; scanf("%d", &guess);. On a rare input error, the value of guess is indeterminate and not necessarily 0. Still coding a helper function like int promptUserForGuess(const char* prompt) is a very good improvement. \$\endgroup\$ – chux Jan 2 '18 at 18:15
  • \$\begingroup\$ The time_t* is unnecessary in srand((unsigned)time((time_t*)NULL)); yet not bad to use - a style isseu I guess. Overall - good review. \$\endgroup\$ – chux Jan 2 '18 at 18:18
1
\$\begingroup\$

Off by 1. Number generated is from 1 to 99.

rannum = rand() % 99 + 1;

Yet only guesses in the range 1 to 98 allowed to loop

while (guess<99 && guess>0){

Better to use code constants.

const int minn = 1;
const int maxn = 99;
rannum = rand() % (maxn - minn + 1) + minn;
while (guess>= minn && guess <= maxn){
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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