4
\$\begingroup\$

This is a program in C++ that guesses a number given by the user. I have to do homework and there's my attempt to solve an exercise from the programming book. If you know what can be done better and how to make more beautiful my code, let know me.

#include <iostream>

int main() {
    std::cout << "Enter number from range 0 to 100: ";
    int num;
    std::cin >> num;
    int guess = 50;
    bool lower = false;
    bool upper = false;
    int min = 0;
    int max = 100;
    while (guess != num) {
        if (num < guess) {
            std::cout << "Is this number lower than " << guess << "? (0 - false, 1 - true)\n";
            std::cin >> lower;
            if (lower) {
                upper = false;
                max = guess;
            }
        } else if (num > guess) {
            std::cout << "Is this number greater than " << guess << "? (0 - false, 1 - true)\n";
            std::cin >> upper;
            if (upper) {
                lower = false;
                min = guess;
            }
        } else {
            std::cout << "Let's guess, this number is equal to 50, isn't it?\n";
        }
        guess = (min+max)/2;
    }
    std::cout << "You entered a number " << guess << ", did I guess?\n";
    return 0;
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Overview:

You need to concentrate on writing self-documenting code. This basically means using self-documenting variable names and writing functions that do specific obvious self-documented tasks.

int main(int argc, char* argv[])
{
     // Handle command line arguments.
     // Set appropriate variables.

     playGuessNumberGame();  // Pass parameters you extracted from
                             // command line or environment.
}

void playGame()
{
     int  hiddenNumber = getNumberFromPlayer();
     bool finished     = false;

     while(!finished) {
         int guess = getNumberFromPlayer();  // Or calculate guess ?
         printHint(hiddenNumber, guess);

         finished = guess == hiddenNumber;
     }
}

Another thing I would think about is storing state in an object that is updated. In this case you have the guess that is updated based on whether it was lower or higher. I would wrap this logic into an object that understands how to use this data:

class Guesser
{
     int   min;
     int   max;
     int   currentGuess;
     public:
         Guesser(): min(0), max(100), currentGuess(50) {}
         int getBestGuess() const {return currentGuess;}
         void updateGuess(bool toLow) {
             if (toLow) {
                 int newGuess = currentGuess + (max - currentGuess)/2;
                 min = currentGuess;
                 currentGuess = newGuess;
             }
             else {
                 int newGuess = min + (currentGuess - min)/2;
                 max = currentGuess;
                 currentGuess = newGuess;
             }
              
         }
};

This way you isolate the variables that are associated with guessing, so they are not accidentally modified by external code.

Issues

There seems to be an issue with your tests. You are both testing the actual value then asking for user input to confirm what you actually know.

    if (num < guess) {
        // Why are you asking this question.
        // You already know that the num is less than guess.
        // Are you trying to automate the processes or trying to
        // get user input to direct the action?
        std::cout << "Is this number lower than " << guess << "? (0 - false, 1 - true)\n";
        std::cin >> lower;


        // We know it is lower.
        // but you only update things if the user confirms.
        // if you get it wrong does this upset the program?
        if (lower) {
            upper = false;
            max = guess;
        }

Code Review:

You don't check if the input is valid:

    int num;
    std::cin >> num;

When accepting user input, always validate that you got something valid. Otherwise you will end up with the application locking up or going into an infinite loop. In this case the stream simply sets itself as bad when invalid data is input, which can be detected with if ()

   if (std::cin >> num) {
        // Read worked we have a number in the variable.
   }

Note: This will validate things like 54XX as good, since the first part is a valid number. You may want to check the whole line to make sure it is just a number:

bool getValidNumberFromUser(int& num)
{
   std::string userInput;
   std::getline(std::cin, userInput);

   std::stringstream userInputStream(userInput);

   if (userInputStream >> num) {
        // Succesfully read a number.
        // but there should be no other trash on the line.
        // so let us validate that we got just a number.
        char garbage;
        if (userInputStream >> garbage) {
            // we got some garbage so there was an error on
            // user input. Lets return false from this function.
            return false;
        }
        // If we got here not garbage.
        return true;
   }
   // If we get here the user input did not even start with a number
   // So let us fail.
   return false;
}

bool isNumberOutOfRange(int number)
{
    return number < 0 || number > 100;
}


int getNumberFromPlayer()
{
    std::cout << "Enter number from 0 -> 100\n";

    int playerNumber;
    while(!getValidNumberFromUser(playerNumber) || isNumberOutOfRange(playerNumber)) {
        std::cout << "You entered an invalid number. Try Again\n";
    }
    return playerNumber;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.