patterncModerateCanonical
"Guess the number Game" in C
Viewed 0 times
numbergametheguess
Problem
Just started learning C. I hope my code adheres to best practices and idioms
#include
#include
#include
#include
int main(void)
{
int iRandomNum;
int iGuess;
srand(time(NULL));
iRandomNum = (rand() % 10);
printf("Guess a number between 1 and 10:");
scanf("%d", &iGuess);
// isdigit function expects a character so the guess value has to be ascii value of the digit character
iGuess += 48;
if (isdigit(iGuess)){
iGuess -=48; // change back to the original value for comparison with random number
if (iGuess == iRandomNum){
printf("You Guessed Correctly\n");
}
else{
printf("The correct answer was %d\n",iRandomNum);
printf("You guessed %d\n",iGuess);
}
}
else{
printf("This is not a digit!\n");
}
}Solution
I have observed a few things that may help you improve your code.
Use consistent formatting
The code as posted has inconsistent indentation which makes it hard to read and understand. Pick a style and apply it consistently.
Check return values for errors
The call to
Don't use Hungarian notation
Prefixing every variable with an abbreviation of its type is usually called "Hungarian notation" and it was once popular. Even then it was a bad idea. Don't clutter up your source code with that; instead, concentrate on defining meaningful names for each variable and choose types appropriately. I'd suggest
Use a better random number generator
You are currently using
There are a number of problems with this approach. The most significant problem is that the range this returns (0 through 9 inclusive) does not match what the user is asked to guess ("a number between 1 and 10") Second, this will generate lower numbers more often than higher ones -- it's not a uniform distribution. Another problem is that the low order bits of the random number generator are not particularly random, so neither is the result. On my machine, there's a slight but measurable bias toward 0 with that. See this answer for details, but I'd recommend changing that to
Think of the user
When someone asks me to "Guess a number between 1 and 10", I usually say \$e\$. (Yes, I'm afraid I'm that kind of person.) It seems clear from the context of your program that what you really mean is to guess an integer, and it seems likely that, technically speaking, you don't want numbers solely between 1 and 10 but that you mean to include 1 and 10 among the possible choices. To more clearly convey that, I'd suggest asking the user to "guess a whole number from 1 to 10 inclusive" but I'm probably too used to being around engineers and scientists.
Check your logic
If I input
Use consistent formatting
The code as posted has inconsistent indentation which makes it hard to read and understand. Pick a style and apply it consistently.
Check return values for errors
The call to
scanf can fail. You can check the return values to make sure they haven't or your program may crash (or worse) when given malformed input or due to low system resources. Rigorous error handling is the difference between mostly working versus bug-free software. You should strive for the latter.Don't use Hungarian notation
Prefixing every variable with an abbreviation of its type is usually called "Hungarian notation" and it was once popular. Even then it was a bad idea. Don't clutter up your source code with that; instead, concentrate on defining meaningful names for each variable and choose types appropriately. I'd suggest
secretNumber and userGuess in this case.Use a better random number generator
You are currently using
iRandomNum = (rand() % 10);There are a number of problems with this approach. The most significant problem is that the range this returns (0 through 9 inclusive) does not match what the user is asked to guess ("a number between 1 and 10") Second, this will generate lower numbers more often than higher ones -- it's not a uniform distribution. Another problem is that the low order bits of the random number generator are not particularly random, so neither is the result. On my machine, there's a slight but measurable bias toward 0 with that. See this answer for details, but I'd recommend changing that to
secretNumber = rand() / (RAND_MAX / 10) + 1;Think of the user
When someone asks me to "Guess a number between 1 and 10", I usually say \$e\$. (Yes, I'm afraid I'm that kind of person.) It seems clear from the context of your program that what you really mean is to guess an integer, and it seems likely that, technically speaking, you don't want numbers solely between 1 and 10 but that you mean to include 1 and 10 among the possible choices. To more clearly convey that, I'd suggest asking the user to "guess a whole number from 1 to 10 inclusive" but I'm probably too used to being around engineers and scientists.
Check your logic
If I input
"e" as my guess, the scanf call will fail and the variable may be uninitialized.Code Snippets
iRandomNum = (rand() % 10);secretNumber = rand() / (RAND_MAX / 10) + 1;Context
StackExchange Code Review Q#156219, answer score: 13
Revisions (0)
No revisions yet.