ABCplus
ABCplus

Reputation: 4021

How to avoid "if" chains?

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?

Upvotes: 277

Views: 50517

Answers (30)

Uri Raz
Uri Raz

Reputation: 434

I think C++23's monadic operations for optional would do well, though the functions would have to be changed a bit.

The and_then() method does the break or call next function operation, and chaining the method allows calling the functions one by one, till one of them returns false.

To give a quick & dirty example:

#include <iostream>
#include <optional>
#include <cstdlib>

using namespace std;

optional<bool> func1() {
    cout << "func1\n";

    if (rand() % 2)
        return true;
    else
        return nullopt;
}

optional<bool> func2(optional<bool> v) {
    cout << "func2\n";

    if (rand() % 2)
        return true;
    else
        return nullopt;
}

optional<bool> func3(optional<bool> v) {
    cout << "func3\n";

    if (rand() % 2)
        return true;
    else
        return nullopt;
}

void func4() {
    cout << "func4\n";
}

int main() {
    srand(time(NULL));

    func1()
      .and_then(func2)
      .and_then(func3);

    func4();

    return 0;
}

Upvotes: 0

sampathsris
sampathsris

Reputation: 22280

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();

Upvotes: 19

Gaskoin
Gaskoin

Reputation: 2485

Why is nobody giving the simplest solution ? :D

If all your functions have the same signature then you can do it this way (for C language):

bool (*step[])() = {
    &executeStepA,
    &executeStepB,
    &executeStepC,
    ... 
};

for (int i = 0; i < numberOfSteps; i++) {
    bool condition = step[i]();

    if (!condition) {
        break;
    }
}

executeThisFunctionInAnyCase();

For a clean C++ solution, you should create an interface class that contains an execute method and wraps your steps in objects.
Then, the solution above will look like this:

Step *steps[] = {
    stepA,
    stepB,
    stepC,
    ... 
};

for (int i = 0; i < numberOfSteps; i++) {
    Step *step = steps[i];

    if (!step->execute()) {
        break;
    }
}

executeThisFunctionInAnyCase();

Upvotes: 7

John Wu
John Wu

Reputation: 52240

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.

Upvotes: 135

Debasis
Debasis

Reputation: 3740

There's a nice technique which doesn't need an additional wrapper function with the return statements (the method prescribed by Itjax). It makes use of a do while(0) pseudo-loop. The while (0) ensures that it is actually not a loop but executed only once. However, the loop syntax allows the use of the break statement.

void foo()
{
  // ...
  do {
      if (!executeStepA())
          break;
      if (!executeStepB())
          break;
      if (!executeStepC())
          break;
  }
  while (0);
  // ...
}

Upvotes: 35

Fattie
Fattie

Reputation: 12621

You just do this..

coverConditions();
executeThisFunctionInAnyCase();

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

99 times of 100, this is the only way to do it.

Never, ever, ever try to do something "tricky" in computer code.


By the way, I'm pretty sure the following is the actual solution you had in mind...

The continue statement is critical in algorithmic programming. (Much as, the goto statement is critical in algorithmic programming.)

In many programming languages you can do this:

-(void)_testKode
    {
    NSLog(@"code a");
    NSLog(@"code b");
    NSLog(@"code c\n");
    
    int x = 69;
    
    {
    
    if ( x == 13 )
        {
        NSLog(@"code d---\n");
        continue;
        }
    
    if ( x == 69 )
        {
        NSLog(@"code e---\n");
        continue;
        }
    
    if ( x == 13 )
        {
        NSLog(@"code f---\n");
        continue;
        }
    
    }
    
    NSLog(@"code g");
    }

(Note first of all: naked blocks like that example are a critical and important part of writing beautiful code, particularly if you are dealing with "algorithmic" programming.)

Again, that's exactly what you had in your head, right? And that's the beautiful way to write it, so you have good instincts.

However, tragically, in the current version of objective-c (Aside - I don't know about Swift, sorry) there is a risible feature where it checks if the enclosing block is a loop.

enter image description here

Here's how you get around that...

-(void)_testKode
    {
    NSLog(@"code a");
    NSLog(@"code b");
    NSLog(@"code c\n");
    
    int x = 69;
    
    do{
    
    if ( x == 13 )
        {
        NSLog(@"code d---\n");
        continue;
        }
    
    if ( x == 69 )
        {
        NSLog(@"code e---\n");
        continue;
        }
    
    if ( x == 13 )
        {
        NSLog(@"code f---\n");
        continue;
        }
    
    }while(false);
    
    NSLog(@"code g");
    }

So don't forget that ..

do { } while(false);

just means "do this block once".

ie, there is utterly no difference between writing do{}while(false); and simply writing {} .

This now works perfectly as you wanted...here's the output...

enter image description here

So, it's possible that's how you see the algorithm in your head. You should always try to write what's in your head. ( Particularly if you are not sober, because that's when the pretty comes out! :) )

In "algorithmic" projects where this happens a lot, in objective-c, we always have a macro like...

#define RUNONCE while(false)

... so then you can do this ...

-(void)_testKode
    {
    NSLog(@"code a");
    int x = 69;
    
    do{
    if ( x == 13 )
        {
        NSLog(@"code d---\n");
        continue;
        }
    if ( x == 69 )
        {
        NSLog(@"code e---\n");
        continue;
        }
    if ( x == 13 )
        {
        NSLog(@"code f---\n");
        continue;
        }
    }RUNONCE
    
    NSLog(@"code g");
    }

There are two points:

a, even though it's stupid that objective-c checks the type of block a continue statement is in, it's troubling to "fight that". So it's a tough decision.

b, there's the question should you indent, in the example, that block? I lose sleep over questions like that, so I can't advise.

Hope it helps.

Upvotes: 9

Dr. Debasish Jana
Dr. Debasish Jana

Reputation: 7118

Very simple.

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

This will preserve the boolean variables conditionA, conditionB and conditionC as well.

Upvotes: 1

inigoD
inigoD

Reputation: 1741

Why using OOP? in pseudocode:

abstract class Abstraction():
   function executeStepA(){...};
   function executeStepB(){...};   
   function executeStepC(){...};
   function executeThisFunctionInAnyCase(){....}
   abstract function execute():

class A(Abstraction){
   function execute(){
      executeStepA();
      executeStepB();
      executeStepC();
   }
}
 class B(Abstraction){
   function execute(){
      executeStepA();
      executeStepB();
   }
}
class C(Abstraction){
     function execute(){
       executeStepA();
     }
}

this way your if's dissapear

item.execute();
item.executeThisFunctionInAnyCase();

Usually, ifs can be avoided using OOP.

Upvotes: 0

pablo1977
pablo1977

Reputation: 4433

An alternative solution would be to define an idiom through macro hacks.

 #define block for(int block = 0; !block; block++)

Now, a "block" can be exited with break, in the same way as for(;;) and while() loops. Example:

int main(void) {

    block {
       if (conditionA) {
          // Do stuff A...
          break; 
       }
       if (conditionB) {
          // Do stuff B...
          break; 
       }
       if (conditionC) {
          // Do stuff C...
          break; 
       }
       else {
         // Do default stuff...
       }
    } /* End of "block" statement */
    /* --->   The "break" sentences jump here */

    return 0;
} 

In despite of the "for(;;)" construction, the "block" statement is executed just once.
This "blocks" are able to be exited with break sentences.
Hence, the chains of if else if else if... sentences are avoided.
At most, one lastly else can hang at the end of the "block", to handle "default" cases.

This technique is intended to avoid the typical and ugly do { ... } while(0) method.
In the macro block it is defined a variable also named block defined in such a way that exactly 1 iteration of for is executed. According to the substitution rules for macros, the identifier block inside the definition of the macro block is not recursively replaced, therefore block becomes an identifier inaccesible to the programmer, but internally works well to control de "hidden" for(;;) loop.

Moreover: these "blocks" can be nested, since the hidden variable int block would have different scopes.

Upvotes: 2

Yakaas
Yakaas

Reputation: 191

Conditions can be simplified if conditions are moved under individual steps, here's a c# pseudo code,

the idea is to use a choreography instead of a central orchestration.

void Main()
{
    Request request = new Request();
    Response response = null;

    // enlist all the processors
    var processors = new List<IProcessor>() {new StepA() };

    var factory = new ProcessorFactory(processors);

    // execute as a choreography rather as a central orchestration.
    var processor = factory.Get(request, response);
    while (processor != null)
    {
        processor.Handle(request, out response);
        processor = factory.Get(request, response); 
    }

    // final result...
    //response
}

public class Request
{
}

public class Response
{
}

public interface IProcessor
{
    bool CanProcess(Request request, Response response);
    bool Handle(Request request, out Response response);
}

public interface IProcessorFactory
{
    IProcessor Get(Request request, Response response);
}   

public class ProcessorFactory : IProcessorFactory
{
    private readonly IEnumerable<IProcessor> processors;

    public ProcessorFactory(IEnumerable<IProcessor> processors)
    {
        this.processors = processors;
    }

    public IProcessor Get(Request request, Response response)
    {
        // this is an iterator
        var matchingProcessors = processors.Where(x => x.CanProcess(request, response)).ToArray();

        if (!matchingProcessors.Any())
        {
            return null;
        }

        return matchingProcessors[0];
    }
}

// Individual request processors, you will have many of these...
public class StepA: IProcessor
{
    public bool CanProcess(Request request, Response response)
    {
        // Validate wether this can be processed -- if condition here
        return false;
    }

    public bool Handle(Request request, out Response response)
    {
        response = null;
        return false;
    }
}

Upvotes: -1

epichorns
epichorns

Reputation: 1278

An easy solution is using a condition boolean variable, and the same one can be reused over and over again in order to check all the results of the steps being executed in sequence:

    bool cond = executeStepA();
    if(cond) cond = executeStepB();
    if(cond) cond = executeStepC();
    if(cond) cond = executeStepD();

    executeThisFunctionInAnyCase();

Not that it was not necessary to do this beforehand: bool cond = true; ... and then followed by if(cond) cond = executeStepA(); The cond variable can be immediately assigned to the result of executeStepA(), therefore making the code even shorter and simpler to read.

Another more peculiar but fun approach would be this (some might think that this is a good candidate for the IOCCC though, but still):

    !executeStepA() ? 0 :
      !executeStepB() ? 0 :
      !executeStepC() ? 0 :
      !executeStepD() ? 0 : 1 ;

    executeThisFunctionInAnyCase();

The result is exactly the same as if we did what the OP posted, i.e.:

    if(executeStepA()){
        if(executeStepB()){
            if(executeStepC()){
                if(executeStepD()){
                }
            }
        }
    }

    executeThisFunctionInAnyCase();

Upvotes: 2

MichaelCMS
MichaelCMS

Reputation: 4763

In my opinion function pointers are the best way to go through this.

This approach was mentioned before, but I'd like to go even deeper into the pros of using such approach against an arrowing type of code.

From my experience, this sort of if chains happen in an initialization part of a certain action of the program. The program needs to be sure that everything is peachy before attempting to start.

In often cases in many of the do stuff functions some things might get allocated , or ownership could be changed. You will want to reverse the process if you fail.

Let's say you have the following 3 functions :

bool loadResources()
{
   return attemptToLoadResources();
}
bool getGlobalMutex()
{
   return attemptToGetGlobalMutex();
}
bool startInfernalMachine()
{
   return attemptToStartInfernalMachine();
}

The prototype for all of the functions will be:

typdef bool (*initializerFunc)(void);

So as mentioned above, you will add into a vector using push_back the pointers and just run them in the order. However, if your program fails at the startInfernalMachine , you will need to manually return the mutex and unload resources . If you do this in your RunAllways function, you will have a baad time.

But wait! functors are quite awesome (sometimes) and you can just change the prototype to the following :

typdef bool (*initializerFunc)(bool);

Why ? Well, the new functions will now look something like :

bool loadResources(bool bLoad)
{
   if (bLoad)
     return attemptToLoadResources();
   else
     return attemptToUnloadResources();
}
bool getGlobalMutex(bool bGet)
{
  if (bGet)
    return attemptToGetGlobalMutex();
  else
    return releaseGlobalMutex();
}
...

So now,the whole of the code, will look something like :

vector<initializerFunc> funcs;
funcs.push_back(&loadResources);
funcs.push_back(&getGlobalMutex);
funcs.push_back(&startInfernalMachine);
// yeah, i know, i don't use iterators
int lastIdx;
for (int i=0;i<funcs.size();i++)
{
   if (funcs[i](true))
      lastIdx=i;
   else 
      break;
}
// time to check if everything is peachy
if (lastIdx!=funcs.size()-1)
{
   // sad face, undo
   for (int i=lastIdx;i>=0;i++)
      funcs[i](false);
}

So it's definately a step forward to autoclean your project, and get past this phase. However, implementation is a bit awkward, since you will need to use this pushback mechanism over and over again. If you have only 1 such place, let's say it's ok, but if you have it 10 places, with an oscilating number of functions... not so fun.

Fortunately, there's another mechanism that will allow you to make a even better abstraction : variadic functions. After all, there's a varying number of functions you need to go thorough. A variadic function would look something like this :

bool variadicInitialization(int nFuncs,...)
{
    bool rez;
    int lastIdx;
    initializerFunccur;
    vector<initializerFunc> reverse;
    va_list vl;
    va_start(vl,nFuncs);
    for (int i=0;i<nFuncs;i++)
    {
        cur = va_arg(vl,initializerFunc);
        reverse.push_back(cur);
        rez= cur(true);
        if (rez)
            lastIdx=i;
        if (!rez)
            break;
    }
    va_end(vl);

    if (!rez)
    {

        for (int i=lastIdx;i>=0;i--)
        {
            reverse[i](false);
        }
    }
    return rez;
}

And now your code will be reduced (anywhere in the application) to this :

bool success = variadicInitialization(&loadResources,&getGlobalMutex,&startInfernalMachine);
doSomethingAllways();

So this way you can do all those nasty if lists with just one function call, and be sure that when the function exits you will not have any residues from initializations.

Your fellow team mates will be really grateful for making 100 lines of code possible in just 1.

BUT WAIT! There's more! One of the main traits of the arrow-type code is that you need to have a specific order! And that specific order needs to be the same in the whole application (multithreading deadlock avoidance rule no 1 : always take mutexes in the same order throughout the whole application) What if one of the newcomers, just makes the functions in a random order ? Even worse, what if you are asked to expose this to java or C# ? (yeah, cross platform is a pain)

Fortunately there's an approach for this. In bullet points , this is what I would suggest :

  • create an enum , starting from the first resource to the last

  • define a pair which takes a value from the enum and pairs it to the function pointer

  • put these pairs in a vector (I know, I just defined the use of a map :) , but I always go vector for small numbers)

  • change the variadic macro from taking function pointers to integers (which are easily exposed in java or C# ;) ) )

  • in the variadic function, sort those integers

  • when running, run the function assigned to that integer.

At the end, your code will ensure the following :

  • one line of code for initialization, no matter how many stuff needs to be ok

  • enforcing of the order of calling : you cannot call startInfernalMachine before loadResources unless you (the architect) decides to allow this

  • complete cleanup if something fails along the way (considering you made deinitialization properly)

  • changing the order of the initialization in the whole application means only changing the order in the enum

Upvotes: -1

M.M
M.M

Reputation: 141554

Well, 50+ answers so far and nobody has mentioned what I usually do in this situation! (i.e. an operation that consists of several steps, but it would be overkill to use a state machine or a function pointer table):

if ( !executeStepA() )
{
    // error handling for "A" failing
}
else if ( !executeStepB() )
{
    // error handling for "B" failing
}
else if ( !executeStepC() )
{
    // error handling for "C" failing
}
else
{
    // all steps succeeded!
}

executeThisFunctionInAnyCase();

Advantages:

  • Does not end up with huge indent level
  • Error handling code (optional) is on the lines just after the call to the function that failed

Disadvantages:

  • Can get ugly if you have a step that's not just wrapped up in a single function call
  • Gets ugly if any flow is required other than "execute steps in order, aborting if one fails"

Upvotes: 0

Niall
Niall

Reputation: 30605

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();

Upvotes: 13

datenwolf
datenwolf

Reputation: 162164

Fake loops already got mentioned, but I didn't see the following trick in the answers given so far: You can use a do { /* ... */ } while( evaulates_to_zero() ); to implement a two-way early-out breaker. Using break terminates the loop without going through evaluating the condition statement, whereas a continue will evaulate the condition statement.

You can use that if you have two kinds of finalization, where one path must do a little more work than the other:

#include <stdio.h>
#include <ctype.h>

int finalize(char ch)
{
    fprintf(stdout, "read a character: %c\n", (char)toupper(ch));

    return 0;
}

int main(int argc, char *argv[])
{
    int ch;
    do {
        ch = fgetc(stdin);
        if( isdigit(ch) ) {
            fprintf(stderr, "read a digit (%c): aborting!\n", (char)ch);
            break;
        }
        if( isalpha(ch) ) {
            continue;
        }
        fprintf(stdout, "thank you\n");
    } while( finalize(ch) );

    return 0;
}

Executing this gives the following session protocol:

dw@narfi ~/misc/test/fakeloopbreak $ ./fakeloopbreak 
-
thank you
read a character: -

dw@narfi ~/misc/test/fakeloopbreak $ ./fakeloopbreak 
a
read a character: A

dw@narfi ~/misc/test/fakeloopbreak $ ./fakeloopbreak 
1
read a digit (1): aborting!

Upvotes: 0

glampert
glampert

Reputation: 4411

For C++11 and beyond, a nice approach might be to implement a scope exit system similar to D's scope(exit) mechanism.

One possible way to implement it is using C++11 lambdas and some helper macros:

template<typename F> struct ScopeExit 
{
    ScopeExit(F f) : fn(f) { }
    ~ScopeExit() 
    { 
         fn();
    }

    F fn;
};

template<typename F> ScopeExit<F> MakeScopeExit(F f) { return ScopeExit<F>(f); };

#define STR_APPEND2_HELPER(x, y) x##y
#define STR_APPEND2(x, y) STR_APPEND2_HELPER(x, y)

#define SCOPE_EXIT(code)\
    auto STR_APPEND2(scope_exit_, __LINE__) = MakeScopeExit([&](){ code })

This will allow you to return early from the function and ensure whatever cleanup code you define is always executed upon scope exit:

SCOPE_EXIT(
    delete pointerA;
    delete pointerB;
    close(fileC); );

if (!executeStepA())
    return;

if (!executeStepB())
    return;

if (!executeStepC())
    return;

The macros are really just decoration. MakeScopeExit() can be used directly.

Upvotes: 7

Yakk - Adam Nevraumont
Yakk - Adam Nevraumont

Reputation: 275385

[&]{
  bool conditionA = executeStepA();
  if (!conditionA) return; // break
  bool conditionB = executeStepB();
  if (!conditionB) return; // break
  bool conditionC = executeStepC();
  if (!conditionC) return; // break
}();
executeThisFunctionInAnyCase();

We create an anonymous lambda function with implicit reference capture, and run it. The code within it runs immediately.

When it wants to stop, it simply returns.

Then, after it runs, we run executeThisFunctionInAnyCase.

return within the lambda is a break to end of block. Any other kind of flow control just works.

Exceptions are left alone -- if you want to catch them, do it explicitly. Be careful about running executeThisFunctionInAnyCase if exceptions are thrown -- you generally do not want to run executeThisFunctionInAnyCase if it can throw an exception in an exception handler, as that results in a mess (which mess will depend on the language).

A nice property of such capture based inline functions is you can refactor existing code in place. If your function gets really long, breaking it down into component parts is a good idea.

A variant of this that works in more languages is:

bool working = executeStepA();
working = working && executeStepB();
working = working && executeStepC();
executeThisFunctionInAnyCase();

where you write individual lines that each short-circuit. Code can be injected between these lines, giving you multiple "in any case", or you can do if(working) { /* code */ } between execution steps to include code that should run if and only if you haven't already bailed out.

A good solution to this problem should be robust in the face of adding new flow control.

In C++, a better solution is throwing together a quick scope_guard class:

#ifndef SCOPE_GUARD_H_INCLUDED_
#define SCOPE_GUARD_H_INCLUDED_
template<typename F>
struct scope_guard_t {
  F f;
  ~scope_guard_t() { f(); }
};
template<typename F>
scope_guard_t<F> scope_guard( F&& f ) { return {std::forward<F>(f)}; }
#endif

then in the code in question:

auto scope = scope_guard( executeThisFunctionInAnyCase );
bool conditionA = executeStepA();
if (!conditionA) return;
bool conditionB = executeStepB();
if (!conditionB) return;
bool conditionC = executeStepC();
if (!conditionC) return;

and the destructor of scope automaticlaly runs executeThisFunctionInAnyCase. You can inject more and more such "at end of scope" (giving each a different name) whenever you create a non-RAII resource that needs cleaning up. It can also take lambdas, so you can manipulate local variables.

Fancier scope guards can support aborting the call in the destructor (with a bool guard), block/allow copy and move, and support type-erased "portable" scope-guards that can be returned from inner contexts.

Upvotes: 1

Joe
Joe

Reputation: 6757

To improve on Mathieu's C++11 answer and avoid the runtime cost incurred through the use of std::function, I would suggest to use the following

template<typename functor>
class deferred final
{
public:
    template<typename functor2>
    explicit deferred(functor2&& f) : f(std::forward<functor2>(f)) {}
    ~deferred() { this->f(); }

private:
    functor f;
};

template<typename functor>
auto defer(functor&& f) -> deferred<typename std::decay<functor>::type>
{
    return deferred<typename std::decay<functor>::type>(std::forward<functor>(f));
}

This simple template class will accept any functor that can be called without any parameters, and does so without any dynamic memory allocations and therefore better conforms to C++'s goal of abstraction without unnecessary overhead. The additional function template is there to simplify use by template parameter deduction (which is not available for class template parameters)

Usage example:

auto guard = defer(executeThisFunctionInAnyCase);
bool conditionA = executeStepA();
if (!conditionA) return;
bool conditionB = executeStepB();
if (!conditionB) return;
bool conditionC = executeStepC();
if (!conditionC) return;

Just as Mathieu's answer this solution is fully exception safe, and executeThisFunctionInAnyCase will be called in all cases. Should executeThisFunctionInAnyCase itself throw, destructors are implicitly marked noexceptand therefore a call to std::terminate would be issued instead of causing an exception to be thrown during stack unwinding.

Upvotes: 4

Falco
Falco

Reputation: 3416

After reading all the answers, I want to provide one new approach, which could be quite clear and readable in the right circumstances: A State-Pattern.

If you pack all Methods (executeStepX) into an Object-class, it can have an Attribute getState()

class ExecutionChain
{
    public:
        enum State
        {
          Start,
          Step1Done,
          Step2Done,
          Step3Done,
          Step4Done,
          FinalDone,
        };
        State getState() const;

        void executeStep1();
        void executeStep2();
        void executeStep3();
        void executeStep4();
        void executeFinalStep();
    private:
        State _state;
};

This would allow you to flatten your execution code to this:

void execute
{
    ExecutionChain chain;

    chain.executeStep1();

    if ( chain.getState() == Step1Done )
    {
        chain.executeStep2();
    }

    if ( chain.getState() == Step2Done )
    {
        chain.executeStep3();
    }

    if ( chain.getState() == Step3Done )
    {
        chain.executeStep4();
    }

    chain.executeFinalStep();
}

This way it is easily readable, easy to debug, you have a clear flow control and can also insert new more complex behaviors (e.g. execute Special Step only if at least Step2 is executed)...

My problem with other approaches like ok = execute(); and if (execute()) are that your code should be clear and readable like a flow-diagram of what is happening. In the flow diagram you would have two steps: 1. execute 2. a decision based on the result

So you shouldn't hide your important heavy-lifting methods inside of if-statements or similar, they should stand on their own!

Upvotes: 1

user823738
user823738

Reputation: 17521

Another approach - do - while loop, even though it was mentioned before there was no example of it which would show how it looks like:

do
{
    if (!executeStepA()) break;
    if (!executeStepB()) break;
    if (!executeStepC()) break;
    ...

    break; // skip the do-while condition :)
}
while (0);

executeThisFunctionInAnyCase();

(Well there's already an answer with while loop but do - while loop does not redundantly check for true (at the start) but instead at the end xD (this can be skipped, though)).

Upvotes: 3

Dr. Sahib
Dr. Sahib

Reputation: 166

while(executeStepA() && executeStepB() && executeStepC() && 0);
executeThisFunctionInAnyCase();

executeThisFunctionInAnyCase() had to be executed in any case even if the other functions do not complete.

The while statement:

while(executeStepA() && executeStepB() && executeStepC() && 0)

will execute all the functions and will not loop as its a definite false statement. This can also be made to retry a certain times before quitting.

Upvotes: 0

user1585121
user1585121

Reputation:

You could also do this:

bool isOk = true;
std::vector<bool (*)(void)> funcs; //vector of function ptr

funcs.push_back(&executeStepA);
funcs.push_back(&executeStepB);
funcs.push_back(&executeStepC);
//...

//this will stop at the first false return
for (auto it = funcs.begin(); it != funcs.end() && isOk; ++it) 
    isOk = (*it)();
if (isOk)
 //doSomeStuff
executeThisFunctionInAnyCase();

This way you have a minimal linear growth size, +1 line per call, and it's easily maintenable.


EDIT: (Thanks @Unda) Not a big fan because you loose visibility IMO :

bool isOk = true;
auto funcs { //using c++11 initializer_list
    &executeStepA,
    &executeStepB,
    &executeStepC
};

for (auto it = funcs.begin(); it != funcs.end() && isOk; ++it) 
    isOk = (*it)();
if (isOk)
 //doSomeStuff
executeThisFunctionInAnyCase();

Upvotes: 19

Leopold Asperger
Leopold Asperger

Reputation: 1015

Just a side note; when an if scope always causes a return (or break in a loop), then don't use an else statement. This can save you a lot of indentation overall.

Upvotes: 2

What Would Be Cool
What Would Be Cool

Reputation: 6758

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:

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.

Upvotes: 6

Karsten
Karsten

Reputation: 1907

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 ;-)

Upvotes: 3

ButterDog
ButterDog

Reputation: 5225

As @Jefffrey said, you can use the conditional short-circuit feature in almost every language, I personally dislike conditional statements with more than 2 condition (more than a single && or ||), just a matter of style. This code does the same (and probably would compile the same) and it looks a bit cleaner to me. You don't need curly braces, breaks, returns, functions, lambdas (only c++11), objects, etc. as long as every function in executeStepX() returns a value that can be cast to true if the next statement is to be executed or false otherwise.

if (executeStepA())
 if (executeStepB())
  if (executeStepC())
   //...
    if (executeStepN()); // <-- note the ';'

executeThisFunctionInAnyCase();

Any time any of the functions return false, none of the next functions are called.

I liked the answer of @Mayerz, as you can vary the functions that are to be called (and their order) in runtime. This kind of feels like the observer pattern where you have a group of subscribers (functions, objects, whatever) that are called and executed whenever a given arbitrary condition is met. This might be an over-kill in many cases, so use it wisely :)

Upvotes: 1

Shoe
Shoe

Reputation: 76240

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

Upvotes: 494

Khaled.K
Khaled.K

Reputation: 5940

Given the function:

string trySomething ()
{
    if (condition_1)
    {
        do_1();
        ..
            if (condition_k)
            {
                do_K();

                return doSomething();
            }
            else
            {
                return "Error k";
            }
        ..
    }
    else
    {
        return "Error 1";
    }
}

We can get rid of the syntactical nesting, by reversing the validation process:

string trySomething ()
{
    if (!condition_1)
    {
        return "Error 1";
    }

    do_1();

    ..

    if (!condition_k)
    {
        return "Error k";
    }

    do_K();

    return doSomething ();
}

Upvotes: -1

Noctis Skytower
Noctis Skytower

Reputation: 22001

If your code is as simple as your example and your language supports short-circuit evaluations, you could try this:

StepA() && StepB() && StepC() && StepD();
DoAlways();

If you are passing arguments to your functions and getting back other results so that your code cannot be written in the previous fashion, many of the other answers would be better suited to the problem.

Upvotes: 6

CarrKnight
CarrKnight

Reputation: 2928

This looks like a state machine, which is handy because you can easily implement it with a state-pattern.

In Java it would look something like this:

interface StepState{
public StepState performStep();
}

An implementation would work as follows:

class StepA implements StepState{ 
    public StepState performStep()
     {
         performAction();
         if(condition) return new StepB()
         else return null;
     }
}

And so on. Then you can substitute the big if condition with:

Step toDo = new StepA();
while(toDo != null)
      toDo = toDo.performStep();
executeThisFunctionInAnyCase();

Upvotes: 4

Related Questions