How to structure a loop that repeats until success and handles failures

Question

I am a self-taught programmer. I started programming about 1.5 years ago. Now I have started to have programming classes in school. We have had programming classes for 1/2 year and will have another 1/2 right now.

In the classes we are learning to program in C++ (which is a language that I already knew to use quite well before we started).

I have not had any difficulties during this class but there is one recurring problem that I have not been able to find a clear solution to.

The problem is like this (in Pseudocode):

 do something
 if something failed:
     handle the error
     try something (line 1) again
 else:
     we are done!

Here is an example in C++

The code prompts the user to input a number and does so until the input is valid. It uses cin.fail() to check if the input is invalid. When cin.fail() is true I have to call cin.clear() and cin.ignore() to be able to continue to get input from the stream.

I am aware that this code does not check of EOF. The programs we have written are not expected to do that.

Here is how I wrote the code in one of my assignments in school:

for (;;) {
    cout << ": ";
    cin >> input;
    if (cin.fail()) {
        cin.clear();
        cin.ignore(512, '\n');
        continue;
    }
    break;
}

My teacher said was that I should not be using break and continue like this. He suggested that I should use a regular while or do ... while loop instead.

It seems to me that using break and continue is the simplest way to represent this kind of loop. I actually thought about it for quite a while but did not really come up with a clearer solution.

I think that he wanted me to do something like this:

do {
    cout << ": ";
    cin >> input;
    bool fail = cin.fail();
    if (fail) {
        cin.clear();
        cin.ignore(512, '\n');
    }
} while (fail);

To me this version seems alot more complex since now we also have variable called fail to keep track of and the check for input failure is done twice instead of just once.

I also figured that I can write the code like this (abusing short circuit evaluation):

do {
    cout << ": ";
    cin >> input;
    if (fail) {
        cin.clear();
        cin.ignore(512, '\n');
    }
} while (cin.fail() && (cin.clear(), cin.ignore(512, '\n', true);

This version works exactly like the other ones. It does not use break or continue and the cin.fail() test is only done once. It does however not seem right to me to abuse the "short circuit evaluation rule" like this. I do not think my teacher would like it either.

This problem does not only apply to just cin.fail() checking. I have used break and continue like this for many other cases that involve repeating a set of code until a condition is met where something also has to be done if the condition is not met (like calling cin.clear() and cin.ignore(...) from the cin.fail() example).

I have kept using break and continue throughout the course and now my teacher has now stopped complaining about it.

What are your opinions about this?

Do you think my teacher is right?

Do you know a better way to represent this kind of problem?

Answer 1

I would write the if-statement slightly different, so it is taken when the input is successful.

for (;;) {
    cout << ": ";
    if (cin >> input)
        break;
    cin.clear();
    cin.ignore(512, '\n');
}

It's shorter as well.

Which suggests a shorter way that might be liked by your teacher:

cout << ": ";
while (!(cin >> input)) {
    cin.clear();
    cin.ignore(512, '\n');
    cout << ": ";
}

Answer 2

What you have to strive for is avoiding raw loops.

Move the complex logic into a helper function and suddenly things are a lot clearer:

bool getValidUserInput(string & input)
{
    cout << ": ";
    cin >> input;
    if (cin.fail()) {
        cin.clear();
        cin.ignore(512, '\n');
        return false;
    }
    return true;
}

int main() {
    string input;
    while (!getValidUserInput(input)) { 
        // We wait for a valid user input...
    }
}

Answer 3

It's not so much that for(;;) is bad. It's just not as clear as patterns like:

while (cin.fail()) {
    ...
}

Or as Sjoerd put it:

while (!(cin >> input)) {
    ...
}

Let's consider the primary audience for this stuff as being your fellow programmers, including the future version of yourself who no longer remembers why you stuck the break at the end like that, or jumped the break with the continue. This pattern from your example:

for (;;) {
    ...
    if (blah) {
        continue;
    }
    break;
}

...requires a couple seconds of extra thought to simulate versus other patterns. It's not a hard and fast rule, but jumping the break statement with the continue feels messy, or clever without being useful, and putting the break statement at the end of the loop, even if it works, is unusual. Normally both break and continue are used to prematurely evacuate the loop or that iteration of the loop, so seeing either of them at the end feels a little weird even if it's not.

Other than that, good stuff!

Answer 4

My logic instructor at school always said, and pounded it into my brain, that there should only be one entrance and one exit to loops. If not you start getting spaghetti code and not knowing where the code is going during debugging.

In real life I feel that code readability is really important so that if someone else needs to fix or debug an issue with your code it's easy for them to do.

As well if it's a performance issue because you are running through this look millions of times the for loop might be faster. Testing would answer that question.

I don't know C++ but I completely understood the first two of your code examples. I am assuming that continue goes to the end of the for loop and then loops it again. The while example I understood completely but for me it was easier to understand than your for(;;) example. The third one I would have to do some thought work to figure it out.

So going by for me which was easier to read (not knowing c++) and what my logic instructor said I would use the while loop one.

Answer 5

to me, the most straight forward C translation of the pseudocode is

do
    {
    success = something();
    if (success == FALSE)
        {
        handle_the_error();
        }
    } while (success == FALSE)
\\ moving on ...

i don't get why this obvious translation is a problem.

perhaps this:

while (!something())
    {
    handle_the_error();
    }

that looks simpler.