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

Just do

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

It's that simple.

---

Due to three edits that each has fundamentally changed the question (four if one counts the revision back to version #1), I include the code example I'm answering to:

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

executeThisFunctionInAnyCase();

Answer 2

It's seems like you want to do all your call from a single block. As other have proposed it, you should used either a while loop and leave using break or a new function that you can leave with return (may be cleaner).

I personally banish goto, even for function exit. They are harder to spot when debugging.

An elegant alternative that should work for your workflow is to build a function array and iterate on this one.

const int STEP_ARRAY_COUNT = 3;
bool (*stepsArray[])() = {
   executeStepA, executeStepB, executeStepC
};

for (int i=0; i<STEP_ARRAY_COUNT; ++i) {
    if (!stepsArray[i]()) {
        break;
    }
}

executeThisFunctionInAnyCase();

Answer 3

Several answers hinted at a pattern that I saw and used many times, especially in network programming. In network stacks there is often a long sequence of requests, any of which can fail and will stop the process.

The common pattern was to use do { } while (false);

I used a macro for the while(false) to make it do { } once; The common pattern was:

do
{
    bool conditionA = executeStepA();
    if (! conditionA) break;
    bool conditionB = executeStepB();
    if (! conditionB) break;
    // etc.
} while (false);

This pattern was relatively easy to read, and allowed objects to be used that would properly destruct and also avoided multiple returns making stepping and debugging a bit easier.

Answer 4

an interesting way is to work with exceptions.

try
{
    executeStepA();//function throws an exception on error
    ......
}
catch(...)
{
    //some error handling
}
finally
{
    executeThisFunctionInAnyCase();
}

If you write such code you are going somehow in the wrong direction. I wont see it as "the problem" to have such code, but to have such a messy "architecture".

Tip: discuss those cases with a seasoned developer which you trust ;-)

Answer 5

Because you also have [...block of code...] between executions, I guess you have memory allocation or object initializations. In this way you have to care about cleaning all you already initialized at exit, and also clean it if you will meet problem and any of functions will return false.

In this case, best what I had in my experience (when I worked with CryptoAPI) was creating small classes, in constructor you initialize your data, in destructor you uninitialize it. Each next function class have to be child of previous function class. If something went wrong - throw exception.

class CondA
{
public:
    CondA() { 
        if (!executeStepA()) 
            throw int(1);
        [Initialize data]
    }
    ~CondA() {        
        [Clean data]
    }
    A* _a;
};

class CondB : public CondA
{
public:
    CondB() { 
        if (!executeStepB()) 
            throw int(2);
        [Initialize data]
    }
    ~CondB() {        
        [Clean data]
    }
    B* _b;
};

class CondC : public CondB
{
public:
    CondC() { 
        if (!executeStepC()) 
            throw int(3);
        [Initialize data]
    }
    ~CondC() {        
        [Clean data]
    }
    C* _c;
};

And then in your code you just need to call:

shared_ptr<CondC> C(nullptr);
try{
    C = make_shared<CondC>();
}
catch(int& e)
{
    //do something
}
if (C != nullptr)
{
   C->a;//work with
   C->b;//work with
   C->c;//work with
}
executeThisFunctionInAnyCase();

I guess it is best solution if every call of ConditionX initialize something, allocs memory and etc. Best to be sure everything will be cleaned.

Answer 6

In C++ (the question is tagged both C and C++), if you can't change the functions to use exceptions, you still can use the exception mechanism if you write a little helper function like

struct function_failed {};
void attempt(bool retval)
{
  if (!retval)
    throw function_failed(); // or a more specific exception class
}

Then your code could read as follows:

try
{
  attempt(executeStepA());
  attempt(executeStepB());
  attempt(executeStepC());
}
catch (function_failed)
{
  // -- this block intentionally left empty --
}

executeThisFunctionInAnyCase();

If you're into fancy syntax, you could instead make it work via explicit cast:

struct function_failed {};
struct attempt
{
  attempt(bool retval)
  {
    if (!retval)
      throw function_failed();
  }
};

Then you can write your code as

try
{
  (attempt) executeStepA();
  (attempt) executeStepB();
  (attempt) executeStepC();
}
catch (function_failed)
{
  // -- this block intentionally left empty --
}

executeThisFunctionInAnyCase();