• 97.7k
• 5
• 126
• 341

• 6.7k
• 23
• 35

On Comments

I do have comments, I hope they make the program readable!

Unfortunately, no. Your comments don’t help readability and only take up unnecessary space (which makes the program less readable).

The problem is that your comments convey no useless information. For instance,

// Pause.
pause();

How is this comment helping?

There’s a commonly-quoted adage,

Code explains the how, comments explain the why

which actually covers most cases: use code to explain how a solution is implemented. Only use comments when necessary to explain why you do something.

In practice, this means that most code doesn’t need comments at all, good code should ideally be self-explanatory; if it isn’t, take that as a hint to rewrite it.

Here’s a positive example, however:

// We could 'punish' however, we will be nice.
lives++;

Now this comment is more helpful. But it could still be a bit clearer.

There’s one exception to this no-comments rule:

Do provide documentation for all functions via comments. But again, use the documentation to explain stuff that is not explained by the function name and signature.

For instance,

///--------------------------------------
/// Pauses the console window.
///--------------------------------------
void pause();

... not particularly helpful. What does "pauses the console window" actually mean? A more helpful description would be, "Halts execution until the user hits any hey".

Your other function documentations are much better in this regard. But take care to explicitly describe all parameters, the return value, and all side effects.

Implementation

(In addition to what @towi already wrote.)

Input

Your input routine does three times the work:

1. It inputs a string
2. It tests whether this is a valid number, via isValidInput_Int.
3. It converts it to a number via atoi.

isValidInput_int is actually a lot of boilerplate code doing the same thing as just calling atoi. In fact, isValidInput_int doesn’t work at all because atoi cannot be used reliably to test whether a valid number was entered. It doesn’t throw an exception, it just silently returns 0. You have no way of knowing whether the user entered 0 or an invalid number.

Furthermore, isValidInput_int uses the bool type but you never actually use the bool constants true and false, instead you use 1 and 0. Do not do this. Unfortunately, C++ allows this but it’s a weakness of the type system (thanks to C).

Furthermore, atoi is redundant here anyway, just input the number directly. That is, instead of reading a string, do this:

int difficulty;
if (cin >> difficulty) {
 // Input was succesful, proceed.
}
else {
 printError(1, true);
}

Use meaningful variable names

(difficulty * 2) * 10 crops up several times. What does it mean? Put it into a meaningful variable to increase readability (range would be a meaningful name).

inputVal describes where the value comes from, but not what it signifies. How about nextGuess instead?

Also, you should settle on a single convention for names. At the moment you mix between camelCase and under_score, sometimes_WithCapital_Letters. Modern C++ code generally uses lowercase_words_separated_by_underscore but you’re free to use your own convention – the important thing is consistency.

Other stuff

You seem to always finish your functions with return; – that’s redundant. You only need return if you want to return a value, or if you want to exit the function prematurely. At the end of a function, you don’t need it.

Furthermore, main is a special case, return 0; is implied at the end of it, so this is also redundant (but all other functions that return values need explicit returns).

Finally, the logic flow in game isn’t entirely obvious. This could be cleared up. In particular, when do lives go up and down? It might help to simplify the input reading routine (see above) and to extract the guess into a separate function. In general, try to limit the level of nesting drastically. Four levels of nesting (here, a for and three ifs) should really be the exception. Your logic here could really be simplified, for instance by reversing the condition if (lives != 1), and exiting the loop early:

if(lives == 1)
{
 cout << "Incorrect, the number was: [" + convertIntToString(numberToGuess) + "]\n";
 break;
}
if(inputVal > numberToGuess)
{
 message = "Incorrect, try guessing lower!\n";
}
if(inputVal < numberToGuess)
{
 message = "Incorrect, try guessing higher!\n";
}

In my opinion, this still isn’t as readable as it could be because it takes up a lot of unnecessary vertical space. I find the following much more readable but be aware that this opinion isn’t shared by all.

if(lives == 1) {
 cout << "Incorrect, the number was: [" + convertIntToString(numberToGuess) + "]\n";
 break;
}
if(inputVal > numberToGuess)
 message = "Incorrect, try guessing lower!\n";
if(inputVal < numberToGuess)
 message = "Incorrect, try guessing higher!\n";

Some people will hate me for giving this advice but they probably wear their pants on their head. ;-)