namespace std
Others have said this already but it's important enough to be repeated: Don't put your own definitions into namespace std
. It's undefined behavior.
About the only thing I'm aware of that you may put into namespace std
are specializations for templates already defined by the standard. So, for example, if you have
struct MyType
{
int a;
};
inline constexpr bool
operator==(const MyType& lhs, const MyType& rhs) noexcept
{
return lhs.a == rhs.a;
}
then you may do
#include <functional> // std::hash
namespace std
{
template <>
struct hash<MyType>
{
using argument_type = MyType;
using result_type = std::size_t;
result_type
operator()(const argument_type& mt) const noexcept
{
return mt.a;
}
};
}
so you can, say, use MyType
as key in a std::unordered_map
.
The whole point of namespace
s is to separate stuff. So put your own stuff into your own namespace
. For example, Boost has its stuff in namespace boost
and sub-namespace
s thereof. You could use namespace ethan_bierlein
or something. In other languages, it is common practice to use one's domain as in package com.example.myproduct
(assuming you own example.com
) but I haven't seen this practice in C++ yet.
Correctness
Consider the following program.
int
main()
{
const auto age = easy_input<int>("How old are you? ");
std::cout << "Hello, " << age << " year old!\n";
}
If compiled and run as this
$ ./a.out
How old are you? 17
Hello, 17 year old!
everything seems fine. However, if the user inputs nonsense,
$ ./a.out
How old are you? don't care
Hello, 0 year old!
$
the result is probably not what you'd expect from a well-designed user interface. However, it gets worse.
int
main()
{
const auto ounces = easy_input<int>("How many ounces of beer dou you want? ");
const auto age = easy_input<int>("How old are you? ");
//std::cout << "ounces = " << ounces << ", age = " << age << "\n";
if (age >= 18)
std::cout << "Here are your " << ounces << " ounces of beer.\n";
else
std::cout << "Sorry, you're not old enough.\n";
}
In action:
$ ./a.out
How many ounces of beer dou you want? too many
How old are you? Here are your 0 ounces of beer.
$
Don't worry if you cannot reproduce the results of the above two examples result of the second example. It's undefined behavior so anything (including but not limited to your cat getting pregnant) could happen.
What's the reason for this?
There are two problems. First, in
TInput user_input_value; // (1)
std::cout << prompt; // (2)
std::cin >> user_input_value; // (3)
return user_input_value; // (4)
if TInput
is a builtin type like int
, the variable user_input_value
is not initialized on line 1. If the input on line 3 succeeds, the value is set to the input, which is fine. However, if the input is invalid, no value will be assigned and you'll return the uninitialized value. If the stream is good()
, the extraction operator on line 3 will either successfully extract and assign the value or, if invalid input is given, set the failbit
and assign 0. Therefore, if the first input is invalid, 0 will be returned (for ounces
) and the failbit
set. Then, on the second entry, nothing is assigned to user_input_value
and an uninitialized int
returned (for age
). This results in undefined behavior.
Running the above examples second (“too many ounces of beer”) example through a tool like Valgrind can unveil the error. (To my surprise, neither ASan nor UbSan were able to detect the error.)
I originally thought that if input fails for whatever reason, the destination value would never be changed. This seemed to be the case but apparently was changed in C++11 such that now 0 is assigned for invalid input provided the stream was good()
to begin with. Thanks to Mooing Duck for discovering this (see comments).
The seemingly simple fix to this problem is to use value-initialization for user_input_variable
.
TInput user_input_value {};
Since your question is tagged with C++14, we can now at least use one C++11 feature (uniform initialization) with pride.
However, while this “solution” fixes the undefined behavior, it still has its issues. If the user enters invalid input, it should probably be yelled at and not 0 (or whatever a default-constructed TInput
is) returned silently. So you should really check the stream after the operation.
TInput user_input_value {}; // value-initialization not strictly needed any more
std::cout << prompt;
if (std::cin >> user_input_value)
return user_input_value;
throw std::istream::failure {"bad user input"};
You could have the same effect by setting the exceptions
mask of std::stdin
but this would also affect other uses of std::cin
even outside your function so it might surprise users of your function. I'd consider a utility function messing with the mask bad.
Some people will argue that a user inputting invalid data is by no means an “exceptional” event so throwing an exception is inappropriate. If you're like them, you might prefer returning an std::experimental::optional<TInput>
result. Unfortunately, it is not standard yet but many implementations support it and there is a readily available version in Boost.Optional.
But this is still not as useful as it could be. Consider the “beer” example again.
$ ./a.out
How many ounces of beer dou you want? 4 2
How old are you? Sorry, you're not old enough.
$
The first invocation of your function happily consumes the 4
, stops at the white-space and leaves the 2
on the stream. The second invocation then sees the 2
readily available and without waiting for any user input, immediately and equally happily returns 2.
cr_oag has suggested you address this by calling std::cin.ignore
. However, I don't think that this is the ideal solution. If you ask the user how many ounces of beer she wants and she inputs 4 2
, it is unlikely she really meant 4. It would be better to treat this as an error and ask for clarification.
There is another, closely related, problem with your current version. Consider how this seemingly reasonable program…
int
main()
{
const auto name = easy_input<std::string>("What's your name? ");
std::cout << "Hello, " << name << "!\n";
}
…behaves not quite as expected.
$ ./a.out
What's your name? Ethan Bierlein
Hello, Ethan!
$
While it might create a warm and cozy feeling to be called with the first name by the program, most of us would probably consider this a bug.
For least surprise, I recommend that you always treat user input line-oriented: Read a single, but complete, line, and treat its entirety as one input. You can easily do this via std::getline
. You can then try to convert that string into the target type and see if it has any characters remaining.
std::cout << prompt;
std::string line {};
if (!std::getline(std::cin, line))
throw std::istream::failure {"I/O error"};
std::istringstream iss {line};
TInput value {};
if (!(iss >> value) || !iss.eof())
throw std::istream::failure {"bad user input"};
return value;
This is still not ideal. While it behaves as expected for, say, numbers, the example asking for a name would now reject any input longer than one word. Not quite what we want. You would need some template meta-programming to special-case string input. But explaining how that would work is too much to go through in a code review.
As a matter of fact, there is already a function template in Boost.Lexical_Cast that does this (and more) for you (#include <boost/lexical_cast.hpp>
).
std::string line {};
std::cout << prompt;
if (!std::getline(std::cin, line))
throw std::istream::failure {"I/O error"};
return boost::lexical_cast<TInput>(line);
Portability
user86418 has already mentioned that #pragma once
is not portable but if you use it, the #include
guards become redundant. On the other hand, #pragma once
can be more efficient than #include
guards because the preprocessor can immediately stop processing the file while with #include
guards, it has to process the whole header anyway to find out where the matching #endif
is. So if you want the best of both worlds, you could use something like this.
#if HAVE_PRAGMA_ONCE
#pragma once
#endif
#ifndef EASY_INPUT_H
#define EASY_INPUT_H
// ...
#endif
Your users could then compile with -DHAVE_PRAGMA_ONCE
to eventually speed up their compilations while still being portable to implementations that don't have #pragma once
. If you're used to using GNU Autoconf, you'll be very familiar with those HAVE_${FEATURE}
macros.
Another option is to use tricks like
#ifdef __GNUC__
#pragma once
#endif
(GCC defines __GNUC__
) but I don't like this kind of being clever very much and prefer having the user have the last word.
As Mooing Duck points out, compilers ought to ignore unknown #pragma
s. However, it is still a good idea to make them conditional. For example, if you compile with -Werror=unknown-pragmas
(And you should, because it is enabled by -Wall -Werror
.), GCC will reject code with unknown #pragma
s. This is conforming as in standard configuration, GCC ignores them gracefully as it is supposed to. Your library should not force your users to use less rigorous warning levels. Such “noisy” or “unclean” libraries are very annoying, to say the least.
Genericity
Others have suggested making the stream to read from a parameter or accepting other prompt
s than strings. I don't think that this would add much value as I've yet to see a program that needs to read interactive user input from anything but standard input. Some lost souls might want to read from std::wcin
, though. Those would probably also be interested in a std::wstring
prompt
that would be streamed to std::wcout
. Eventually, there could be good reasons to print the prompt
to standard error output (for example if standard output is to be redirected to some file).
Having an overload that uses no (or a default) prompts seems useful but anything else I think is superfluous. A default parameter comes in handy here.
template <typename TInput>
TInput easy_input(const std::string& prompt = "");
Yes, you'll needlessly construct a temporary std::string
if you call the function with a string literal but so what (see next section)? And I don't think that using, say, an int
as prompt
would be a very frequent use-case.
Performance
Is there anything that can be improved performance-wise?
Maybe, but don't worry about it. You are asking for user input. It would take very bad code for an input function that would be slow enough to make any difference compared to how long humans need to read, think and type.
Going Fancy
If you want to make your program fancy, consider adding line-editing support. It is very annoying for a user, having to delete an almost completely typed answer only because you realize that you have a typo in the first few characters. The GNU Readline library provides a de-facto standard mechanism for this. With GNU Readline there also goes GNU History which allows a user to re-fetch a previous answer using the arrow-keys. This is very handy if you have to give the same answer or similar answers many times. Of course, you should make the feature optional because your users might not have installed the library.
Also note the license requirements; quoting the project website:
Readline is free software, distributed under the terms of the GNU General Public License, version 3. This means that if you want to use Readline in a program that you release or distribute to anyone, the program must be free software and have a GPL-compatible license.
If you don't publish your software at all, you're also fine. So you can always experiment around freely for your personal learning and private usage.
There might be other libraries providing similar line-editing support but I don't know any of them.
Williham Totland commented that there is another free software library for line-editing, Linenoise. While checking its website, I also found the Editline Library (libedit) (also free software). I've never used either of them.
Style
Besides the fact that you shouldn't declare stuff in namespace std
, you have put the DocString alongside with the implementation. If you decide to separate declaration and definition, the DocString should go with the declaration because that's what is important for your users.
I assume you wanted to write a DocString that can be processed by Doxygen. If so, note that you should use @tparam
to document template arguments.
std::getline(std::cin, string)
? \$\endgroup\$