1

My C++ professor is adamant that when checking for input failure, one must use separate while() loops for each individual input. He indicated that the following method of checking more than one input gathered in a single cin statement would not work:

while (!good){
    cout << "Enter the length and the width of the rectangle: ";
    cin >> length >> width;
    if(!cin){
        cout << "Bad input, try again" << endl;
        cin.clear();
        cin.ignore(200, '\n');}
    else {
        good = true;}}

His proposed method:

bool good = false;
while (!good){
    cout << "Enter the length rectangle: ";
    cin >> length;
    if(!cin){
        cout << "Bad input, try again" << endl;
        cin.clear();
        cin.ignore(200, '\n');}
    else {
        good = true;}}  
while (good){
    cout << "Enter the width rectangle: ";
    cin >> width;
    if(!cin){
        cout << "Bad input, try again" << endl;
        cin.clear();
        cin.ignore(200, '\n');}
    else {
        good = false;}}

My method above seems to work just fine. If non-numerical characters are inputted for either of the inputs, the loop will prompt for new input and clear the fail-state.

Is it simply considered bad form to check for input failure as I have done? I'm hoping to learn more clearly why my method is erroneous. If my method is faulty, is there a way to check for input failure without using separate loops for every single user-input? I mainly ask because I'm writing a program that involves getting many inputs from a file, and individually checking each variable seems overly tedious. I'm thinking there must be a better way.

jguberman
  • 375
  • 1
  • 4
  • 14
  • Your professor needs to learn about this new syntax (only about 40 years old) called a do-while. – Ben Voigt Mar 24 '15 at 23:57
  • To closer with the greek pseudonym: Pay more attention, neither version of the code checks the `eof` flag. – Ben Voigt Mar 24 '15 at 23:59
  • You might find [C++ FAQ: How can I get std::cin to skip invalid input characters?](https://isocpp.org/wiki/faq/input-output#istream-and-ignore) useful (see the second code sample.) –  Mar 25 '15 at 00:02
  • @remyabel: Both variants already do that. – Ben Voigt Mar 25 '15 at 00:02
  • 1
    @BenVoigt Did you look at the link? I'm talking about reducing the size of the code. –  Mar 25 '15 at 00:04

2 Answers2

3

Both are "correct" in that they recover from failed inputs and give the user another chance.

Both are only useful for interactive input. You mention reading from a file -- there's really no recovery possible. Ignoring a mangled field will simply cause the next field to be consumed and interpreted differently from what its location in the file indicates. Best course of action when reading from a file is to output as good an explanation as possible (exactly where in the file the error occurred, e.g. line and columns numbers, what kind of data was expected, and what about the data encountered was invalid).

In interactive usage, clearing the buffer and prompting again is useful.. Your version has the disadvantage that after entering a length correctly, if the user fat-fingers the width, they can't just re-enter the wrong data, but have to type the length a second time as well.

Writing the loop a second time is incredibly pointless, however. Instead you should write a helper function, something like:

template<typename T>
T prompt_for_value( const char* const prompt )
{
    T result;
    while (true) {
         std::cout << prompt << std::flush;
         if (std::cin >> result) return result;
         std::cout << "Bad input, try again" << std::endl;
    }
}

double width = prompt_for_value<double>("Enter the width in meters: ");
double height = prompt_for_value<double>("Enter the height: ");

Notice that the code is shorter overall and avoids the clunky use of the good variable which inverted its meaning halfway through the original code. Also, the call site now is very clean and focuses completely on the important information -- the prompt and the data type of the input.

Thanks the C++11 lambda support, it's now also very easy to add parameter validation to the helper function:

T prompt_for_value( const char* const prompt, std::function<bool(T)> validation = {} )
{
    T result;
    while (true) {
         std::cout << prompt << std::flush;
         if (std::cin >> result) {
             if (validation && !validation(result)) {
                 std::cout << "Input out of range, try again" << std::endl;
                 continue;
             }
             return result;
         }
         std::cout << "Bad input, try again" << std::endl;
    }
}

double width = prompt_for_value<double>("Enter the width in meters: ",
                                        [](int w) { return w >= 0; });
double height = prompt_for_value<double>("Enter the height: ",
                     [&width](int h) { return (h >= 0) && (h <= 10000.0 / width); }));
Ben Voigt
  • 260,885
  • 36
  • 380
  • 671
  • @remyabel: So will the original. And I clearly said that this method of retrying is only useful for interactive input. And I hardcoded `cin` and `cout` to reduce the possibility of mistakenly using it for file input. Did you READ the answer? – Ben Voigt Mar 25 '15 at 00:14
0

What you have indicated as your professor's proposed method is very bad form as given due to violation of the don't-repeat-yourself (DRY) principle in the code. If you want to do basically the same thing multiple times, you should either use a loop or a function to avoid writing duplicate logic.

Your method involves all-or-nothing checking and therefore may require the user to repeat themselves. The usability of this should be decided on a case-by-case basis.

Both methods fail to detect extraneous characters at the end of the input line. Such input might reasonably be interpreted as erroneous -- regardless of whether or not it happens to work with the subsequent input request.

However, as you have demonstrated, the all-or-nothing approach does in fact basically work. Given that, here's an alternative all-or-nothing implementation based on the automatic exception-based checking that the iostream library is capable of.

   cin.exceptions( ~std::ios::goodbit );

   int length=0, width=0;

   for ( bool ok=false; !ok; )
      {
      try
         {

         cout << "Enter the length and the width of the rectangle: ";
         cin >> std::skipws >> length >> width;

         std::string s;
         std::getline( cin, s );
         s.erase( std::remove_if( s.begin(), s.end(), ::isspace ), s.end() );
         if ( !s.empty() ) throw std::runtime_error("too much input");

         ok = true;
         }
      catch( std::ios::failure const & e )
         {
         cout<<"\n"<<"bad input ["<<e.what()<<"], try again...\n";
         cin.clear();
         cin.ignore( std::numeric_limits<std::streamsize>::max(), '\n' );
         }
      catch( std::runtime_error const & e )
         {
         cout<<"\n"<<e.what()<<", try again...\n";
         }
      }

   cout<<"\n"<<"length="<<length<<", width="<<width<<"\n";

Output:

Enter the length and the width of the rectangle: x

bad input [basic_ios::clear], try again...
Enter the length and the width of the rectangle: 1 x

bad input [basic_ios::clear], try again...
Enter the length and the width of the rectangle: 1 2 x

too much input, try again...
Enter the length and the width of the rectangle: 1 2 3

too much input, try again...
Enter the length and the width of the rectangle: 4 5

length=4, width=5
Brent Bradburn
  • 40,766
  • 12
  • 126
  • 136
  • Checking for trailing characters is a decent idea, but the exception handling is adding complexity for no benefit whatsoever. Exceptions should be used to enable centralized error handling from several layers away in the call stack, not for local retry. – Ben Voigt Mar 25 '15 at 02:48
  • Using the iostreams library is complicated, and there is at least the possibility that turning on exceptions makes things simpler -- especially if you want to perform multiple I/O operations. Using exceptions gives me more confidence that if something goes wrong I will know about it. I don't see any harm in using exceptions here instead of manual checking -- certainly execution speed isn't a concern. My implementation has more lines of code, but mostly that is to support additional error-condition checking. – Brent Bradburn Mar 25 '15 at 06:46
  • I like to point out the exception mode of iostreams especially because, for file-based I/O, an all-or-nothing approach might be made *much* cleaner/simpler by bypassing manual error checking. I guess the comparable-complexity alternative would be to ignore all errors until you have completed the entire I/O sequence and *only then* ask the library if it actually worked. Maybe this is a reasonable approach, but to me it seems inelegant and error prone. – Brent Bradburn Mar 25 '15 at 13:17