General observations
To be honest, your code is extremely clear and readable to me. I wouldn't guess that you were a beginner from reading your code. You have eliminated the use of magic numbers, and use global constants instead which is good!
Anonymous namespaces
The keyword static in this context means that it has internal linkage. An anonymous namespace also does the same thing, but they are considered to be somewhat superior to the static keyword in C++.
The link I cited has some great answers.
But mainly,
staticwill only work for functions and objects, an anonymous namespace on the other hand can let you have your own type definitions, classes, structs (almost everything)...
// Globals.h
namespace
{
// constants
}
Prefer using constexpr
The keyword
constexprwas introduced in C++11 and improved in C++14. It means constant expression. Likeconst, it can be applied to variables: A compiler error is raised when any code attempts to modify the value. Unlikeconst,constexprcan also be applied to functions and class constructors. constexpr indicates that the value, or return value, is constant and, where possible, is computed at compile time.
Use constexpr when you can, it tells the compiler that it's literally just a constant.
It forces the compiler to calculate the value of something at compile-time. Moreover, you can pass it as a template argument too
namespace
{
constexpr char COMPUTER_GUESSER { 'c' };
}
Use an enum
This point can depend according to your style, but I think that an enum is called for here.
I am talking about these variables
COMPUTER_GUESSER = 'c';
PLAYER_GUESSER = 'p';
QUIT = 'q';
ANSWER_IS_YES = 'y';
ANSWER_IS_NO = 'n';
I believe that having an enum here makes sense because you can group these variables as they all are related to the user's choice, this is what it would look like
enum class Choice : char
{
COMPUTER_GUESSER = 'c',
PLAYER_GUESSER = 'p',
QUIT = 'q',
ANSWER_IS_YES = 'y',
ANSWER_IS_NO = 'n',
}
if (input == Choice::QUIT) //...
else if (input == Choice::ANSWER_YES) //...
Generating a random int
C++ has std::uniform_int_distribution which is better than C's rand().
Consider inlining smaller functions
int randomNumGenerator(const int max, const int min)
{
return rand() % max + min;
}
int rangeNumToGuess(const int max, const int min)
{
return ((max - min) / 2) + min;
}
int rangeNum(const int max, const int min)
{
return max - min;
}
Inlining these functions can improve the performance a lot, but you need to place the definition of these functions in the header file, you can specify inline but it's likely that the compiler will inline them itself.
instead of executing the function call CPU instruction to transfer control to the function body, a copy of the function body is executed without generating the call.
Always handle invalid input
std::cout << "Enter your guess number: ";
while (std::cin >> userGuess)
{
//...
}
Here, std::cin is expecting an integer, if the user accidentally enters something else, std::cin will fail, leading to strange behavior in your
There are a few ways, this article is worth reading.
A small bug
In your restart() function
bool restart()
{
char userChoice{};
std::cout << "Play again? (y/n): ";
std::cin >> userChoice;
char lowerUserChoice = tolower(userChoice);
if (lowerUserChoice == ANSWER_IS_YES)
{
startGame();
}
else if (lowerUserChoice == ANSWER_IS_NO)
{
computerOrPlayer(QUIT);
}
else
{
std::cout << "Please choose the available option\n";
restart();
}
return true;
}
Since you recursively call restart() on invalid input, you should return the value you get. Otherwise, the function won't return anything
else
{
std::cout << "Please choose a valid option!\n";
return restart();
}