4
\$\begingroup\$

I'm just starting to learn C++ (not my first language), and started with working on a program to simulate the classic "Guess My Number" game, and am mostly looking for feedback on optimization and/or general tips. I wanted to write it to allow the player to either guess like normal, but also allow the player to choose and have the computer guess. The most interesting part about this, in my opinion anyway, is determining if the player is cheating, and this is also what I'm most interested in regarding feedback (but I'd welcome feedback on anything).

Another side-question is: should I use the enum's name when referring to the enum values, such as resultOption::CORRECT, or is just using CORRECT alright? I know I should never use using namespace std for clarity's sake.

#include <cstdlib>
#include <ctime>
#include <iostream>



enum mainOption { GUESSER = 1, CHOOSER_DUMB, CHOOSER_SMART, EXIT };


const int MIN_GUESS = 1;
const int MAX_GUESS = 100;
const std::string PLAY_AGAIN_MSG = "\nThat was fun! Let's play again!\n";


int mainMenu();
void playGuesser();
void playChooser(bool smart);



int main() {
    std::srand(static_cast<unsigned int>(time(0)));

    int option;
    do {
        option = mainMenu();

        switch (option) {
            case GUESSER:
                playGuesser();
                break;
            case CHOOSER_DUMB:
                playChooser(false);
                break;
            case CHOOSER_SMART:
                playChooser(true);
                break;
            case EXIT:
                std::cout << "We should do this again some time!\n\n";
                break;
            default:
                std::cout << "**ERROR**: Invalid menu option  {" << option << "}\n\n";
        }
    } while (option != EXIT);

    return EXIT_SUCCESS;
}



int mainMenu() {
    int option;


    std::cout << "  1. Play as guesser\n"
              << "  2. Play as chooser\n"
              << "  3. Play as chooser (against smartest guesses)\n"
              << "  4. Exit\n";

    do {
        std::cout << "Please choose a game mode to begin: ";
        std::cin >> option;

        if (option < 1 || option > EXIT) {
            std::cout << "Invalid option. ";
        }
    } while (option < 1 || option > EXIT);


    return option;
}


void playGuesser() {
    int number = (std::rand() % (MAX_GUESS - MIN_GUESS + 1)) + MIN_GUESS;
    int guess, guesses = 0;

    std::cout << "\nOkay, I'm ready!" << std::endl;

    do { //Repeat until number is guessed correctly
        std::cout << "Please pick an integer between " << MIN_GUESS << " and " << MAX_GUESS << ": ";
        std::cin >> guess;

        if (guess < MIN_GUESS || guess > MAX_GUESS) {
            std::cout << "Sorry, invalid guess. ";
        }
        else {
            guesses++;

            if (guess < number) {
                std::cout << "No, that's too low." << std::endl;
            }
            else if (guess > number) {
                std::cout << "Sorry, that's too high." << std::endl;
            }
            else {
                std::cout << "Congrats, you got it!! It only took " << guesses << " guess"
                          << ((guesses == 1) ? "" : "es") << "." << std::endl; //pluralize "guesses"
            }
        }
    } while (guess != number);

    std::cout << PLAY_AGAIN_MSG;
}

void playChooser(bool smart) {
    enum resultOption { HIGH = 1, LOW, CORRECT };

    int guess, guesses = 0;
    int min, max;
    int result;
    bool cheater = false;



    std::cout << "\nAlrighty, pick a number between " << MIN_GUESS << " and " << MAX_GUESS
              << " but don't tell me!" << std::endl
              << "Hit any key when you're ready..." << std::endl;
    std::cin.get();


    min = MIN_GUESS;
    max = MAX_GUESS;
    do { //until the guess is right or it's determined the player is cheating
        if (smart) {
            guess = (max + min) / 2;
        }
        else {
            guess = std::rand() % (max - min + 1) + min;
        }
        guesses++;

        std::cout << "  1. Too high"  << std::endl
                  << "  2. Too low"   << std::endl
                  << "  3. Correct!"  << std::endl;
        do {
            std::cout << "Is it " << guess << "? ";
            std::cin >> result;

            if (result < 1 || result > CORRECT) {
                std::cout << "Invalid option. ";
            }
        } while (result < 1 || result > CORRECT);

        if (result == CORRECT) {
            std::cout << "Awesome! I got it right in " << guesses << " guesses." << std::endl;
        }
        else if (result == HIGH || result == LOW) {
            if (max == min) { //If it's the only option left, it can't be wrong.
                cheater = true;
            }
            else if (result == HIGH) {
                if (guess == min) { //It can't be lower than the min.
                    cheater = true;
                }
                else {
                    max = guess - 1;
                }
            }
            else if (result == LOW) {
                if (guess == max) { //It can't be higher than the max.
                    cheater = true;
                }
                else {
                    min = guess + 1;
                }
            }

            if (cheater) {
                std::cout << "Hmm, it seems like someone likes to cheat ;)" << std::endl
                          << "You had me going there for " << guesses << " guesses." << std::endl;
            }
        }
        else {
            std::cout << "**ERROR**: Invalid result. {" << result << "}" << std::endl;
        }

    } while (result != CORRECT && !cheater);


    std::cout << PLAY_AGAIN_MSG;
}
\$\endgroup\$
2
  • \$\begingroup\$ You could consider using interval halving instead when guessing, that way you substantially decrease the number of times you need to guess. \$\endgroup\$
    – AndersK
    Jan 14 '16 at 11:05
  • \$\begingroup\$ @AndersK. Yes, that's the difference between the 2nd and 3rd game mode. In the 2nd game mode, the computer chooses randomly inside the valid range, but in the 3rd game mode, the computer halves it with each guess by averaging the min and max. It's less fun to play against, I think, which is why I put in both modes. Also, in real life, most people guess more-or-less randomly inside the range. \$\endgroup\$
    – BrainFRZ
    Jan 14 '16 at 16:42
5
\$\begingroup\$

It looks like quite a bit of refactoring can be done (especially with all of the conditionals in the larger functions), but I'll start with some other points.

  • The random number of blank lines at the top is a little annoying. You really only need one between the different sections. You can also move main() at the very bottom to eliminate the need for the function prototypes.

  • Great work on having the seed generation done in main() only! Many people seem to do that incorrectly.

    Now, if you have access to C++11, you should consider using the <random> library instead for random number generation. There are many different ways of using it, which would depend on your application.

  • This is probably a little picky, but you can make your constants constexpr instead (also available only in C++11). This will make them compile-time defined.

  • I would try to keep all the menu work in separate functions and just have them called from main(). It's probably not bad to keep them as is, but I like to try to keep main() as simple as possible.

  • One might assume that a function called mainMenu() would be void and just display the menu options. Since your function is returning a value, you can rename it to something like getMainMenuOption().

\$\endgroup\$
2
\$\begingroup\$
  1. Avoid passing bool as a parameter. It's not immediately apparent how playChooser(false) differs from playChooser(true). Much better to define an enum that specifies the real meaning: enum { DUMB, SMART}; A call of playChooser(SMART) or playChooser(DUMB) is much more informative to the reader.
  2. The code:

    if (result == CORRECT)
    // ...
    else if (result == HIGH || result == LOW) 
    

    ...is redundant. You've already assured that the only values that can be assigned to result are LOW, HIGH or CORRECT.

  3. To help that, however, you might consider moving the code to get a choice from the user into a function, and add an assert (or similar) to its end that verifies the logic so it can only ever produce the correct values:

    resultOption get_guess() {
        std::cout << "  1. Too high" << std::endl
            << "  2. Too low" << std::endl
            << "  3. Correct!" << std::endl;
        do {
            std::cout << "Is it " << guess << "? ";
            std::cin >> result;
    
            if (result < 1 || result > CORRECT) {
                std::cout << "Invalid option. ";
            }
        } while (result < 1 || result > CORRECT);
    
        assert(result == CORRECT || result == LOW || result == HIGH);
        return result;
    }
    

    Alternatively, consider using enum class enumOptions, which makes it rather more difficult to accidentally assign some random number instead of one of the values given in the enumeration.

  4. I think I'd move the logic for determining whether the user is cheating into its own function as well. This has little relationship to the rest of the logic, so it makes sense to segregate it. Cheating is a sufficiently unexpected result that it might make sense to have that function throw an exception. Then we can have logic at a higher level that explicitly decides what to do when/if the user cheats. Right now, its reaction to that is not immediately apparent (I think it considers that the end of the current game, then offers to play another game, but I'd just about have to test it to be entirely certain).

\$\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.