Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

Alternative Solution

###Alternative Solution HowHow about this?

Comments on original Code

###Comments on original Code ThingsThings that stand out in the original:

###Alternative Solution How about this?

###Comments on original Code Things that stand out in the original:

Alternative Solution

How about this?

Comments on original Code

Things that stand out in the original:

added 89 characters in body
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341
template<class T>
T input(T low, T high, std::string const& message)
{
 std::string line;
 forwhile (;;std::cin)
 {
 std::cout << message;
 std::getline(std::cin, line);
 std::stringstream lineStream(line);
 T result;
 char x;
 if (lineStream >> result && result >= low && result <= high && !(lineStream >> x))
 {
 // This body entered iff.
 // 1) We successfully read a T from the stream into `result`
 // 2) result >= low
 // 3) result <= hight
 // 4) There is only white space on the line after the result.
 return result;
 }
 }
 throw std::runtime_error("No more user input could not read a value");
}
template<class T>
T input(T low, T high, std::string const& message)
{
 std::string line;
 for(;;)
 {
 std::cout << message;
 std::getline(std::cin, line);
 std::stringstream lineStream(line);
 T result;
 char x;
 if (lineStream >> result && result >= low && result <= high && !(lineStream >> x))
 {
 // This body entered iff.
 // 1) We successfully read a T from the stream into `result`
 // 2) result >= low
 // 3) result <= hight
 // 4) There is only white space on the line after the result.
 return result;
 }
 }
}
template<class T>
T input(T low, T high, std::string const& message)
{
 std::string line;
 while (std::cin)
 {
 std::cout << message;
 std::getline(std::cin, line);
 std::stringstream lineStream(line);
 T result;
 char x;
 if (lineStream >> result && result >= low && result <= high && !(lineStream >> x))
 {
 // This body entered iff.
 // 1) We successfully read a T from the stream into `result`
 // 2) result >= low
 // 3) result <= hight
 // 4) There is only white space on the line after the result.
 return result;
 }
 }
 throw std::runtime_error("No more user input could not read a value");
}
added 1138 characters in body
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

How###Alternative Solution How about this?

I would also say it is the easiest to read out of the three solutions.

###Comments on original Code Things that stand out in the original:

while ((cin >> input).fail() || cin.peek() != '\n')

That call to fail() is superfluous as streams are automatically converted to a boolean (using good()) if the stream is used in a boolean context.

Also simply using fail() is a BUG. If you hit the end of input, then your code will go into an infinite loop. Mainly because fail() does not detect the end of input and operator>> will not update input correctly either. So not quite as bullet proof as you were hoping.

Also the use of peek() here seems a bit brittle. If the user typed in a valid input but stuck a space after it before the new line then your code would reject that users input. That's a pretty hard corner case to explain to a user especially if your error message is not very helpful.

In this chunk of code:

{
 cin.clear();
 cin.ignore(numeric_limits<streamsize>::max(), '\n');
 cout << message;
}
cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), '\n');

You seem to have the same bit of code repeated twice. That's never a good sign.

How about this?

I would also say it is the easiest to read out of the three solutions.

###Alternative Solution How about this?

I would also say it is the easiest to read out of the three solutions.

###Comments on original Code Things that stand out in the original:

while ((cin >> input).fail() || cin.peek() != '\n')

That call to fail() is superfluous as streams are automatically converted to a boolean (using good()) if the stream is used in a boolean context.

Also simply using fail() is a BUG. If you hit the end of input, then your code will go into an infinite loop. Mainly because fail() does not detect the end of input and operator>> will not update input correctly either. So not quite as bullet proof as you were hoping.

Also the use of peek() here seems a bit brittle. If the user typed in a valid input but stuck a space after it before the new line then your code would reject that users input. That's a pretty hard corner case to explain to a user especially if your error message is not very helpful.

In this chunk of code:

{
 cin.clear();
 cin.ignore(numeric_limits<streamsize>::max(), '\n');
 cout << message;
}
cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), '\n');

You seem to have the same bit of code repeated twice. That's never a good sign.

Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341
Loading
lang-cpp

AltStyle によって変換されたページ (->オリジナル) /