"Guess the Number" game in C

Question

This is my first program in C. I'd rather not form any bad habits now. Is there anything that looks like bad practice, or something just looks wrong?

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

int main(void) {
    srand(time(NULL));
    int r = rand() % 10 + 1;
    int correct = 0; 
    int guess; 
    int counter = 0; 

    printf("Guess my number! "); 

    do {
        scanf("%d", &guess);
        if (guess == r) {
            counter++;
            printf("You guessed correctly in %d tries! Congratulations!\n", counter);
            correct = 1; 
        }

        if (guess < r) {
            counter++;
            printf("Your guess is too low. Guess again. ");
        }

        if (guess > r) { 
            counter++; 
            printf("Your guess is too high. Guess again. ");
        }
    } while (correct == 0);

    return 0;
}

Answer 1

Here are some of the improvements i can suggest:

• move your counter variable , from the inside the if statements , as increasing the count is independent of the if condition.

• Use srand(time(NULL)); , below the declarations , or else , your compiler may throw this warning , when compiled using C90 standard.

  warning: ISO C90 forbids mixed declarations and code [-pedantic]

  So to ensure portability , avoid statements between declarations.

• In the long run , more meaningful and formal names , can be used instead of names like r and guess.

In my personal opinion , these two changes would be better:

• Using a break; , when the correct number is guessed , can eliminate the need for using an extra variable correct.

• I would prefer to use just a while() instead of do{}while();, if the program logic permits it.

Here is the program , modified with the changes , i listed above:

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

int main(void) 
{

int random_num = 0;
int guessed_num = 0;
int counter = 0; 

srand(time(NULL));
random_num = rand() % 10 + 1;

printf("Guess my number! "); 

    while(1)
    {
        counter++; 

        scanf("%d", &guessed_num);

        if (guessed_num == random_num) 
        {
            printf("You guessed correctly in %d tries! Congratulations!\n", counter); 
            break;
        }

        if (guessed_num < random_num) 
            printf("Your guess is too low. Guess again. ");

        if (guessed_num > random_num) 
            printf("Your guess is too high. Guess again. ");

    } 

return 0;   
}

Answer 2

• You need to use proper indention of everything inside main().
• Every scanf("%d") leaves a trailing line feed '\n' in stdin. They are skipped by further scanf("%d") reads, but if you attempted to scanf a character or string somewhere, you would get odd behavior. It is better to clear stdin after each access. The simplest way to do this is to add a call to getchar() after each scanf call. You can also start the scanf format string with a space, like scanf(" %s", str), the space will then discard left-over white spaces from stdin.
• The if statements could be rewritten as if(guess == r) {...} else if(guess < r) {...} else {}, this will make the program slightly more effective since it prevents multiple, redundant checks of the same variable. If the guess == r then you don't need to check if it is less than r as well.
• Instead of using an int variable correct with possible values 0 and 1, use a bool variable with values false and true (stdbool.h).
• The counter++ should be removed from inside the if statements to the main body of the loop, as it is always increased no matter.

EDIT

Also, consider adding new line characters after each print, for readable output. Code with the above suggestions:

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

int main(void) {
  srand(time(NULL));

  int r = rand() % 10 + 1;
  bool correct = false; 
  int guess; 
  int counter = 0; 

  while(!correct)
  {
    printf("Guess my number! "); 
    scanf("%d", &guess);
    getchar();

    if (guess < r) {
        printf("Your guess is too low. Guess again.\n");
    }
    else if (guess > r) { 
        printf("Your guess is too high. Guess again.\n");
    }
    else /* if (guess == r) */ {
        printf("You guessed correctly in %d tries! Congratulations!\n", counter); 
        correct = true; 
    }

    counter++;
  } /* while(!correct) */

  return 0;   
}

Answer 3

/*
 * File: guessing_game_codereview.c
 * Purpose: The Guessing Game
 * Date: 2015-08-28
 * Author: Robert A. Nader
 * Email: naderra at g ...
 * Platform: Linux
 * Compile: gcc -std=c89 -Wall -Wpedantic \
                 guessing_game_codereview.c \
                 -o guessing_game_codereview
 * Note: Should be fairly portable to any hosted implementation.
 * Kept source file line lengths under 80 characters.
 * ----------------------------------------------------------------------------
 * Added: this "Sample Changelog":
 * Added: sample source code documentation
 * Added: MAX_SECRET constant. (Could have used "#define MAX_SECRET 10"
 *                              but prefer "const int MAX_SECRET = 10;"
 *                              even at the cost of a couple of extra bytes)
 * Removed: needless boolean type
 * Removed: final else statement by using "(guess != secret)"
 * Note 1: "do {} while();" is correct in this case, so would a while () {} be!
 * Added: Test for non-integer user input, with appropriate error message.
 * Added: test for integer range, with appropriate error message.
 * Replace: return 0; for return EXIT_SUCCESS;
 */
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
int main(void) {
  const int MAX_SECRET = 10;
  int secret;
  int guess = 0;
  int guess_count = 0;
  int input_trail_count;
  /* Seed - init the pseudo-random generator */
  srand(time(NULL));
  secret = rand() % MAX_SECRET + 1;
  do {
    printf("Guess my number between 1 and %d inclusive: ", MAX_SECRET); 
    scanf("%d", &guess);
    /* Handle possible user string input by consuming
       and counting any remaining input characters */
    input_trail_count = 0;
    while ('\n' !=  getchar()) { ++input_trail_count; };
    if (0 == input_trail_count) {  /* no trailing chars after integer */
      if (guess > 0 && guess <= MAX_SECRET) { /* integer within range */
        if (guess < secret) {
          printf("Your guess [%d] is too low. Try again.\n", guess);
        } else if (guess > secret) { 
          printf("Your guess [%d] is too high. Try again.\n", guess);
        }
        ++guess_count;
      } else { /* integer out of range */
        printf("Error: integer value [%d] is out of range (1-%d) !\n",
               guess, MAX_SECRET);
      }
    } else { /* detected non-integer input: string */
      printf("Error: detected non-integer input, only integers allowed!\n");
    }
  } while (guess != secret);
  printf("You guessed my number [%d] in [%d] tries!\n", secret, guess_count);
  return EXIT_SUCCESS;
}
/* eof */

• Added: this "Sample Changelog":
• Added: sample source code documentation
• Added: MAX_SECRET constant. (Could have used "#define MAX_SECRET 10" but prefer "const int MAX_SECRET = 10;" even at the cost of a couple of extra bytes)
• Removed: needless boolean type
• Removed: final else statement by using "(guess != secret)"
• Note 1: "do {} while();" is correct in this case, so would a while () {} be!
• Added: Test for non-integer user input, with appropriate error message.
• Added: test for integer range, with appropriate error message.
• Replace: return 0; for return EXIT_SUCCESS;