How to stop writing chain code?
Asked Answered
C

8

9

for example...

if ( /* Condition */ ) {

    if ( /* Condition */ ) {

        if ( /* Condition */ ) {

          // Superb!

        } else {

          // Error 3

        }

    } else {

      // Error 2

    }

} else {

  // Error 1

}

Do you know how to avoid this? Thank you!

Cruise answered 24/11, 2010 at 17:53 Comment(0)
C
11

If this is a library function, throw may be the appropriate action.

if (!condition1) {
    throw "Condition 1 failed.";
}

if (!condition2) {
    throw "Condition 2 failed.";
}

if (!condition3) {
    throw "Condition 3 failed.";
}

// Superb!

Other acceptable actions might be:

  • Returning 0, null, or undefined.
  • Displaying an error to the user and returning.

You will have to determine which failure action is right for your use case.

Chaiken answered 24/11, 2010 at 17:57 Comment(10)
Damn you ninja! 47 seconds? :-(Questa
+1: It's just good practice to avoid the "arrow head" nested conditionals by inverting the conditions to check for the "else" part first. A linear stream of error checks is considerably easier to read and support than a nested stream of success checks.Yan
@David: Nod. One of my university CS professors hated things like return; except at the very end of a function because it was "like goto". So I wrote code like this to piss him off, and actually found out that I prefer how it reads to ridiculous levels of nested conditionals.Chaiken
There are good reasons for a single point of return, especially in older languages like C. Adding a return to the middle of a function may cause a memory leak, because you forgot to free some data. However, a carefully written function has "free" at the end of the function call. In more modern languages, you can use finally and destructors to prevent this kind of mistake. So modern languages don't need the "one return statement" rule anymore. IMO at least.Questa
@Dragontamer5788: A valid point. In C, where I have to free() a bunch of stuff, I usually throw all that code at the end, have a variable hold the return value of the function (defaulted to an error code, and this is assuming a non-void function) and goto the cleanup code from errors. Oh, my CS professor hated that too.Chaiken
@Dragontamer5788: I recently introduced a colleague to the try/finally block (no catch) in C#, much to his delight. It had never occurred to him that he could use it without a catch perform resource de-allocation cleanly without using tricks for logic flow.Yan
@David: Does he know about the using block? With IDisposable objects it's an even cleaner syntax that's just syntactic sugar for try/finally.Chaiken
@cdhowie: Professors often have a nasty habit of instilling poor development practices. I doubt that supporting code is part of their duties, and commanding a platoon of free labor eager to do their bidding likely doesn't help much. I was lucky to have one or two with considerable real-world experience, my only regret was not utilizing them more.Yan
@cdhowie: Yes, using is common practice here. It's not always the cleanest approach, but certainly the more standard. There are always those outlying cases.Yan
@David: I used to get into after-class arguments with this professor all the time. We never got mad at each other, but we had some really interesting discussions. (Example: He hated pointers. So that was a fun one to bring up.)Chaiken
S
2

It looks like you have 3 conditions to check and 4 actions (3 different errors + 1 success). Unfortunately in the general case it's going to require 3 conditional checks and 4 actions. I think the code can be cleaned up a bit by using the following structure though

if (! /* condition 1 */ ) {
  // Error 1
} else if (! /* condition 2 */ ) { 
  // Error 2
} else if (! /* condition 3 */ ) { 
  // Error 3
} else {
  // superb
}
Sprinkling answered 24/11, 2010 at 17:59 Comment(2)
Cleanly stacking the conditions next to the errors using normal language structure in a well ordered fashion. It's beyond me why so many people think the throw approach is better.Stickybeak
@eBusiness Because if there is an error condition and this is a function, throwing an error may very well be the right thing to do. If the three error cases need to be distinguished by callers, you either have to return some object containing the error details (and therefore requiring that this scenario be tested for by the caller) or throw an error. I know which one I prefer.Chaiken
D
1

Well you can use exceptions, or breaks within a block, or multiple functions. Often this requires inverting your conditions to get the code ordered the right way.

do {
    if (! /* Condition 1 */ ) {
        // Error 1
        break;
    }

    if (! /* Condition 2 */ ) {
        // Error 2
        break;
    }

    if (! /* Condition 3 */ ) {
        // Error 3
        break;
    }

    // Superb!
} while (false);

The do-while(false) loop is a way of making a block that you can break out of in languages that will not tolerate an anonymous block. It could just as easily be a function and use returns, or a try-catch with exceptions.

Doctorate answered 24/11, 2010 at 17:58 Comment(6)
And how precisely is this any better than if/else?Garthgartner
@Garthgartner Did I say it was better, I said it was an alternative - I listed others that were better and showed the most unusual approach that people would not have seen. Other answers have shown other approaches. The if-else set answers duplicate conditions which I dislike.Doctorate
It does not answer duplicate conditions: if(!Condition1){ERROR1}else if(!Condition2){ERROR2}else if(!Condition3){ERROR3}else{SUCCESS}Garthgartner
There are no duplicate conditions. This style of do{}while(false) is a throwback to C and other similar languages which have memory management concerns and no exceptions. Even still, it was ugly then, it's ugly now.Garthgartner
To be clear this is a javascript question, memory management is implicit. This suggestion is outdated and bad for the language it's proposed with.Garthgartner
Memory management was not an issue in my comment at all, only repetition of code. Many of the answers have if (c1 && c2 && c3) { SUCCESS } elseif (c1) { .... } etc; } which repeats the conditions. I did not notice the javascript tag, it wasn't mentioned in the question, so I thought the question was language agnostic. C is not an outdated language by the way, it is still very heavily used. Anyhow, I was showing an alternative approach - I usually do - it is not a "bad" way, there are no performance or clarity issues, just not your preference.Doctorate
N
0

Would you prefer this?

if ( /* Condition 1*/ && /* Condition 2*/ && /* Condition 3 */) {
  // Superb!
}
else if (! /* Condition 1*/){
  // Error 1
}
else if (! /* Condition 2*/){
  // Error 2
}
else if (! /* Condition 3*/){
  // Error 3
}
Nightcap answered 24/11, 2010 at 17:57 Comment(0)
Q
0
if ( ! /* Condition */ ) {
Error 1
throw someSortOfException
}

if (! condition 2){
Error 2
throw someSortOfOtherException
}

if (! condition 3){
Error 3
throw another Exception
}

// Success!

Depending on the situation, your code may be prefered. You'll probably want to catch those exceptions somewhere for example.

Questa answered 24/11, 2010 at 17:57 Comment(0)
A
0
if (!condition1)
    // Error 1 (and exit scope if necessary)

if (!condition2)
    // Error 2 (and exit scope if necessary)

if (!condition3)
    // Error 3 (and exit scope if necessary)

// Superb!
Abbyabbye answered 24/11, 2010 at 17:58 Comment(0)
S
0

Yes; you can compose together your if statements into a single compound statement using the AND operator, and handle the error conditions in a single block. Depending on your error handling needs, though, you may need to again have multiple ifs to make sure you're properly handling the error (it depends on your error handling resolution, really).

To wit:

if (CondA && CondB && CondC)
   {
   // Execute success
   }
else
   {
   if (!CondA)
      // Do error A
   else if (!CondB)
      // Do error B
   else if (!CondC)
     // Do error C
   }
}
Scholem answered 24/11, 2010 at 17:58 Comment(0)
G
0

There are a few ways, the simplest is to simply bust out some functions and abstract out the different layers (this should be done anyway if you find yourself going deep.)

if ( /* Condition */ ) {
    value = aFunctionSaysWhat();
} else {
  // value = Error 1
}

....
value aFunctionSaysWhat(){
    if ( /* Condition */ ) {
         return aSecondFunctionHere();
    } else {
      // return Error 2
    }
}

The basic premise is that a function should live on one layer of abstraction if possible, and do one thing.

The next possibility is to flatten it all out, this is superior to your initial nested method, but basically has similar complexity. It could be much cleaner than the functional approach if you only have a few options and you don't plan on adding more.

if(ErrorCondition1){
   //Error 1
}else if(ErrorCondition2){
   //Error 2
}else if(ErrorCondition3){
   //Error 3
}else{
   //Superb
}

Finally, you can store a hash or map with the desired answers and remove the if completely, the ability to implement this relies on your ability to hash some result:

Results = {'Result1':'Error1', 'Result2':'Error2', 'Result3':'Error3', 'Success':'Superb'}

return Results[ConditionHash(Condition)];
Garthgartner answered 24/11, 2010 at 17:59 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.