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 withstd::cin.fail()
). Did I miss anything? Also, is there really a need to type outstd::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, likefunction_snake_case()
andconstantCamelCase
.Should I make parameters
const
? From what I recall, unless there's some compelling reason to make themconst
, you shouldn't. However, I'm new to the language, so please correct me if I'm wrong.
2 Answers 2
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?
-
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– Daniel2018年04月23日 22:11:02 +00:00Commented Apr 23, 2018 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– yuri2018年04月23日 22:20:45 +00:00Commented Apr 23, 2018 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– Daniel2018年04月23日 22:24:33 +00:00Commented Apr 23, 2018 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– Konrad Rudolph2018年04月24日 10:18:00 +00:00Commented Apr 24, 2018 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– Konrad Rudolph2018年04月24日 10:23:08 +00:00Commented Apr 24, 2018 at 10:23
<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 usestd::purr
. If you include<meow.h>
, then you should usepurr
.
Explore related questions
See similar questions with these tags.