Are do-while-false loops common?

Question

A while back I switched the way I handled c style errors.

I found a lot of my code looked like this:

int errorCode = 0;

errorCode = doSomething();
if (errorCode == 0)
{
   errorCode = doSomethingElse();
}

...

if (errorCode == 0)
{
   errorCode = doSomethingElseNew();
}

But recently I've been writing it like this:

int errorCode = 0;

do
{       
   if (doSomething() != 0) break;
   if (doSomethingElse() != 0) break;
   ...
   if (doSomethingElseNew() != 0) break;
 } while(false);

I've seen a lot of code where nothing gets executed after there's an error, but it has always been written in the first style. Is there anyone else who uses this style, and if you don't, why?

edit: just to clarify, usually this construct uses errno otherwise I will assign the value to an int before breaking. Also there's usually more code than just a single function call within the "if (error == 0 )" clauses. Lots of good points to think on, though.

Answer 1

The second snippet just looks wrong. You're effectively re-invented goto.

Anyone reading the first code style will immediately know what's happening, the second style requires more examination, thus makes maintenance harder in the long run, for no real benefit.

Edit, in the second style, you've thrown away the error code, so you can't take any corrective action or display an informative message, log something useful etc....

Answer 2

I think the first one gives you more control over what to do with a particular error. The second way only tells you that an error occurred, not where or what it was.

Of course, exceptions are superior to both...

Answer 3

The first style is a pattern the experienced eye groks at once.

The second requires more thought - you look at it and see a loop. You expect several iterations, but as you read through it, this mental model gets shattered...

Sure, it may work, but programming languages aren't just a way to tell a computer what to do, they are a way to communicate those ideas to other humans too.

Answer 4

This should be done through exceptions, at least if the C++ tag is correct. There is nothing wrong if you are using C only, although I suggest to use a Boolean instead as you are not using the returned error code. You don't have to type != 0 either then...

Answer 5

Why not replace the do/while and break with a function and returns instead?

You have reinvented goto.

Answer 6

I've used the technique in a few places (so you aren't the only one who does it). However, I don't do it as a general rule, and I have mixed feelings about it where I have used it. Used with careful documentation (comments) in a few places, I'm OK with it. Used everywhere - no, generally not a good idea.

Relevant exhibits: files sqlstmt.ec, upload.ec, reload.ec from SQLCMD source code (not, not Microsoft's impostor; mine). The '.ec' extension means that the file contains ESQL/C - Embedded SQL in C which is pre-processed to plain C; you don't need to know ESQL/C to see the loop structures. The loops are all labelled with:

    /* This is a one-cycle loop that simplifies error handling */

Answer 7

Make it short, compact, and easy to quickly read?

How about:

if ((errorcode = doSomething()) == 0
&&  (errorcode = doSomethingElse()) == 0
&&  (errorcode = doSomethingElseNew()) == 0)
    maybe_something_here;
return errorcode; // or whatever is next

Answer 8

If you're using C++, just use exceptions. If you're using C, the first style works great. But if you really do want the second style, just use gotos - this is exactly the type of situation where gotos really are the clearest construct.

    int errorCode = 0;

    if ((errorCode = doSomething()) != 0) goto errorHandler;
    if ((errorCode = doSomethingElse()) != 0) goto errorHandler;
      ...
    if ((errorCode = doSomethingElseNew()) != 0) goto errorHandler;

    return;
errorHandler:
    // handle error

Yes gotos can be bad, and exceptions, or explicit error handling after each call may be better, but gotos are much better than co-opting another construct to try and simulate them poorly. Using gotos also makes it trivial to add another error handler for a specific error:

    int errorCode = 0;

    if ((errorCode = doSomething()) != 0) goto errorHandler;
    if ((errorCode = doSomethingElse()) != 0) goto errorHandler;
      ...
    if ((errorCode = doSomethingElseNew()) != 0) goto errorHandlerSomethingElseNew;

    return;
errorHandler:
    // handle error
    return;
errorHandlerSomethingElseNew:
    // handle error
    return;

Or if the error handling is more of the "unrolling/cleaning up what you've done" variety, you can structure it like this:

    int errorCode = 0;

    if ((errorCode = doSomething()) != 0) goto errorHandler;
    if ((errorCode = doSomethingElse()) != 0) goto errorHandler1;
      ...
    if ((errorCode = doSomethingElseNew()) != 0) goto errorHandler2;

errorHandler2:
    // clean up after doSomethingElseNew
errorHandler1:
    // clean up after doSomethingElse
errorHandler:
    // clean up after doSomething
    return errorCode;

This idiom gives you the advantage of not repeating your cleanup code (of course, if you're using C++, RAII will cover the cleanup code even more cleanly.

Answer 9

The classic C idiom is:

if( (error_val = doSomething()) == 0)
{ 
   //Manage error condition
}

Note that C returns the assigned value from an assignment, enabling a test to be performed. Often people will write:

if( ! ( error_val = doSomething()))

but I retained the == 0 for clarity.

Regarding your idioms...

Your first idiom is ok. Your second idiom is an abuse of the language and you should avoid it.

Answer 10

The first style is a good example of why exceptions are superior: You can't even see the algorithm because it's buried under explicit error handling.

The second style abuses a loop construct to mimic goto in a mislead attempt to avoid having to explicitly spell out goto. It's plainly evil and long-time usage will lead you to the dark side.

Answer 11

Your method isn't really bad and it's not unreadable like people here are claiming, but it is unconventional and will annoy some (as you noticed here).

The first one can get REALLY annoying after your code gets to a certain size because it has a lot of boilerplate.

The pattern I tended to use when I couldn't use exceptions was more like:

fn() {
    while(true) {
        if(doIt())
            handleError();//error detected...
    }
}

bool doIt() {
    if(!doThing1Succeeds())
        return true;
    if(!doThing2Succeeds())
        return true;
    return false;
}

Your second function should be inlined into the first if you put the correct magic incantations in the signature, and each function should be more readable.

This is functionally identical to the while/bail loop without the unconventional syntax (and also a bit easier to understand because you separate out the concerns of looping/error handling from the concerns of "what does your program do in a given loop".

Answer 12

For me, I'd prefer:

if(!doSomething()) {
    doSomethingElse();
}
doSomethingNew();

All of the other stuff is syntactic noise that is obscuring the three function calls. Inside of Else and New you can throw an error, or if older, use longjmp to go back to some earlier handling. Nice, clean and rather obvious.

Answer 13

There seems to be a deeper problem here than your control constructs. Why do you have such complex error control? I.e. you seem to have multiple ways of handling different errors.

Typically, when I get an error, I simply break off the operation, present an error message to the user, and return control to the event loop, if an interactive application. For batch, log the error, and either continue processing on the next data item or abort the processing.

This kind of control flow is easily handled with exceptions.

If you have to handle error numbers, then you can effectively simulate exceptions by continuing normal error processing in case of an error, or returning the first error. Your continued processing after an error occurs seems to be very fragile, or your error conditions are really control conditions not error conditions.

Answer 14

Honestly the more effective C/C++ programmers I've known would just use a gotos in such conditions. The general approach is to have a single exit label with all cleanup after it. Have only one return path from the function. When the cleanup logic starts to get complicated/have conditionals, then break the function into subfunctions. This is pretty typical for systems coding in C/C++ imo, where the APIs you call return error codes rather than throw exceptions.

In general, gotos are bad. Since the usage I've described is so common, done consistently I think its fine.

Answer 15

How about this version then

I'd usually just do something like your first example or possibly with a boolean like this:

bool statusOkay = true;

if (statusOkay)
    statusOkay = (doSomething() == 0);

if (statusOkay)
    statusOkay = (doSomethingElse() == 0);

if (statusOkay)
    statusOkay = (doSomethingElseNew() == 0);

But if you are really keen on the terseness of your second technique then you could consider this approach:

bool statusOkay = true;

statusOkay = statusOkay && (doSomething() == 0);
statusOkay = statusOkay && (doSomethingElse() == 0);
statusOkay = statusOkay && (doSomethingElseNew() == 0);

Just don't expect the maintenance programmers to thank you!