Guess the Number In C++

Question

Mind that this is my first actual program in C++ and I tried to prepare for any possible input. Also the reason I included static_cast<void>(generator(range)) is to throw away the first random value because I read on LearnCpp.com that throwing away the first is a common practice to produce more random results. Anyways here is the code, all suggestions are welcome.

#include <iostream>
#include <limits>
#include <random>
bool validateInput(int input)
{
 using namespace std;
 if (1 <= input && input <= 100)
 return true;
 else if (!input)
 return false;
 else
 return false;
}
int generateRandomNumber()
{
 std::random_device dev;
 std::mt19937_64 range(dev());
 std::uniform_int_distribution<std::mt19937_64::result_type> generator(1, 100);
 static_cast<void>(generator(range));
 return static_cast<int>(generator(range));
}
bool placement(int input, int target)
{
 using namespace std;
 if (input > target)
 {
 cout << "Guess is too high.";
 return false;
 }
 else if (input < target)
 {
 cout << "Guess is too low.";
 return false;
 }
 else
 {
 cout << "Guess is correct!";
 return true;
 }
}
int main()
{
 using namespace std;
 cout << "You have 7 guesses to guess what number I am thinking of between 1 and 100.\n";
 int winning_num{ generateRandomNumber() };
 static int tries{ 1 };
 do
 {
 int num;
 cout << "\nGuess#" << tries << ": Enter a number -> ";
 cin >> num;
 if (!cin)
 {
 cin.clear();
 cin.ignore(numeric_limits<streamsize>::max(), '\n');
 continue;
 }
 cin.ignore(numeric_limits<streamsize>::max(), '\n');
 
 if (validateInput(num))
 ;
 else
 {
 continue;
 }
 if (!placement(num, winning_num))
 {
 cout << "\nIncorrect!\n";
 ++tries;
 }
 else
 {
 cout << "\nCorrect!\n";
 exit(0);
 }
 }
 while (tries <= 7);
 cout << "\nYou did not guess the number in the alloted amount of guesses :(\nTry Again!\n";
}

• sɪʒɪhɪŋ βɪstɦa kxɐll: else if (!input) return false; else return false; It should be return false; after the if and that's all, both else return the same value and the if returns a value so you can omit the else and go return instead.
• sɪʒɪhɪŋ βɪstɦa kxɐll: Also, in the first function it is unnecessary to place using namespace std since you aren't calling anything within std
• sɪʒɪhɪŋ βɪstɦa kxɐll: if (validateInput(num)); else { continue; } is equivalent to write if (!validateInput(num)) continue; because ! is the not operator. It inverts true to false and false to true
• unkn0wn.dev: @Miguel Avila Yeah that is a good point, I should’ve thought of that. Thanks for pointing it out.
• Mast: Please do not update the code in your question after receiving answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .

Answer 1

I like that your code (read functions) follows Single Responsibility Principle.

And also that you've put the number comparison code in a separate function unlike

• User guesses the number generated by computer
• Computer tries to guess your inputted number

And also that you use static_cast. All are good coding practices!

---

I'd prefer if you do the text printing in the placement function itself and call it something like compare_with_guess. That way, you don't have to return values and use another if-else block in the caller. Also, the output messages can also be controlled all at one place.

Another way is to use struct like:

struct InputNum{
private:
 const int input;
public:
 InputNum(const int num):input(num){};
 int get() const{
 return input;
 }
 bool greater_than(const int target) const {
 return input > target;
 }
 bool less_than(const int target) const {
 return input < target;
 }
 bool equals(const int target) const {
 return input == target;
 }
}

---

Use const whenever possible.

const int winning_num{ generateRandomNumber() };// +1 for {} initialiser syntax. 

void compare_with_guess(const int input, const int guess){ // After making the changes I suggested above.

bool validateInput(const int input){

---

Avoid using namespace std; completely, let alone using it multiple times in the code.

https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice

---

• unkn0wn.dev: Thank you for your feedback this helps me very much. The only parts of your answer I do not understand is the line right after public in the struct, and the const outside the function curly brackets. If you could explain those it would be much appreciate.
• aki: @unkn0wn.dev When you pass an const or a const & etc to a function, you can only call its member functions that are marked const. It indicates that calling those functions will not modify the data in the struct. ideone.com/qjMS6K this won't compile since equals is not a const function (sorry I need to lookup the correct term for that) while the num is const&.
• aki: It is the constructor docs.microsoft.com/en-us/cpp/cpp/… It takes num which is const since we only read it and then initialises input equals to num. in the {}, you can add a std::cout to print both of them.
• unkn0wn.dev: Thank you, I understand it better now.
• unkn0wn.dev: Could you also explain the advantages of using a struct instead of a class for this instance?

Answer 2

Well Your code is working fine but there were some extra code which I have removed and now its like this working same as before.
You can have more simple code if you use using namespace std; at the top of you document.

#include <iostream>
#include <limits>
#include <random>
using namespace std;
bool validateInput(int input)
{
 if (1 <= input && input <= 100)
 return true;
 else
 return false;
}
int generateRandomNumber()
{
 random_device dev;
 mt19937_64 range(dev());
 uniform_int_distribution<std::mt19937_64::result_type> generator(1, 100);
 static_cast<void>(generator(range));
 return static_cast<int>(generator(range));
}
bool placement(int input, int target)
{
 if (input > target)
 {
 cout << "Guess is too high.";
 return false;
 }
 else if (input < target)
 {
 cout << "Guess is too low.";
 return false;
 }
 else
 {
 cout << "Guess is correct!";
 return true;
 }
}
int main()
{
 bool end;
 cout << "You have 7 guesses to guess what number I am thinking of between 1 and 100.\n";
 int winning_num{ generateRandomNumber() };
 static int tries{ 1 };
 do
 {
 end=true;
 int num;
 cout << "\nGuess#" << tries << ": Enter a number -> ";
 cin >> num;
 
 if (validateInput(num))
 ;
 else
 {
 continue;
 }
 if (!placement(num, winning_num))
 {
 cout << "\nIncorrect!\n";
 ++tries;
 }
 else
 {
 cout << "\nCorrect!\n";
 exit(0);
 }
 if(tries>7)
 {
 cout<<"\nDo you want to guess more\nnPress y for it and \nPress n for closing\n";
 char guess;
 cin>>guess;
 switch(guess)
 {
 case 'y':
 {
 cout<<"\nOk Lets go\n";
 break;
 }
 case 'n':
 {
 cout<<"\nEnding the program\n";
 exit(0);
 }
 default:
 {
 cout<<"\nEnter right choice\n";
 }
 
 }
 }
 }while (end==true);
 cout << "\nYou did not guess the number in the alloted amount of guesses :(\nTry Again!\n";
}

You can also ask from user when he want to do more guesses or want to end the program making the program user friendly.

• aki: Please provide a review of the code, not only changes. How to Answer