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 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 2

I've seen this pattern before and didn't like it. Usually, it could be cleaned up by pulling the logic into a separate function.

The code then becomes

    ...
    int errorCode = doItAll();
    ...


    int doItAll(void) {
      int errorCode;
      if(errorCode=doSomething()!=0)
        return errorCode;
      if(errorCode=doSomethingElse()!=0)
        return errorCode;
      if(errorCode=doSomethingElseNew()!=0)
        return errorCode;
      return 0;
    }

Combining this with cleanup becomes pretty easy too, you just use a goto and error handler like in eclipses answer.

    ...
    int errorCode = doItAll();
    ...


    int doItAll(void) {
      int errorCode;
      void * aResource = NULL; // Somthing that needs cleanup after doSomethingElse has been called
      if(errorCode=doSomething()!=0) //Doesn't need cleanup
        return errorCode;
      if(errorCode=doSomethingElse(&aResource)!=0)
        goto cleanup;
      if(errorCode=doSomethingElseNew()!=0)
        goto cleanup;
      return 0;
    cleanup:
      releaseResource(aResource);
      return errorCode;
    }

Answer 3

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 4

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!

Answer 5

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 6

The second style is commonly used for managing resource allocations and de-allocations in C, where RAII doesn't come to the rescue. Typically, you would declare some resources before the do, allocate and use them inside the pseudo-loop, and de-allocate them outside.

An example of the general paradigm is as follows:

int status = 0;

// declare and initialize resources
BYTE *memory = NULL;
HANDLE file = INVALID_HANDLE_VALUE;
// etc...

do
{
  // do some work

  // allocate some resources
  file = CreateFile(...);
  if(file == INVALID_HANDLE_VALUE)
  {
    status = GetLastError();
    break;
  }

  // do some work with new resources

  // allocate more resources

  memory = malloc(...);
  if(memory == NULL)
  {
    status = ERROR_OUTOFMEMORY;
    break;
  }

  // do more work with new resources
} while(0);

// clean up the declared resources
if(file != INVALID_HANDLE_VALUE)
  CloseHandle(file);

if(memory != NULL)
  free(memory);

return status;

Having said that, RAII solves the same problem with much cleaner syntax (basically, you can forget the cleanup code altogether) and handles some scenarios that this approach does not, such as exceptions.