We can easily eliminate the unnecessary globals by making use of parameters and the return values of the function. Functions with no meaningful return value should be declared "void", to clarify your intent. The entry point, main(), is an exception. We should declare it returning int, but we are allowed omit the return statement. If you return no value from main(), the compiler makes it as if you had returned "0", which indicates success when returned from main().
We can also eliminate the redundant function declarations by moving the function definitions higher in the source file. This reduces the maintenance burden, it is one less thing we need to change if we want to modify the function signature.
Writing good variable/function names is very important. I recommend you get out of the habit of using an encoded shorthand for them.
Error checking is equally important. I've included basic error checking for reading in the answer here.
#include <iostream>
#include <cstdlib>
#include <ctime>
int generateGuess()
{
int i = std::rand() % 100;
return i + 1;
}
int main()
{
// Seed the random number generator
std::srand(std::time(NULL));
int answer;
if(std::cout << "Enter your number: " && std::cin >> answer)
{
std::cout << "You have selected: " << answer << '\n';
}
else
{
std::cerr << "You did not enter a number\n";
return 1;
}
// Game loop
boolean running = true;
while(running)
{
int guess = generateGuess();
if(guess == answer)
{
std::cout << "The computer guessed correctly: " << guess << '\n';
running = false;
}
else
{
std::cout << "The computer guessed incorrectly: " << guess << '\n';
}
}
}
Some other points:
- Standard header files do not have .h extensions. Use #include <iostream> instead of <iostream.h>
- Comparing a number with NULL is wrong. NULL should be reserved for interacting with pointers. NULL is really just the integer constant 0, which means that your comparison is not checking if g_numb had been set. In fact, global variables are zero initialised (complex types excepted), so you're really just checking if g_numb is zero. This could result in std::srand() being called multiple times.
- As others have mentioned, the easiest way to seed the random sequence is to call std::srand() once at the start of main, and never again.
- Try to keep I/O away from the actual game logic. In such a simple game, this isn't obvious, but it makes the code much easier to read and test if these things are separated. Here, the game logic is mainly in the generateGuess() function. I didn't move the game winning condition to a new function as it is trivial here. For a complex game you should.
- I've avoided checking the winning condition twice. For a more complex game like chess, this is a good idea, and it ensures the logic is consistent (you don't have any chance of accidentally modifying the game state to revert to a non-finished state).
Future improvements:
- Use a loop until the user enters a valid number. Remember valid doesn't just mean a number, it appears to mean a number between 1 and 100.
- Make the computer smarter. It is traditional in such games to let the guesser know if they need to go higher or lower. The computer's guess function should take just enough input that will allow it to make the decision the same way a human might.
