How to avoid “if” chains?

Question

Assuming I have this pseudo-code:

bool conditionA = executeStepA();
if (conditionA){
    bool conditionB = executeStepB();
    if (conditionB){
        bool conditionC = executeStepC();
        if (conditionC){
            ...
        }
    }
}

executeThisFunctionInAnyCase();

Functions executeStepX should be executed if and only if the previous succeed. In any case, the executeThisFunctionInAnyCase function should be called at the end. I'm a newbie in programming, so sorry for the very basic question: is there a way (in C/C++ for example) to avoid that long if chain producing that sort of "pyramid of code", at the expense of the code legibility?

I know that if we could skip the executeThisFunctionInAnyCase function call, the code could be simplified as:

bool conditionA = executeStepA();
if (!conditionA) return;
bool conditionB = executeStepB();
if (!conditionB) return;
bool conditionC = executeStepC();
if (!conditionC) return;

But the constraint is the executeThisFunctionInAnyCase function call. Could the break statement be used in some way?

Answer 1

Would this work? I think this is equivalent with your code.

bool condition = true; // using only one boolean variable
if (condition) condition = executeStepA();
if (condition) condition = executeStepB();
if (condition) condition = executeStepC();
...
executeThisFunctionInAnyCase();

Answer 2

This is a common situation and there are many common ways to deal with it. Here's my attempt at a canonical answer. Please comment if I missed anything and I'll keep this post up to date.

This is an Arrow

What you are discussing is known as the arrow anti-pattern. It is called an arrow because the chain of nested ifs form code blocks that expand farther and farther to the right and then back to the left, forming a visual arrow that "points" to the right side of the code editor pane.

Flatten the Arrow with the Guard

Some common ways of avoiding the Arrow are discussed here. The most common method is to use a guard pattern, in which the code handles the exception flows first and then handles the basic flow, e.g. instead of

if (ok)
{
    DoSomething();
}
else
{
    _log.Error("oops");
    return;
}

... you'd use....

if (!ok)
{
    _log.Error("oops");
    return;
} 
DoSomething(); //notice how this is already farther to the left than the example above

When there is a long series of guards this flattens the code considerably as all the guards appear all the way to the left and your ifs are not nested. In addition, you are visually pairing the logic condition with its associated error, which makes it far easier to tell what is going on:

Arrow:

ok = DoSomething1();
if (ok)
{
    ok = DoSomething2();
    if (ok)
    {
        ok = DoSomething3();
        if (!ok)
        {
            _log.Error("oops");  //Tip of the Arrow
            return;
        }
    }
    else
    {
       _log.Error("oops");
       return;
    }
}
else
{
    _log.Error("oops");
    return;
}

Guard:

ok = DoSomething1();
if (!ok)
{
    _log.Error("oops");
    return;
} 
ok = DoSomething2();
if (!ok)
{
    _log.Error("oops");
    return;
} 
ok = DoSomething3();
if (!ok)
{
    _log.Error("oops");
    return;
} 
ok = DoSomething4();
if (!ok)
{
    _log.Error("oops");
    return;
} 

This is objectively and quantifiably easier to read because

1. The { and } characters for a given logic block are closer together
2. The amount of mental context needed to understand a particular line is smaller
3. The entirety of logic associated with an if condition is more likely to be on one page
4. The need for the coder to scroll the page/eye track is greatly lessened

How to add common code at the end

The problem with the guard pattern is that it relies on what is called "opportunistic return" or "opportunistic exit." In other words, it breaks the pattern that each and every function should have exactly one point of exit. This is a problem for two reasons:

1. It rubs some people the wrong way, e.g. people who learned to code on Pascal have learned that one function = one exit point.
2. It does not provide a section of code that executes upon exit no matter what, which is the subject at hand.

Below I've provided some options for working around this limitation either by using language features or by avoiding the problem altogether.

Option 1. You can't do this: use finally

Unfortunately, as a c++ developer, you can't do this. But this is the number one answer for languages that contain a finally keyword, since this is exactly what it is for.

try
{
    if (!ok)
    {
        _log.Error("oops");
        return;
    } 
    DoSomething(); //notice how this is already farther to the left than the example above
}
finally
{
    DoSomethingNoMatterWhat();
}

Option 2. Avoid the issue: Restructure your functions

You can avoid the problem by breaking the code into two functions. This solution has the benefit of working for any language, and additionally it can reduce cyclomatic complexity, which is a proven way to reduce your defect rate, and improves the specificity of any automated unit tests.

Here's an example:

void OuterFunction()
{
    DoSomethingIfPossible();
    DoSomethingNoMatterWhat();
}

void DoSomethingIfPossible()
{
    if (!ok)
    {
        _log.Error("Oops");
        return;
    }
    DoSomething();
}

Option 3. Language trick: Use a fake loop

Another common trick I see is using while(true) and break, as shown in the other answers.

while(true)
{
     if (!ok) break;
     DoSomething();
     break;  //important
}
DoSomethingNoMatterWhat();

While this is less "honest" than using goto, it is less prone to being messed up when refactoring, as it clearly marks the boundaries of logic scope. A naive coder who cuts and pastes your labels or your goto statements can cause major problems! (And frankly the pattern is so common now I think it clearly communicates the intent, and is therefore not "dishonest" at all).

There are other variants of this options. For example, one could use switch instead of while. Any language construct with a break keyword would probably work.

Option 4. Leverage the object life cycle

One other approach leverages the object life cycle. Use a context object to carry around your parameters (something which our naive example suspiciously lacks) and dispose of it when you're done.

class MyContext
{
   ~MyContext()
   {
        DoSomethingNoMatterWhat();
   }
}

void MainMethod()
{
    MyContext myContext;
    ok = DoSomething(myContext);
    if (!ok)
    {
        _log.Error("Oops");
        return;
    }
    ok = DoSomethingElse(myContext);
    if (!ok)
    {
        _log.Error("Oops");
        return;
    }
    ok = DoSomethingMore(myContext);
    if (!ok)
    {
        _log.Error("Oops");
    }

    //DoSomethingNoMatterWhat will be called when myContext goes out of scope
}

Note: Be sure you understand the object life cycle of your language of choice. You need some sort of deterministic garbage collection for this to work, i.e. you have to know when the destructor will be called. In some languages you will need to use Dispose instead of a destructor.

Option 4.1. Leverage the object life cycle (wrapper pattern)

If you're going to use an object-oriented approach, may as well do it right. This option uses a class to "wrap" the resources that require cleanup, as well as its other operations.

class MyWrapper 
{
   bool DoSomething() {...};
   bool DoSomethingElse() {...}


   void ~MyWapper()
   {
        DoSomethingNoMatterWhat();
   }
}

void MainMethod()
{
    bool ok = myWrapper.DoSomething();
    if (!ok)
        _log.Error("Oops");
        return;
    }
    ok = myWrapper.DoSomethingElse();
    if (!ok)
       _log.Error("Oops");
        return;
    }
}
//DoSomethingNoMatterWhat will be called when myWrapper is destroyed

Again, be sure you understand your object life cycle.

Option 5. Language trick: Use short-circuit evaluation

Another technique is to take advantage of short-circuit evaluation.

if (DoSomething1() && DoSomething2() && DoSomething3())
{
    DoSomething4();
}
DoSomethingNoMatterWhat();

This solution takes advantage of the way the && operator works. When the left hand side of && evaluates to false, the right hand side is never evaluated.

This trick is most useful when compact code is required and when the code is not likely to see much maintenance, e.g you are implementing a well-known algorithm. For more general coding the structure of this code is too brittle; even a minor change to the logic could trigger a total rewrite.

Answer 3

As Rommik mentioned, you could apply a design pattern for this, but I would use the Decorator pattern rather than Strategy since you are wanting to chain calls. If the code is simple, then I would go with one of the nicely structured answers to prevent nesting. However, if it is complex or requires dynamic chaining, then the Decorator pattern is a good choice. Here is a yUML class diagram:

Here is a sample LinqPad C# program:

void Main()
{
    IOperation step = new StepC();
    step = new StepB(step);
    step = new StepA(step);
    step.Next();
}

public interface IOperation 
{
    bool Next();
}

public class StepA : IOperation
{
    private IOperation _chain;
    public StepA(IOperation chain=null)
    {
        _chain = chain;
    }

    public bool Next() 
    {
        bool localResult = false;
        //do work
        //...
        // set localResult to success of this work
        // just for this example, hard coding to true
        localResult = true;
        Console.WriteLine("Step A success={0}", localResult);

        //then call next in chain and return
        return (localResult && _chain != null) 
            ? _chain.Next() 
            : true;
    }
}

public class StepB : IOperation
{
    private IOperation _chain;
    public StepB(IOperation chain=null)
    {
        _chain = chain;
    }

    public bool Next() 
    {   
        bool localResult = false;

        //do work
        //...
        // set localResult to success of this work
        // just for this example, hard coding to false, 
            // to show breaking out of the chain
        localResult = false;
        Console.WriteLine("Step B success={0}", localResult);

        //then call next in chain and return
        return (localResult && _chain != null) 
            ? _chain.Next() 
            : true;
    }
}

public class StepC : IOperation
{
    private IOperation _chain;
    public StepC(IOperation chain=null)
    {
        _chain = chain;
    }

    public bool Next() 
    {
        bool localResult = false;
        //do work
        //...
        // set localResult to success of this work
        // just for this example, hard coding to true
        localResult = true;
        Console.WriteLine("Step C success={0}", localResult);
        //then call next in chain and return
        return (localResult && _chain != null) 
            ? _chain.Next() 
            : true;
    }
}

The best book to read on design patterns, IMHO, is Head First Design Patterns.

Answer 4

You can use an && (logic AND):

if (executeStepA() && executeStepB() && executeStepC()){
    ...
}
executeThisFunctionInAnyCase();

this will satisfy both of your requirements:

• executeStep<X>() should evaluate only if the previous one succeeded (this is called short circuit evaluation)
• executeThisFunctionInAnyCase() will be executed in any case

Answer 5

A lot of good answers already, but most of them seem to tradeoff on some (admittedly very little) of the flexibility. A common approach which doesn't require this tradeoff is adding a status/keep-going variable. The price is, of course, one extra value to keep track of:

bool ok = true;
bool conditionA = executeStepA();
// ... possibly edit conditionA, or just ok &= executeStepA();
ok &= conditionA;

if (ok) {
    bool conditionB = executeStepB();
    // ... possibly do more stuff
    ok &= conditionB;
}
if (ok) {
    bool conditionC = executeStepC();
    ok &= conditionC;
}
if (ok && additionalCondition) {
    // ...
}

executeThisFunctionInAnyCase();
// can now also:
return ok;

Answer 6

You could put all the if conditions, formatted as you want it in a function of their own, the on return execute the executeThisFunctionInAnyCase() function.

From the base example in the OP, the condition testing and execution can be split off as such;

void InitialSteps()
{
  bool conditionA = executeStepA();
  if (!conditionA)
    return;
  bool conditionB = executeStepB();
  if (!conditionB)
    return;
  bool conditionC = executeStepC();
  if (!conditionC)
    return;
}

And then called as such;

InitialSteps();
executeThisFunctionInAnyCase();

If C++11 lambdas are available (there was no C++11 tag in the OP, but they may still be an option), then we can forgo the seperate function and wrap this up into a lambda.

// Capture by reference (variable access may be required)
auto initialSteps = [&]() {
  // any additional code
  bool conditionA = executeStepA();
  if (!conditionA)
    return;
  // any additional code
  bool conditionB = executeStepB();
  if (!conditionB)
    return;
  // any additional code
  bool conditionC = executeStepC();
  if (!conditionC)
    return;
};

initialSteps();
executeThisFunctionInAnyCase();