ChatGPT解决这个技术问题 Extra ChatGPT

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?

@FrédéricHamidi wrong wrong wrong! Do not ever say that driving your program flow with exceptions is good! Exceptions are definitely NOT suited for this purpose, for too many reasons.
@Piotr, I was spoiled by Python (which actually encourages this). I know exceptions are not supposed to be used for flow control in C++, but is it really flow control here? Couldn't a function returning false be considered as akin to an exceptional situation?
That depends on the semantics of a program. A false return can be pretty normal.
I've rolled back your question to its first revision. You should not change your question radically after you have received a certain number of questions (> 0), because that will invalidate all the answers given up to that moment and will create confusion. Open a new question instead.
I wish all “newbie programmers” would ask design questions like this.

D
David

You can use an && (logic AND):

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

this will satisfy both of your requirements:

executeStep() should evaluate only if the previous one succeeded (this is called short circuit evaluation)

executeThisFunctionInAnyCase() will be executed in any case


This is both semantically correct (indeed, we want ALL conditions to be true) as well as a very good programming technique (short circuit evaluation). Furthermore, this can be used in any complex situation where creating functions would mess up the code.
@RobAu: Then it will be good for the junior programmers to finally see code that relies on short-cut evaluation and may prompt them to reseach this topic which in turn would help them on their way to eventually become senior programmers. So clearly a win-win: decent code and somebody learned something from reading it.
This should be the top answer
@RobAu: This is not a hack taking advantage of some obscure syntax trick, it's highly idiomatic in nearly every programming language, to the point of being undebatably standard practice.
This solution only applies if conditions are actually that simple function calls. In real code, those conditions may be 2-5 lines long (and themselves be combinations of many other && and ||) so there is no way you would want to join them into single if statement without screwing readability. And it is not always easy to move those conditions to outer functions, because they may be dependent on a lot of previously calculated local variables, which would create a terrible mess if you try to pass each one as individual argument.
l
ltjax

Just use an additional function to get your second version to work:

void foo()
{
  bool conditionA = executeStepA();
  if (!conditionA) return;

  bool conditionB = executeStepB();
  if (!conditionB) return;

  bool conditionC = executeStepC();
  if (!conditionC) return;
}

void bar()
{
  foo();
  executeThisFunctionInAnyCase();
}

Using either deeply nested ifs (your first variant) or the desire to break out of "part of a function" usually means you do need an extra function.


+1 finally someone posted a sane answer. This is the most correct, safe and readable way, in my opinion.
+1 This is a good illustration of the "Single Responsibility Principle". Function foo works its way through a sequence of related conditions and actions. Function bar is cleanly separated from the decisions. If we saw the details of the conditions and actions, it might turn out that foo is still doing too much, but for now this is a good solution.
The downside is that C doesn't have nested functions so if those 3 steps needed to use variables from bar you would be have to manually pass them to foo as parameters. If that is the case and if foo is only called once I would err towards using the goto version to avoid defining two tightly coupled functions that would end up not being very reusable.
Not sure of the short-circuiting syntax for C, but in C# foo() could be written as if (!executeStepA() || !executeStepB() || !executeStepC()) return
@user1598390 Goto's are used all the time, especially in systems programming when you need to unwind a lot of set-up code.
c
cmaster - reinstate monica

Old school C programmers use goto in this case. It is the one usage of goto that's actually encouraged by the Linux styleguide, it's called the centralized function exit:

int foo() {
    int result = /*some error code*/;
    if(!executeStepA()) goto cleanup;
    if(!executeStepB()) goto cleanup;
    if(!executeStepC()) goto cleanup;

    result = 0;
cleanup:
    executeThisFunctionInAnyCase();
    return result;
}

Some people work around using goto by wrapping the body into a loop and breaking from it, but effectively both approaches do the same thing. The goto approach is better if you need some other cleanup only if executeStepA() was successfull:

int foo() {
    int result = /*some error code*/;
    if(!executeStepA()) goto cleanupPart;
    if(!executeStepB()) goto cleanup;
    if(!executeStepC()) goto cleanup;

    result = 0;
cleanup:
    innerCleanup();
cleanupPart:
    executeThisFunctionInAnyCase();
    return result;
}

With the loop approach you would end up with two levels of loops in that case.


+1. A lot of people see a goto and immediately think "This is terrible code" but it does have its valid uses.
Except this is actually quite messy code, from a maintenance point-of-view. Particularly when there are multiple labels, where the code also turns harder to read. There are more elegant ways to achieve this: use functions.
-1 I see the goto and I don't even have to think to see that this is terrible code. I once had to maintain such, it's very nasty. The OP suggests a reasonable C language alternative at the end of the question, and I included that in my answer.
There's nothing wrong with this limited, self-contained use of goto. But beware, goto is a gateway drug and if you're not careful you will one day realize that no one else eats spaghetti with their feet, and you've been doing it for 15 years starting after you thought "I can just fix this otherwise nightmarish bug with a label ..."
I have written probably a hundred thousand lines of extremely clear, easy-to-maintain code in this style. The two key things to understand are (1) discipline! establish a clear guideline for the layout of every function and only violate it at extreme need, and (2) understand that what we are doing here is simulating exceptions in a language that does not have them. throw is in many ways worse than goto because with throw it's not even clear from the local context where you're going to end up! Use the same design considerations for goto-style control flow as you would for exceptions.
J
John Wu

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

The { and } characters for a given logic block are closer together The amount of mental context needed to understand a particular line is smaller The entirety of logic associated with an if condition is more likely to be on one page 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:

It rubs some people the wrong way, e.g. people who learned to code on Pascal have learned that one function = one exit point. 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.


Finally? C++ does not have a finally clause. An object with a destructor is used in situations where you think you need a finally clause.
Edited to accommodate the two comments above.
With trivial code (e.g. in my example) nested patterns are probably more comprehensible. With real world code (which could span several pages), the guard pattern is objectively easier to read because it will require less scrolling and less eye tracking, e.g. the average distance from { to } is shorter.
I have seen nested pattern where the code wasn't visible on a 1920 x 1080 screen anymore... Try finding out what error handling code will be performed if the third action failed... I have used do { ... } while (0) instead so you don't need the final break (on the other hand, while (true) { ... } allows "continue" to start all over again.
Your option 4 is a memory leak in C++ actually (ignoring the minor syntax error). One doesn't use "new" in this kind of case, just say "MyContext myContext;".
C
Cheers and hth. - Alf

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

I answered this (more concisely) in response to the first version of the question, and it got 20 upvotes before it was deleted by Bill the Lizard, after some commentary and editing by Darkness Races in Orbit.
@Cheersandhth-Alf: I can't believe it was mod-deleted. That's just too bad. (+1)
My +1 to your courage! (3 times does matter :D).
It's completely important that newbie programmers learn about things like order-execution with multiple boolean "ands", how that is different in different languages etc. And this answer is a great pointer to that. But it's just a "non-starter" in normal commercial programming. It's not simple, it's not maintainable, it's not modifiable. Sure, if you're just writing some quick "cowboy code" for yourself, do this. But it just has sort of "no connection to everyday software engineering as practiced today." BTW sorry for the ridiculous "edit-confusion" you had to endure here, @cheers :)
@JoeBlow: I'm with Alf on this - I find the && list much clearer. I commonly break the conditions on to separate lines and add a trailing // explanation to each.... In the end, it's massively less code to look at, and once you understand how && works there's no ongoing mental effort. My impression is that most professional C++ programmers would be familiar with this, but as you say in different industries/projects the focus and experience differs.
M
Matthieu M.

There is actually a way to defer actions in C++: making use of an object's destructor.

Assuming that you have access to C++11:

class Defer {
public:
    Defer(std::function<void()> f): f_(std::move(f)) {}
    ~Defer() { if (f_) { f_(); } }

    void cancel() { f_ = std::function<void()>(); }

private:
    Defer(Defer const&) = delete;
    Defer& operator=(Defer const&) = delete;

    std::function<void()> f_;
}; // class Defer

And then using that utility:

int foo() {
    Defer const defer{&executeThisFunctionInAnyCase}; // or a lambda

    // ...

    if (!executeA()) { return 1; }

    // ...

    if (!executeB()) { return 2; }

    // ...

    if (!executeC()) { return 3; }

    // ...

    return 4;
} // foo

This is just complete obfuscation to me. I really don't understand why so many C++ programmers like to solve a problem by tossing as many language features at it as possible, until the point where everyone has forgotten about what problem you were actually solving: they no longer care, because they are now so interested in using all those exotic language features. And from there on, you can keep yourself busy for days and weeks writing meta code, then maintaining the meta code, then write meta code for handling the meta code.
@Lundin: Well, I don't understand how one can be satisfied with brittle code that will break as soon as an early continue/break/return is introduced or an exception is thrown. On the other hand, this solution is resilient in the face of future maintenance, and only relies on the fact that destructors are executed during unwinding which is one of the most important feature of C++. Qualifying this of exotic while it is the underlying principle that powers all the Standard containers is amusing, to say the least.
@Lundin: The benefit of Matthieu's code is that executeThisFunctionInAnyCase(); will execute even if foo(); throws an exception. When writing exception-safe code, it is good practice to put all such cleanup functions in a destructor.
@Brian Then don't throw any exception in foo(). And if you do, catch it. Problem solved. Fix bugs by fixing them, not by writing work-arounds.
@Lundin: the Defer class is a reusable little piece of code that lets you do any end-of-block cleanup, in an exception safe way. it's more commonly known as a scope guard. yes any use of a scope guard can be expressed in other more manual ways, just like any for loop can be expressed as a block and a while loop, which in turn can be expressed with if and goto, which can be expressed in assembly language if you want, or for those who are true masters, by altering the bits in memory via cosmic rays directed by butterfly effect of special short grunts and chants. but, why do that.
T
Tim Visée

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);
  // ...
}

Using a function with multiple returns is similar, but more readable, in my opinion.
Yes, definitely it's more readable... however from an efficiency point of view the additional function call (parameter passing and return) overhead can be avoided with the do {} while (0) construct.
You still are free to make the function inline. Anyway, this is a nice technique to know, because it helps in more than this problem.
@Lundin You have to take code locality into account, spreading code out over too many places has it's issues too.
In my experience, this is a very unusual idiom. It would take me a while to figure out what the heck was going on, and that's a bad sign when I'm reviewing code. Given that it appears to have no advantages over the other, more common and therefore more readable approaches, I couldn't sign off on it.
佚名

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

It was about the accidental function calls inside push_back(), but you fixed it anyway :)
I'd love to know why this got a downvote. Assuming the execute steps are indeed symmetric, as they appear to be, then it's clean and maintainable.
Though this might arguably look cleaner. It might be harder to understand for people, and is definitely harder to understand for the compiler!
Treating your functions more as data is often a good idea--once you've done it you will also notice subsequent refactorings. Even better is where each piece you are handling is an object reference, not just a function reference--this will give you even more ability to improve your code down the line.
Slightly over-engineered for a trivial case, but this technique definitively has a nice feature others don't: You can change the order of execution and [number of] the functions called at runtime, and that is nice :)
s
sampathsris

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

I usually call the variable ok when using the same variable like this.
I would be very interested in knowing why the downvotes. What's going wrong here?
compare ur answer with short circuit approach for cyclomatic complexity.
C
ClickRick

Assuming the desired code is as I currently see it:

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

I would say that the correct approach, in that it's the simplest to read and easiest to maintain, would have fewer levels of indentation, which is (currently) the stated purpose of the question.

// Pre-declare the variables for the conditions
bool conditionA = false;
bool conditionB = false;
bool conditionC = false;

// Execute each step only if the pre-conditions are met
conditionA = executeStepA();
if (conditionA)
    conditionB = executeStepB();
if (conditionB)
    conditionC = executeStepC();
if (conditionC) {
    ...
}

// Unconditionally execute the 'cleanup' part.
executeThisFunctionInAnyCase();

This avoids any need for gotos, exceptions, dummy while loops, or other difficult constructs and simply gets on with the simple job at hand.


When using loops, its usually acceptable to use return and break to jump out of the loop without needing to introduce extra "flag" variables. In this case, using a goto would be similarly innocuou - remember that you are trading extra goto complexity for extra mutable variable complexity.
@hugomg The variables were in the original question. There is no extra complexity here. There were assumptions made about the question (e.g. that the variables are need in the redacted code) so they were retained. If they are not needed then the code can be simplified, but given the incomplete nature of the question there is no other assumption which can validly be made.
Very useful approach, esp. for use by a self professed newbie, it provides a cleaner solution with no drawbacks. I note it also not does not rely on the steps having the same signature or even being functions rather than blocks. I can see this being used as a first pass refactor even where a more sophisticated approach is valid.
o
ouah

Could break statement be used in some way?

Maybe not the best solution but you can put your statements in a do .. while (0) loop and use break statements instead of return.


It wasn't I who downvoted it, but this would be an abuse of a looping construct for something where the effect is what's currently wanted but would inevitably result in pain. Probably for the next developer who has to maintain it in 2 years time after you've moved on to another project.
@ClickRick using do .. while (0) for macro definitions is also abusing loops but it is considered OK.
Perhaps, but there are cleaner ways to achieve it.
@ClickRick the only purpose of my answer is to answer Could break statement be used in some way and the answer is yes, the first words in my answer suggests this may be not the solution to use.
This answer should just be a comment
N
Niall

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

A
Alexander Oh

If you dislike goto and dislike do { } while (0); loops and like to use C++ you can also use a temporary lambda to have the same effect.

[&]() { // create a capture all lambda
  if (!executeStepA()) { return; }
  if (!executeStepB()) { return; }
  if (!executeStepC()) { return; }
}(); // and immediately call it

executeThisFunctionInAnyCase();

if you dislike goto && you dislike do { } while (0) && you like C++ ... Sorry, couldn't resist, but that last condition fails because the question is tagged c as well as c++
@ClickRick it's always difficult to please everybody. In my opinion there is no such thing as C/C++ you usually code in either one of those and the usage of the other is frowned.
R
Roman Mik

The chains of IF/ELSE in your code is not the language issue, but the design of your program. If you're able to re-factor or re-write your program I'd like to suggest that you look in Design Patterns (http://sourcemaking.com/design_patterns) to find a better solution.

Usually, when you see a lot of IF's & else's in your code , it is an opportunity to implement the Strategy Design Pattern (http://sourcemaking.com/design_patterns/strategy/c-sharp-dot-net) or maybe a combination of other patterns.

I'm sure there're alternatives to write a long list of if/else , but I doubt they will change anything except that the chain will look pretty to you (However, the beauty is in the eye of the beholder still applies to code too:-) ) . You should be concerned about things like (in 6 months when I have a new condition and I don't remember anything about this code , will I be able to add it easily? Or what if the chain changes, how quickly and error-free will I be implement it)


C
Community

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.

https://i.stack.imgur.com/glq81.png

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

https://i.stack.imgur.com/trGXZ.png

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.


You missed the second best voted answer.
Did you put the bounty to get more rep? :)
Instead of putting all those comments in the if, you could also use more descriptive function names and put the comments in the functions.
Yuck. I'll take the 1 line solution which reads concisely with short-circuit evaluation (which has been in languages for over 20 years and is well known) over this mess any day. I think we can both agree we are happy not working with each-other.
R
Rik

Have your execute functions throw an exception if they fail instead of returning false. Then your calling code could look like this:

try {
    executeStepA();
    executeStepB();
    executeStepC();
}
catch (...)

Of course I'm assuming that in your original example the execution step would only return false in the case of an error occuring inside the step?


using exception to control flow is often consider as bad practice and smelly code
b
blgt

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;

Why ok &= conditionX; and not simply ok = conditionX;?
@user3253359 In many cases, yes, you can do that. This is a conceptual demo; in working code we'd try to simplify it as much as possible
+1 One of the few clean and maintainable answers which works in c, as stipulated in the question.
c
celtschk

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

Refactoring value checks into exceptions is not necessarily a good way to go, there is considerable overhead unwinding exceptions.
-1 Using exceptions for normal flow in C++ like this is poor programming practice. In C++ exceptions should be reserved for exceptional circumstances.
From the question text (emphasis by me): "Functions executeStepX should be executed if and only if the previous succeed." In other words, the return value is used to indicate failure. That is, this is error handling (and one would hope that failures are exceptional). Error handling is exactly what exceptions were invented for.
Nope. Firstly, exceptions were created to allow error propagation, not error handling; secondly, "Functions executeStepX should be executed if and only if the previous succeed." doesn't mean that boolean false returned by previous function indicates an obviously exceptional/erroneous case. Your statement is thus non sequitur. Error handling and flow sanitization can be implemented by many different means, exceptions are a tool allowing error propagation and out-of-place error handling, and excel at that.
N
Noctis Skytower

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.


In fact I edited my question to better explain the topic, but it was rejected to do not invalidate most answers. :\
I'm a new user in SO, and a newbie programmer. 2 questions then: is there the risk that an another question like the one you said will be marked as duplicated because of THIS question? Another point is: how could a newbie SO user/programmer choose the best answer between all there (almost good I suppose..)?
g
glampert

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.


There is no need for macros to make this work. And [=] is usually wrong for a scoped lambda.
Yes, the macros are just for decoration and can be thrown away. But wouldn't you say that capture by value is the safest "generic" approach?
no: if your lambda will not persist beyond the current scope where the lambda is created, use [&]: it is safe, and minimally surprising. Capture by value only when the lambda (or copies) could survive longer than the scope at the point of declaration...
Yes, that makes sense. I'll change it. Thanks!
r
rageandqq

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

M
Macke

Assuming you don't need individual condition variables, inverting the tests and using the else-falthrough as the "ok" path would allow you do get a more vertical set of if/else statements:

bool failed = false;

// keep going if we don't fail
if (failed = !executeStepA())      {}
else if (failed = !executeStepB()) {}
else if (failed = !executeStepC()) {}
else if (failed = !executeStepD()) {}

runThisFunctionInAnyCase();

Omitting the failed variable makes the code a bit too obscure IMO.

Declaring the variables inside is fine, no worry about = vs ==.

// keep going if we don't fail
if (bool failA = !executeStepA())      {}
else if (bool failB = !executeStepB()) {}
else if (bool failC = !executeStepC()) {}
else if (bool failD = !executeStepD()) {}
else {
     // success !
}

runThisFunctionInAnyCase();

This is obscure, but compact:

// keep going if we don't fail
if (!executeStepA())      {}
else if (!executeStepB()) {}
else if (!executeStepC()) {}
else if (!executeStepD()) {}
else { /* success */ }

runThisFunctionInAnyCase();

C
CarrKnight

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

W
What Would Be Cool

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:

https://i.stack.imgur.com/lkall.png

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.


What is the benefit of this over something like Jefffrey's answer?
far more change resilient, when the requirements change this approach is simpler to manage without a lot of domain knowledge. Especially when you consider how deep and long some sections of nested ifs can get. It can all get very fragile and thus high risk to work with. Don't get me wrong some optimization scenarios may result in you ripping this out and going back to ifs but 99% of the time this is fine. But the point is when you get to that level you don't care about maintainability you need the performance.
B
Bill Door

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.


J
Joe

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.


+1 I was looking for this answer so I wouldn't have to post it. You should perfect-forward functor in deferred'd constructor, no need to force a move.
@Yakk changed the constructor to a forwarding constructor
A
AxFab

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

Fortunately, the debugger spots them for you. If you're debugging and not single-stepping through code, you're doing it wrong.
I don't understand what you mean, neither why I couldn't use single-stepping ?
A
Arkady

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.


H
Hot Licks

Here's a trick I've used on several occasions, in both C-whatever and Java:

do {
    if (!condition1) break;
    doSomething();
    if (!condition2) break;
    doSomethingElse()
    if (!condition3) break;
    doSomethingAgain();
    if (!condition4) break;
    doYetAnotherThing();
} while(FALSE);  // Or until(TRUE) or whatever your language likes

I prefer it over nested ifs for the clarity of it, especially when properly formatted with clear comments for each condition.


In Java I would just solve this with returns and a finally block.
K
Karsten

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


I think this idea cannot replace every if-chain. Anyway, in many cases, this is a very good approach!