You are still reading an
cin, and the only error case you are taking care of is entering a blank line. The normal reader will stop when it sees a non-digit, leaving the rest to be read later, even though the code waited here until Enter was pressed.
For example, if the user typed
25xyz it would happily return 25 and not find any error. You're making a big fuss out of closing the standard input handle (?!) and really only allowing him to enter blank lines before the real answer.
What such a function needs to do is read the whole line as a string, e.g. with
getline. Then parse the line and make sure it contains the number and nothing else as opposed to stopping when it sees a non-digit. The newer functions do that naturally and are faster, so use
from_chars to do the conversion.
There are several issues with the form of the function itself, too.
bool readNumber(int& value, const std::string failPrompt = "")
First, you are using an "out" parameter, which is one of the annoying issues with standard I/O and something we preach against in Code Reviews. We want the user to be able to write
const auto years = readNumber(); in the "nice" way that all variables should be defined.
Similarly, you are passing a
std::string by value ? You know that's a big beginner mistake. The default value of an empty string is created by calling the
const char* constructor which is inefficient since it handles a more general case. The proper, efficient and idiomatic way to specify an empty string is with
But, it is also best practice to pass this as a
string_view, so it can efficiently take a lexical string literal without copying it, or an existing
Your behavior of returning
false or however we decide to fail if there is no prompt given means that many many uses will use a canned generic fail string like the one in your example. For an interactive program, it should normally retry with a message explaining the input needed. I don't know how you can customize it meaningfully; nothing about the caller changes what input this function considers legal.
I'm thinking this is meant for use in simple interactive programming exercises and student code. So, it should be simple and does not need lots of configuration options and flexibility. It reads from standard input, period.
It should promote good programming practices, and not be "different" because I/O streams are different.
I imagine something like this:
const auto age = input<int>("What is your age in years?", between(1,999));
The prompt can be integrated into the same call, which is not only handy to use, but facilitates implementations that are fancier than just dumb TTY. The prompt can be repeated after an error, or the cursor repositioned in the correct spot, or it could be a pop-up form, etc.
Doing the validation in the same call is important because that is where it is doing the retry and not returning until it gets something it likes. If you know you got a valid number but still need to check the range, it makes the user code some kind of retry capability himself anyway.
Note that I used a named constraint rather than just two more parameters. This makes it clear what the numbers mean, and allows for more kinds of constraints. In fact,
between can be an object and this uses the same form as the more general one that takes a lambda.
Note that the resulting value is "do or die" and can be used to initialize a
There may be cases where entering something is optional, but that is not typical of these little problems, and should be done with a different call that uses a sum type like
optional. In fact, the template should be written to just take
optional<T> as being an empty string or the normal parsing work for
const auto old_score= input<std::optional<int>>("Enter your previous high score, if any:");
Sometimes you might have an existing value that can be used as a default, so a blank line takes that default. This could use a different function that takes an in/out parameter, say:
int block_size = 32768;
edit(block_size, "block size");
The function would automatically show the default value and format the prompt.
Or, it could use additional parameters to enable a default value:
const auto block_size = input<int>("block size", Default(32768));
But this has issues. The word
default is a reserved word already. Having multiple different optional arguments is a complex issue in itself that we can avoid. And, the prompt is formatted automatically with the default value and punctuation, so we want to give it a simple label rather than a sentence. That alone makes me thing it should be a different function.
Really, what I want for this kind of utility program, also suitable for student program exercises, is a whole form of input handled in a unit, not just one line at a time. These programs should normally take input via command line arguments, but prompt if run interactively. So, the arguments should be described once and used for both cases.
But a single-value BASIC-like
input statement that works better than standard input for interactive questions and can be used to initialize
const variables in the "best practices" manner would be far better than just letting the poor beginner get distracted by this before even getting to the real code.
Even if it's not great, by giving them this function we indicate that it's not their problem.