What are some better ways to avoid the do-while(0); hack in C++?

When the code flow is like this:

if(check())
{
  ...
  ...
  if(check())
  {
    ...
    ...
    if(check())
    {
      ...
      ...
    }
  }
}

I have generally seen this work around to avoid the above messy code flow:

do {
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
} while(0);

What are some better ways that avoid this work-around/hack so that it becomes a higher-level (industry level) code?

Any suggestions which are out of the box are welcome!

It is considered acceptable practice to isolate these decisions in a function and use returns instead of breaks. While all these checks correspond to the same level of abstraction as of the function, it is quite logical approach.

For example:

void foo(...)
{
   if (!condition)
   {
      return;
   }
   ...
   if (!other condition)
   {
      return;
   }
   ...
   if (!another condition)
   {
      return;
   }
   ... 
   if (!yet another condition)
   {
      return;
   }
   ...
   // Some unconditional stuff       
}

There are times when using goto is actually the RIGHT answer - at least to those who are not brought up in the religious belief that "goto can never be the answer, no matter what the question is" - and this is one of those cases.

This code is using the hack of do { ... } while(0); for the sole purpose of dressing up a goto as a break. If you are going to use goto, then be open about it. It's no point in making the code HARDER to read.

A particular situation is just when you have a lot of code with quite complex conditions:

void func()
{
   setup of lots of stuff
   ...
   if (condition)
   {
      ... 
      ...
      if (!other condition)
      {
          ...
          if (another condition)
          {
              ... 
              if (yet another condition)
              {
                  ...
                  if (...)
                     ... 
              }
          }
      }
  .... 

  }
  finish up. 
}

It can actually make it CLEARER that the code is correct by not having such a complex logic.

void func()
{
   setup of lots of stuff
   ...
   if (!condition)
   {
      goto finish;
   }
   ... 
   ...
   if (other condition)
   {
      goto finish;
   }
   ...
   if (!another condition)
   {
      goto finish;
   }
   ... 
   if (!yet another condition)
   {
      goto finish;
   }
   ... 
   .... 
   if (...)
         ...    // No need to use goto here. 
 finish:
   finish up. 
}

Edit: To clarify, I'm by no means proposing the use of goto as a general solution. But there are cases where goto is a better solution than other solutions.

Imagine for example that we are collecting some data, and the different conditions being tested for are some sort of "this is the end of the data being collected" - which depends on some sort of "continue/end" markers that vary depending on where you are in the data stream.

Now, when we're done, we need to save the data to a file.

And yes, there are often other solutions that can provide a reasonable solution, but not always.

You can use a simple continuation pattern with a bool variable:

bool goOn;
if ((goOn = check0())) {
    ...
}
if (goOn && (goOn = check1())) {
    ...
}
if (goOn && (goOn = check2())) {
    ...
}
if (goOn && (goOn = check3())) {
    ...
}

This chain of execution will stop as soon as checkN returns a false. No further check...() calls would be performed due to short-circuiting of the && operator. Moreover, optimizing compilers are smart enough to recognize that setting goOn to false is a one-way street, and insert the missing goto end for you. As the result, the performance of the code above would be identical to that of a do/while(0), only without a painful blow to its readability.

1. Try to extract the code into a separate function (or perhaps more than one). Then return from the function if the check fails.

2. If it's too tightly coupled with the surrounding code to do that, and you can't find a way to reduce the coupling, look at the code after this block. Presumably, it cleans up some resources used by the function. Try to manage these resources using an RAII object; then replace each dodgy break with return (or throw, if that's more appropriate) and let the object's destructor clean up for you.

3. If the program flow is (necessarily) so squiggly that you really need a goto, then use that rather than giving it a weird disguise.

4. If you have coding rules that blindly forbid goto, and you really can't simplify the program flow, then you'll probably have to disguise it with your do hack.

TLDR: RAII, transactional code (only set results or return stuff when it is already computed) and exceptions.

Long answer:

In C, the best practice for this kind of code is to add an EXIT/CLEANUP/other label in the code, where cleanup of local resources happens and an error code (if any) is returned. This is best practice because it splits code naturally into initialization, computation, commit and return:

error_code_type c_to_refactor(result_type *r)
{
    error_code_type result = error_ok; //error_code_type/error_ok defd. elsewhere
    some_resource r1, r2; // , ...;
    if(error_ok != (result = computation1(&r1))) // Allocates local resources
        goto cleanup;
    if(error_ok != (result = computation2(&r2))) // Allocates local resources
        goto cleanup;
    // ...

    // Commit code: all operations succeeded
    *r = computed_value_n;
cleanup:
    free_resource1(r1);
    free_resource2(r2);
    return result;
}

In C, in most codebases, the if(error_ok != ... and goto code is usually hidden behind some convenience macros (RET(computation_result), ENSURE_SUCCESS(computation_result, return_code), etc.).

C++ offers extra tools over C:

• The cleanup block functionality can be implemented as RAII, meaning you no longer need the entire cleanup block and enabling client code to add early return statements.

• You throw whenever you cannot continue, transforming all the if(error_ok != ... into straight-forward calls.

Equivalent C++ code:

result_type cpp_code()
{
    raii_resource1 r1 = computation1();
    raii_resource2 r2 = computation2();
    // ...
    return computed_value_n;
}

This is best practice because:

• It is explicit (that is, while error handling is not explicit, the main flow of the algorithm is)

• It is straightforward to write client code

• It is minimal

• It is simple

• It has no repetitive code constructs

• It uses no macros

• It doesn't use weird do { ... } while(0) constructs

• It is reusable with minimal effort (that is, if I want to copy the call to computation2(); to a different function, I don't have to make sure I add a do { ... } while(0) in the new code, nor #define a goto wrapper macro and a cleanup label, nor anything else).

I'm adding an answer for the sake of completeness. A number of other answers pointed out that the large condition block could be split out into a separate function. But as was also pointed out a number of times is that this approach separates the conditional code from the original context. This is one reason that lambdas were added to the language in C++11. Using lambdas was suggested by others but no explicit sample was provided. I've put one in this answer. What strikes me is that it feels very similar to the do { } while(0) approach in many ways - and maybe that means it's still a goto in disguise....

earlier operations
...
[&]()->void {

	if (!check()) return;
	...
	...
	if (!check()) return;
	...
	...
	if (!check()) return;
	...
	...
}();
later operations

Certainly not the answer, but an answer (for the sake of completeness)

Instead of :

do {
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
} while(0);

You could write:

switch (0) {
case 0:
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
}

This is still a goto in disguise, but at least it's not a loop any more. Which means you won't have to very carefully check there is not some continue hidden somewhere in the block.

The construct is also simple enough that you can hope the compiler will optimize it away.

As suggested by @jamesdlin, you can even hide that behind a macro like

#define BLOC switch(0) case 0:

And use it like

BLOC {
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
}

This is possible because the C language syntax expect a statement after a switch, not a bracketed block and you can put a case label before that statement. Until now I didn't see the point of allowing that, but in this particular case it is handy to hide the switch behind a nice macro.

I would recommend an approach similar to Mats answer minus the unnecessary goto. Only put the conditional logic in the function. Any code that always runs should go before or after the function is invoked in the caller:

void main()
{
	//do stuff always
	func();
	//do other stuff always
}

void func()
{
	if (!condition)
		return;
	...
	if (!other condition)
		return;
	...
	if (!another condition)
		return;
	... 
	if (!yet another condition)
		return;
	...
}

The code flow itself is already a code smell that to much is happening in the function. If there is not a direct solution to that (the function is a general check function), then using RAII so you can return instead of jumping to the an end-section of the function might be better.

If you don't need to introduce local variables during the execution then you can often flatten this:

if (check()) {
  doStuff();
}  
if (stillOk()) {
  doMoreStuff();
}
if (amIStillReallyOk()) {
  doEvenMore();
}

// edit 
doThingsAtEndAndReportErrorStatus()

Similar to dasblinkenlight's answer, but avoids the assignment inside the if that could get "fixed" by a code reviewer:

bool goOn = check0();
if (goOn) {
    ...
    goOn = check1();
}
if (goOn) {
    ...
    goOn = check2();
}
if (goOn) {
    ...
}

...

I use this pattern when the results of a step need to be checked before the next step, which differs from a situation where all the checks could be done up front with a big if( check1() && check2()... type pattern.

Use exceptions. Your code will look much more cleaner (and exceptions were created exactly for handling errors in the execution flow of a program). For cleaning up resources (file descriptors, database connections, etc,), read the article Why doesn't C++ provide a "finally" construct?.

#include <iostream>
#include <stdexcept>   // For exception, runtime_error, out_of_range

int main () {
    try {
        if (!condition)
            throw std::runtime_error("nope.");
        ...
        if (!other condition)
            throw std::runtime_error("nope again.");
        ...
        if (!another condition)
            throw std::runtime_error("told you.");
        ...
        if (!yet another condition)
            throw std::runtime_error("OK, just forget it...");
    }
    catch (std::runtime_error &e) {
        std::cout << e.what() << std::endl;
    }
    catch (...) {
        std::cout << "Caught an unknown exception\n";
    }
    return 0;
}

For me do{...}while(0) is fine. If you don't want to see the do{...}while(0), you can define alternative keywords for them.

Example:

SomeUtilities.hpp:

#define BEGIN_TEST do{
#define END_TEST }while(0);

SomeSourceFile.cpp:

BEGIN_TEST
   if(!condition1) break;
   if(!condition2) break;
   if(!condition3) break;
   if(!condition4) break;
   if(!condition5) break;
   
   //processing code here

END_TEST

I think the compiler will remove the unneccessary while(0) condition in do{...}while(0) in binary version and convert the breaks into unconditional jump. You can check it's assembly language version to be sure.

Using goto also produces cleaner code and it is straightforward with the condition-then-jump logic. You can do the following:

{
   if(!condition1) goto end_blahblah;
   if(!condition2) goto end_blahblah;
   if(!condition3) goto end_blahblah;
   if(!condition4) goto end_blahblah;
   if(!condition5) goto end_blahblah;
   
   //processing code here

 }end_blah_blah:;  //use appropriate label here to describe...
                   //  ...the whole code inside the block.
 

Note the label is placed after the closing }. This is the avoid one possible problem in goto that is accidentally placing a code in between because you didn't see the label. It is now like do{...}while(0) without condition code.

To make this code cleaner and more comprehensible, you can do this:

SomeUtilities.hpp:

#define BEGIN_TEST {
#define END_TEST(_test_label_) }_test_label_:;
#define FAILED(_test_label_) goto _test_label_

SomeSourceFile.cpp:

BEGIN_TEST
   if(!condition1) FAILED(NormalizeData);
   if(!condition2) FAILED(NormalizeData);
   if(!condition3) FAILED(NormalizeData);
   if(!condition4) FAILED(NormalizeData);
   if(!condition5) FAILED(NormalizeData);

END_TEST(NormalizeData)

With this, you can do nested blocks and specify where you want to exit/jump-out.

BEGIN_TEST
   if(!condition1) FAILED(NormalizeData);
   if(!condition2) FAILED(NormalizeData);

   BEGIN_TEST
      if(!conditionAA) FAILED(DecryptBlah);
      if(!conditionBB) FAILED(NormalizeData);   //Jump out to the outmost block
      if(!conditionCC) FAILED(DecryptBlah);
  
      // --We can now decrypt and do other stuffs.

   END_TEST(DecryptBlah)

   if(!condition3) FAILED(NormalizeData);
   if(!condition4) FAILED(NormalizeData);

   // --other code here

   BEGIN_TEST
      if(!conditionA) FAILED(TrimSpaces);
      if(!conditionB) FAILED(TrimSpaces);
      if(!conditionC) FAILED(NormalizeData);   //Jump out to the outmost block
      if(!conditionD) FAILED(TrimSpaces);

      // --We can now trim completely or do other stuffs.

   END_TEST(TrimSpaces)

   // --Other code here...

   if(!condition5) FAILED(NormalizeData);

   //Ok, we got here. We can now process what we need to process.

END_TEST(NormalizeData)

Spaghetti code is not the fault of goto, it's the fault of the programmer. You can still produce spaghetti code without using goto.

This is a well-known and well-solved problem from a functional programming perspective - the maybe monad.

In response to the comment I received below I have edited my introduction here: You can find full details about implementing C++ monads in various places which will allow you to achieve what Rotsor suggests. It takes a while to grok monads so instead I'm going to suggest here a quick "poor-mans" monad-like mechanism for which you need know about nothing more than boost::optional.

Set up your computation steps as follows:

boost::optional<EnabledContext> enabled(boost::optional<Context> context);
boost::optional<EnergisedContext> energised(boost::optional<EnabledContext> context);

Each computational step can obviously do something like return boost::none if the optional it was given is empty. So for example:

struct Context { std::string coordinates_filename; /* ... */ };

struct EnabledContext { int x; int y; int z; /* ... */ };

boost::optional<EnabledContext> enabled(boost::optional<Context> c) {
   if (!c) return boost::none; // this line becomes implicit if going the whole hog with monads
   if (!exists((*c).coordinates_filename)) return boost::none; // return none when any error is encountered.
   EnabledContext ec;
   std::ifstream file_in((*c).coordinates_filename.c_str());
   file_in >> ec.x >> ec.y >> ec.z;
   return boost::optional<EnabledContext>(ec); // All ok. Return non-empty value.
}

Then chain them together:

Context context("planet_surface.txt", ...); // Close over all needed bits and pieces

boost::optional<EnergisedContext> result(energised(enabled(context)));
if (result) { // A single level "if" statement
    // do work on *result
} else {
    // error
}

The nice thing about this is that you can write clearly defined unit tests for each computational step. Also the invocation reads like plain English (as is usually the case with functional style).

If you don't care about immutability and it is more convenient to return the same object each time you could come up with some variation using shared_ptr or the like.

How about moving the if statements into an extra function yielding a numerical or enum result?

int ConditionCode (void) {
   if (condition1)
      return 1;
   if (condition2)
      return 2;
   ...
   return 0;
}


void MyFunc (void) {
   switch (ConditionCode ()) {
      case 1:
         ...
         break;

      case 2:
         ...
         break;

      ...

      default:
         ...
         break;
   }
}

I am not particularly into the way using break or return in such a case. Given that normally when we are facing such a situation, it is usually a comparatively long method.

If we are having multiple exit points, it may cause difficulties when we want to know what will cause certain logic to be executed: Normally we just keep on going up blocks enclosing that piece of logic, and the criteria of those enclosing block tell us the situation:

For example,

if (conditionA) {
    ....
    if (conditionB) {
        ....
        if (conditionC) {
            myLogic();
        }
    }
}

By looking at enclosing blocks, it is easy to find out that myLogic() only happen when conditionA and conditionB and conditionC is true.

It becomes a lot less visible when there are early returns:

if (conditionA) {
    ....
    if (!conditionB) {
        return;
    }
    if (!conditionD) {
        return;
    }
    if (conditionC) {
        myLogic();
    }
}

We can no longer navigate up from myLogic(), looking at enclosing block to figure out the condition.

There are different workarounds that I used. Here is one of them:

if (conditionA) {
    isA = true;
    ....
}

if (isA && conditionB) {
    isB = true;
    ...
}

if (isB && conditionC) {
    isC = true;
    myLogic();
}

(Of course it is welcomed to use the same variable to replace all isA isB isC.)

Such an approach will at least give the reader of code, that myLogic() is executed when isB && conditionC. The reader is given a hint that he need to further lookup what will cause isB to be true.

Something like this perhaps

#define EVER ;;

for(EVER)
{
    if(!check()) break;
}

or use exceptions

try
{
    for(;;)
        if(!check()) throw 1;
}
catch()
{
}

Using exceptions you can also pass data.

typedef bool (*Checker)();

Checker * checkers[]={
 &checker0,&checker1,.....，&checkerN,NULL
};

bool checker1(){
  if(condition){
    .....
    .....
    return true;
  }
  return false;
}

bool checker2(){
  if(condition){
    .....
    .....
    return true;
  }
  return false;
}

......

void doCheck(){
  Checker ** checker = checkers;
  while( *checker && (*checker)())
    checker++;
}

How about that?

I'm not a C++ programmer, so I won't write any code here, but so far nobody has mentioned an object oriented solution. So here is my guess on that:

Have a generic interface that provides a method to evaluate a single condition. Now you can use a list of implementations of those conditions in your object containing the method in question. You iterate over the list and evaluate each condition, possibly breaking out early if one fails.

The good thing is that such design sticks very well to the open/closed principle, because you can easily add new conditions during initialization of the object containing the method in question. You can even add a second method to the interface with the method for condition evaluation returning a description of the condition. This can be used for self-documenting systems.

The downside, however, is that there is slightly more overhead involved because of the usage of more objects and the iteration over the list.

Another pattern useful if you need different cleanup steps depending on where the failure is:

    private ResultCode DoEverything()
    {
        ResultCode processResult = ResultCode.FAILURE;
        if (DoStep1() != ResultCode.SUCCESSFUL)
        {
            Step1FailureCleanup();
        }
        else if (DoStep2() != ResultCode.SUCCESSFUL)
        {
            Step2FailureCleanup();
            processResult = ResultCode.SPECIFIC_FAILURE;
        }
        else if (DoStep3() != ResultCode.SUCCESSFUL)
        {
            Step3FailureCleanup();
        }
        ...
        else
        {
            processResult = ResultCode.SUCCESSFUL;
        }
        return processResult;
    }

First, a short example to show why goto is not a good solution for C++:

struct Bar {
    Bar();
};

extern bool check();

void foo()
{
    if (!check())
       goto out;

    Bar x;

    out:
}

Try to compile this into an object file and see what happens. Then try the equivalent do+ break + while(0).

That was an aside. The main point follows.

Those little chunks of code often require some kind of cleanup should the whole function fail. Those cleanups usually want to happen in the opposite order from the chunks themselves, as you "unwind" the partially-finished computation.

One option to obtain these semantics is RAII; see @utnapistim's answer. C++ guarantees that automatic destructors run in the opposite order to constructors, which naturally supplies an "unwinding".

But that requires lots of RAII classes. Sometimes a simpler option is just to use the stack:

bool calc1()
{
    if (!check())
        return false;

    // ... Do stuff1 here ...

    if (!calc2()) {
        // ... Undo stuff1 here ...
        return false;
    }

    return true;
}

bool calc2()
{
    if (!check())
        return false;

    // ... Do stuff2 here ...

    if (!calc3()) {
        // ... Undo stuff2 here ...
        return false;
    }

    return true;
}

...and so on. This is easy to audit, since it puts the "undo" code next to the "do" code. Easy auditing is good. It also makes the control flow very clear. It is a useful pattern for C, too.

It can require the calc functions to take lots of arguments, but that is usually not a problem if your classes/structs have good cohesion. (That is, stuff that belongs together lives in a single object, so these functions can take pointers or references to a small number of objects and still do lots of useful work.)

This is the way I do it.

void func() {
  if (!check()) return;
  ...
  ...
  
  if (!check()) return;
  ...
  ...
  
  if (!check()) return;
  ...
  ...
}

If you code has a long block of if..else if..else statements, you may try and rewrite the entire block with the help of Functors or function pointers. It may not be the right solution always but quite often is.

http://www.cprogramming.com/tutorial/functors-function-objects-in-c++.html

Consolidate it into one if statement:

if(
    condition
    && other_condition
    && another_condition
    && yet_another_condition
    && ...
) {
        if (final_cond){
            //Do stuff
        } else {
            //Do other stuff
        }
}

This is the pattern used in languages such as Java where the goto keyword was removed.

I am amazed by the number of different answers being presented here. But, finally in the code which I have to change (i.e. remove this do-while(0) hack or anything), I did something different from any of the answers being mentioned here and I am confused why no one thought this. Here's what I did:

Initial code:

do {

    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
} while(0);

finishingUpStuff.

Now:

finish(params)
{
  ...
  ...
}

if(!check()){
    finish(params);    
    return;
}
...
...
if(!check()){
    finish(params);    
    return;
}
...
...
if(!check()){
    finish(params);    
    return;
}
...
...

So, what has been done here is that the finishing up stuff has been isolated in a function and things have suddenly become so simple and clean!

I thought this solution was worth mentioning, so provided it here.

If using the same error handler for all errors, and each step returns a bool indicating success:

if(
    DoSomething() &&
    DoSomethingElse() &&
    DoAThirdThing() )
{
    // do good condition action
}
else
{
    // handle error
}

(Similar to tyzoid's answer, but conditions are the actions, and the && prevents additional actions from occurring after the first failure.)

Why wasn't flagging method answered it is used since ages.

//you can use something like this (pseudocode)
long var = 0;
if(condition)  flag a bit in var
if(condition)  flag another bit in var
if(condition)  flag another bit in var
............
if(var == certain number) {
Do the required task
}