6
\$\begingroup\$

Challenge

This is my first 'big' project in C++. (Another number-guessing game, yay!) It's a slightly modified version of the learncpp.com quiz I set as a challenge for myself. The original challenge statement reads:

Implement a game of hi-lo. First, your program should pick a random integer between 1 and 100. The user is given 7 tries to guess the number.

If the user does not guess the correct number, the program should tell them whether they guessed too high or too low. If the user guesses the right number, the program should tell them they won. If they run out of guesses, the program should tell them they lost, and what the correct number is. At the end of the game, the user should be asked if they want to play again. If the user doesn’t enter ‘y’ or ‘n’, ask them again.

Note: You do not need to implement error handling for the user’s guess.

Objectives

I set some additional objectives:

  • Write modern, C++11 / C++14-compliant code;

  • Add some basic documentation, explaining the purpose of functions and their parameters;

  • Do basic error handling for all user input. This is actually part two of the quiz.

Code

main.cpp:

#include <ios> // `std::streamsize`
#include <iostream> // `std::cin`, `std::cout`
#include <limits> // `std::numeric_limits`
#include <cstdlib> // `rand()`, `srand()`
#include <ctime> // `time()`

/* Get a random int in the range [min, max] (max inclusive). */
// CSRE: Take note: not my code, so don't review this function, please :)
int getRandomInt(int min, int max) {
    // source:
    // http://www.learncpp.com/cpp-tutorial/59-random-number-generation/
    static const double fraction = 1.0 / (RAND_MAX + 1.0);
    return min + static_cast<int>((max - min + 1) * (rand() * fraction));
}

/* Get the user's next guess. `try_` is the try count. */
int getUserGuess(int try_) {
    int guess;
    while (true) {
        std::cout << "Guess # " << try_ << ": ";
        std::cin >> guess;
        if (std::cin.fail()) {
            // extraction failure
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        } else {
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            return guess;
        }
    }
}

/* Play the number guessing game. Return WON_GAME if the user wins the
game, LOST_GAME otherwise. `min` and `max` are the lower and upper
bounds for the randomly generated number, respectively. `tries` is the
amount of tries the user gets before having lost the game. */
void playNumberGuessingGame(int min, int max, int tries) {
    std::cout << "Let's play a game. I'm thinking of a number. ";
    std::cout << "You have " << tries << " tries to guess what it is.\n";
    int number = getRandomInt(min, max);
    for (int try_ = 0; try_ < tries; try_++) {
        int guess = getUserGuess(try_);
        if (guess == number) {
            std::cout << "Correct! You win!\n";
            return;
        } else if (guess < number) {
            std::cout << "Your guess is too low.\n";
        } else {
            std::cout << "Your guess is too high.\n";
        }
    }
    // if we fall through, the user has lost :(
    std::cout << "Sorry, you lose. The correct number was " << number << '\n';
}

/* Ask the user if they want to play again. */
bool playAgain() {
    char response;
    while (true) {
        std::cout << "Would you like to play again (y/n)? ";
        std::cin >> response;
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        if (response == 'y') {
            return true;
        } else if (response == 'n') {
            return false;
        }
    }
}

int main() {
    constexpr int MIN = 0;
    constexpr int MAX = 99;
    constexpr int TRIES = 7;

    srand(time(0));
    rand();

    do {
        playNumberGuessingGame(MIN, MAX, TRIES);
    } while (playAgain());

    std::cout << "Thank you for playing.\n";
}

Compilation

I compiled my code with the following command:

g++ -std=c++14 -Wall -Werror -Wextra -Wpedantic -o main main.cpp

Replace g++ with your compiler of choice.

Concerns

I normally only write high-level code. The code I've written here feels like an awkward translation from Python to C++. These are my main concerns / questions:

  • Is there some default format for C++ documentation? For example, in Python, there's docstrings, including a standard (PEP-257).

  • How did I do on error handling? I just implemented what I learned here (calling std::cin.ignore() and checking for extraction failure with std::cin.fail()). Did I miss anything? Also, is there really a need to type out std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');, or is it worth extracting that into a function of its own?

  • How did I do on code style? What are the rules regarding naming conventions in C++? The most common scheme I've seen is functionCamelCase(), ClassPascalCase, CONSTANT_UPPERCASE, MACROUPPERCASE, but I've seen variations, like function_snake_case() and constantCamelCase.

  • Should I make parameters const? From what I recall, unless there's some compelling reason to make them const, you shouldn't. However, I'm new to the language, so please correct me if I'm wrong.

\$\endgroup\$
6
\$\begingroup\$

Overall quite well done. You avoided many of the common pitfalls beginners tend to make.
There is no official C++ coding style and depending on who you ask you get different answers on which one is the best. That being said, your style is completely acceptable.

Even against your explicit wish the getRandomInt function has to be reviewd.
In modern C++ you should not rely on rand and friends for various reasons. Instead prefer to use the functions provided by the STL.

Regarding your comments.
From what I've seen docstring type of comments are unusal and discouraged. Usually people strife to write self-documenting code and only write comments for tricky parts that really need explaining.
In your case you can actually apply this as well. Take for example your getUserGuess function. It has a comment stating "/* Get the user's next guess. try_ is the try count. */". The first half of the comment is not necessary because of the function name. The second part can be eliminated if you rename try_ to try_count. Now the code documents itself and your comment is no longer needed.
The problem with comments is that they need to be maintained just like regular code and people tend to not maintain comments leading to hard to read code over time. As another example look at playNumberGuessingGame. It has a lengthy comment explaining that the function returns WON_GAME and LOST_GAME. Based on your own findings this would indicate the function returns some sort of constant (maybe an enum value). However it returns nothing at all! So your comment actually does more bad than good. Also a part of it can be avoided by renaming tries to max_tries, making the intent much clearer.

Another small thing is to prefer the prefix (++foo) over the postfix (foo++) operator as that will avoid an extra copy when dealing with iterators and such. It's a good habit to get into.

While your choice of infinity loop is certainly valid it's worth pointing out that there are other choices along with certain drawbacks that one should be aware of.

Regarding your design, have you considered putting this into a class to group functionality together instead of having free floating functions?

\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks. My bad on the comment, it was outdated (I guess that demonstrates your point perfectly). I know C++ is all about OOP, but there is no shared state here, so I don't know if it makes sense to create a class. \$\endgroup\$ – Daniel Apr 23 '18 at 22:11
  • \$\begingroup\$ @Coal_ If nothing else it might help you get acquainted with classes as they have their own set of gotchas to be aware of. \$\endgroup\$ – yuri Apr 23 '18 at 22:20
  • \$\begingroup\$ I'll be sure to ask for people to review more complicated projects in which I do use classes, later. \$\endgroup\$ – Daniel Apr 23 '18 at 22:24
  • \$\begingroup\$ I’d like to upvote this answer but I can’t endorse your comments about docstrings, since they’re wrong. Docstrings are not discouraged. Superfluous comments are, but documenting an API (= public functions) is virtually universally recommended. I agree with your specific example: the docstrings could be written better (in particular, it fails to document the function’s side-effects and return value). But it should be there. \$\endgroup\$ – Konrad Rudolph Apr 24 '18 at 10:18
  • \$\begingroup\$ Your last paragraph also goes against the general recommendation in modern C++: do not attempt to write Java-style OOP in C++. It’s cargo cult programming. There’s nothing wrong with free functions in C++. Using member functions in this context doesn’t quite make sense. Having a class to represent the game is a different matter entirely, and arguably does make sense, although not a lot since there’s very little shared state (hmm … the random number generator maybe?). \$\endgroup\$ – Konrad Rudolph Apr 24 '18 at 10:23
4
\$\begingroup\$
  • <iostream> already includes <ios>.
  • getRandomInt is biased. Well, rand is also horrible.
  • It is perhaps more idiomatic to use the >> expression directly as the condition:

    if(std::cin >> guess) {
        // succeeded
    }
    else {
        // failed
    }
    
  • If you include <cmeow>, you should use std::purr. If you include <meow.h>, then you should use purr.
\$\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.