Do else blocks increase code complexity? [on hold]

In my view, the explicit else block is preferable. When I see this:

if (sky.Color != Blue) {
   ...
}  else {
   ...
}

I know that I'm dealing with mutually exclusive options. I don't need to read whats inside the if blocks to be able to tell.

When I see this:

if (sky.Color != Blue) {
   ...
} 
return false;

It looks, on first glance, that it returns false regardless and has an optional side effect the sky isn't blue.

As I see it, the structure of the second code doesn't reflect what the code actually does. That's bad. Instead, choose the first option which reflects the logical structure. I don't want to have to check for return/break/continue logic in each block to know the logical flow.

Why anyone would prefer the other is a mystery to me, hopefully someone will take the opposite position will enlighten me with his answer.

I forgot to mention a case in which I agree with the omission of an else block, and is when using an if block to apply a constraint that must be logically satisfied for all following code, such as a null-check (or any other guards).

I'd say that guard conditions are okay in the case that you've left the happy path. An error or failure has occurred which prevents the ordinary execution from continuing. When this happens you should throw an exception, return an error code, or whatever is appropriate for your language.

The principle reason for removing the else block that I have found is excess indenting. The short-circuiting of else blocks enables a much flatter code structure.

Many if statements are essentially guards. They're preventing the dereferencing of null pointers and other "don't do this!" errors. And they lead to a quick termination of the current function. For example:

if (obj === NULL) {
    return NULL;
}
// further processing that now can assume obj attributes are set

Now it may seem that an else clause would be very reasonable here:

if (obj === NULL) {
    return NULL;
}
else if (obj.color != Blue) {
   // handle case that object is not blue
}

And indeed, if that's all you're doing, it's not much more indented than the example above. But, this is seldom all you're doing with real, multi-level data structures (a condition that occurs all the time when processing common formats like JSON, HTML, and XML). So if you want to modify the text of all children of a given HTML node, say:

elements = tree.xpath(".//p");
if (elements === NULL) {
    return NULL;
}
p = elements[0]
if ((p.children === NULL) or (p.children.length == 0)) {
    return NULL;
}
for (c in p.children) {
    c.text = c.text.toUpperCase();
}
return p;

The guards do not increase the indentation of the code. With an else clause, all of the actual work starts moving over to the right:

elements = tree.xpath(".//p");
if (elements === NULL) {
    return NULL;
}
else {
    p = elements[0]
    if ((p.children === NULL) or (p.children.length == 0)) {
        return NULL;
    }
    else {
        for (c in p.children) {
            c.text = c.text.toUpperCase();
        }
        return p;
    }
}

This is starting to have significant rightward motion, and this is a pretty trivial example, with only two guard conditions. Every additional guard adds another indentation level. Real code I have written processing XML or consuming detailed JSON feeds can easily stack up 3, 4, or more guard conditions. If you always use the else, you end up indented 4, 5, or 6 levels in. Maybe more. And that definitely contributes to a sense of code complexity, and more time spent understanding what lines up with what. The quick, flat, early-exit style eliminates some of that "excess structure," and seems simpler.

Addendum Comments on this answer made me realize that it might not have been clear that NULL/empty handling is not the only reason for guard conditionals. Not nearly! While this simple example focuses on NULL/empty handling and doesn't contain other reasons, searching deep structures such as XML and ASTs for "relevant" content, for example, often has a long series of specific tests to weed out irrelevant nodes and uninteresting cases. A series of "if this node is not relevant, return" tests will cause the same kind of rightward drift that NULL/empty guards will. And in practice, subsequent relevance tests are frequently coupled with NULL/empty guards for the correspondingly deeper data structures searched--a double whammy for rightward drift.

Although the if-else case described has the greater complexity, in most (but not all) practical situations it does not matter much.

Years ago when I was working on software that was used for the aviation industry (it had to go through a certification process), yours was a question that came up. It turned out that there was a financial cost to that 'else' statement as it increased the code complexity.

Here's why.

The presence of the 'else' created an implicit 3rd case that had to be evaluated--that of neither the 'if' nor the 'else' being taken. You might be thinking (like I was at the time) WTF?

The explanation went like this ...

From a logical standpoint, there are only the two options--the 'if' and the 'else'. However, what we were certifying was the object file the compiler was generating. Therefore, when there was an 'else', an analysis had to be done to confirm that the generated branching code was correct and that a mysterious 3rd path did not appear.

Contrast this to the case where there is only an 'if' and no 'else'. In that case, there are only two options--the 'if' path and the 'non-if' path. Two cases to evaluate is less complex than three.

Hope this helps.

The most important goal of code is to be understandable as committed (as opposed to easily refactored, which is useful but less important). A slightly more complex Python example can be used to illustrate this:

def value(key):
    if key == FROB:
        return FOO
    elif key == NIK:
        return BAR
    [...]
    else:
        return BAZ

This is pretty clear - it's the equivalent of a case statement or dictionary lookup, the higher-level version of return foo == bar:

KEY_VALUES = {FROB: FOO, NIK: BAR, [...]}
DEFAULT_VALUE = BAZ

def value(key):
    return KEY_VALUES.get(key, default=DEFAULT_VALUE)

This is clearer, because even though the number of tokens is higher (32 vs 24 for the original code with two key/value pairs), the semantics of the function is now explicit: It is just a lookup.

(This has consequences for refactoring - if you wanted to have a side effect if key == NIK, you have three choices:

1. Revert to the if/elif/else style and insert a single line in the key == NIK case.
2. Save the lookup result to a value and add an if statement to value for the special case before returning.
3. Put an if statement in the caller.

Now we see why the lookup function is a powerful simplification: It makes the third option obviously the simplest solution, since in addition to resulting in a smaller difference than the first option it's the only one which makes sure that value does only one thing.)

Going back to OP's code, I think this can serve as a guide for the complexity of else statements: Code with an explicit else statement is objectively more complex, but more important is whether it makes the semantics of the code clear and simple. And that has to be answered on a case by case basis, since every piece of code has a different meaning.

I think it depends on what kind of if statement you're using. Some if statements can be looked at as expressions, and others can only be seen as control-flow statements.

Your example looks like an expression to me. In C-like languages, the ternary operator ?: is very much like an if statement, but it is an expression, and the else part can't be omitted. It makes sense that an else branch can't be omitted in an expression, because the expression must always have a value.

Using the ternary operator, your example would look like this:

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue)
               ? true
               : false;
}

Since it's a boolean expression, though, it really ought to be simplified all the way to

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue);
}

Including an explicit else clause makes your intent clearer. Guards can make sense in some situations, but in selecting between two possible values, neither of which is exceptional or unusual, they don't help.

One other thing to think about in this argument is the habits each approach promotes.

• if/else reframes a coding sample in the terms of branches. As you say, sometimes this explicitness is good. There really are two possible execution paths and we want to highlight that.

• In other cases, there's only one execution path and then all those exceptional things that programmers must worry about. As you mentioned, guard clauses are great for this.

• There is, of course, the 3 or more case (n), where we usually encourage abandoning the if/else chain and using a case/switch statement or polymorphism.

The guiding principle I keep in mind here is that a method or function should have only one purpose. Any code with an if/else or switch statement is for branching only. So it should be absurdly short (4 lines) and only delegate to the correct method or produce the obvious result (like your example). When your code is that short, it's hard to miss those multiple returns :) Much like "goto considered harmful", any branching statement can be abused, so putting some critical thought into else statements is usually worth it. As you mentioned, just having an else block provides a place for code to clump. You and your team will have to decide how you feel about that.

What the tool is really complaining about is the fact that you have a return/break/throw inside the if block. So as far as it's concerned, you wrote a guard clause but screwed it up. You can choose to make the tool happy or you can "entertain" its suggestion without accepting it.

They add visual cluttering in the code, so yes, they might add complexity.

However, the opposite is also true, excessive refactoring to reduce the code length can also add complexity (not in this simple case of course).

I agree with the statement that the else block states intent. But my conclusion is different, because you're adding complexity in your code in order to achieve this.

I disagree with your statement that it allows you to prevent subtle mistakes in logic.

Let's see an example, now it should only return true if the Sky is Blue and there is high Humidity, the Humidity is not high or if the Sky is Red. But then, you mistake one brace and place it in the wrong else:

bool CanLeaveWithoutUmbrella() {
    if(sky.Color == Color.Blue)
    {
        if (sky.Humidity == Humidity.High) 
        {
            return true;
        } 
    else if (sky.Humidity != Humidity.High)
    {
        return true;
    } else if (sky.Color == Color.Red) 
    {
            return true;
    }
    } else 
    {
       return false;
    }
}

We've seen all this kind of silly mistakes in production code and can be very difficult to detect.

I would suggest the following:

I find this easier to read, because I can see at a glance that the default is false.

Also, the idea of forcing to move the contents of a block to insert a new clause, is prone to errors. This somehow relates to the open/closed principle (code should be open for extension but closed for modification). You're forcing to modify the existing code (e.g. the coder might introduce an error in braces balancing).

Finally, you're leading people to answer in a predetermined way that you think is the right one. By instance, you present a very simple example but afterwards, you state that people should not take it into consideration. However, the simplicity of the example affects the whole question:

• If the case is as simple as the presented one, then my answer would be to omit any if else clauses at all.

  return (sky.Color == Color.Blue);

• If the case is a bit more complicated, with various clauses, I would answer:

  bool CanLeaveWithoutUmbrella() {

  if(sky.Color == Color.Blue && sky.Humidity == Humidity.High) {
      return true;
  } else if (sky.Humidity != Humidity.High) {
      return true;
  } else if (sky.Color == Color.Red) {
      return true;
  }
  
  return false;
  

  }

• If the case is complicated and not all clauses return true (maybe they return a double), and I want to introduce some breakpoint or loggin command, I would consider something like:

  double CanLeaveWithoutUmbrella() {
  
      double risk = 0.0;
  
      if(sky.Color == Color.Blue && sky.Humidity == Humidity.High) {
          risk = 1.0;
      } else if (sky.Humidity != Humidity.High) {
          risk = 0.5;
      } else if (sky.Color == Color.Red) {
          risk = 0.8;
      }
  
      Log("value of risk:" + risk);
      return risk;
  

  }

• If we're not talking about a method which returns something, but instead a if-else block which sets some variable or calls a method, then yes, I would include a final else clause.

Therefore, simplicity of the example is also a factor to consider.

I think the answer is it depends. Your code sample (as you indirectly point out) is simply returning the evaluation of a condition, and is best represented as you obviously know, as a single expression.

In order to determine whether adding an else condition clarifies or obscures the code, you need to determine what the IF/ELSE represents. Is it a (as in your example) an expression? If so, that expression should be extracted and used. Is it a guard condition? If so, then the ELSE is unnecessary and misleading, as it makes the two branches appear equivalent. Is it an example of procedural polymorphism? Extract it. Is it setting preconditions? Then it can be essential.

Before you decide to include or elimitate the ELSE, decide what it represents, that will tell you whether it should be present or not.

The answer is a bit subjective. An else block can aid readability by establishing that one block of code executes exclusively to another block of code, without needing to read through both blocks. This can be helpful if, say, both the blocks are relatively long. There are cases, though where having ifs without elses can be more readable. Compare the following code with a number of if/else blocks:

if(a == b) {
    if(c <= d) {
        if(e != f) {
            a.doSomeStuff();
            c.makeSomeThingsWith(b);
            f.undo(d, e);
            return 9;
        } else {
            e++;
            return 3;
        }
    } else {
        return 1;
    }
} else {
    return 0;
}

with:

if(a != b) {
    return 0;
}

if(c > d) {
    return 1;
}

if(e != f) {
    e++;
    return 3;
}

a.doSomeStuff();
c.makeSomeThingsWith(b);
f.undo(d, e);
return 9;

The second block of code changes the nested if statements into guard clauses. This reduces indentation, and makes some of the return conditions clearer ("if a != b, then we return 0!")

It has been pointed out in the comments that there may be some holes in my example, so I will flesh it out a little more.

We could have written out the first block of code here like so to make it more readable:

if(a != b) {
    return 0;
} else if(c > d) {
    return 1;
} else if(e != f) {
    e++;
    return 3;
} else {
    a.doSomeStuff();
    c.makeSomeThingsWith(b);
    f.undo(d, e);
    return 9;
}

This code is equivalent to both the blocks above, but it presents some challenges. The else clause here connects these if statements together. In the guard clause version above, the guard clauses are independent of each other. Adding a new one means simply writing a new if between the last if and the rest of the logic. Here, that can also be done, with only a slight amount more cognitive load. The difference, though, is when some additional logic needs to occur before that new guard clause. When the statements were independent, we could do:

...
if(e != f) {
    e++;
    return 3;
}

g.doSomeLogicWith(a);

if(!g.isGood()) {
    return 4;
}

a.doSomeStuff();
...

With the connected if/else chain, this is not so easy to do. As a matter of fact, the easiest path to take would be to break up the chain to look more like the guard clause version.

But really, this makes sense. Using if/else weakly implies a relationship between parts. We could surmise that a != b is somehow related to c > d in the if/else chained version. That supposition would be misleading, as we can see from the guard clause version that they are, in fact, not related. That means that we can do more with independent clauses, like rearrange them (perhaps to optimize query performance or to improve logical flow). We can also cognitively ignore them once we are past them. In an if/else chain, I am less sure that the equivalence relationship between a and b won't pop back up later.

The relationship implied by else is also apparent in the deeply-nested example code, but in a different way. The else statements are separated from the conditional they are attached to, and they are related in reverse order to the conditionals (the first else is attached to the last if, the last else is attached to the first if). This creates a cognitive dissonance where we have to backtrack through the code, trying to line up indentation in our brains. It's a little like trying to match up a bunch of parenthesis. It gets even worse if the indentation is wrong. Optional braces don't help either.

I forgot to mention a case in which I agree with the omission of an else block, and is when using an if block to apply a constraint that must be logically satisfied for all following code, such as a null-check (or any other guards).

This is a nice concession, but it really doesn't invalidate any answer. I could say that in your code example:

bool CanLeaveWithoutUmbrella()
{
    if(sky.Color != Color.Blue)
    {
        return false;
    }
    return true;
}

a valid interpretation for writing code like this could be that "we only must leave with an umbrella if the sky is not blue." Here there is a constraint that the sky must be blue for the code following to be valid to run. I've met the definition of your concession.

What if I say that if the sky color is black, it's a clear night, so no umbrella necessary. (There are simpler ways to write this, but there are simpler ways to write the whole example.)

bool CanLeaveWithoutUmbrella()
{
    if(sky.Color == Color.Blue)
    {
        return true;
    }

    if(sky.Color == Color.Black)
    {
        return true;
    }

    return false;
}

As in the larger example above, I can simply add the new clause without much thought necessary. In an if/else, I can't be so sure I won't mess something up, especially if the logic is more complicated.

I will also point out that your simple example is not so simple, either. A test for a non-binary value is not the same as the inverse of that test with inverted results.

You have simplified your example too much. Either alternative may be more readable, depending on the larger situation: specifically, it depends on whether one of the two branches of the condition is abnormal. Let me show you: in a case where either branch is equally likely, ...

void DoOneOfTwoThings(bool which)
{
    if (which)
        DoThingOne();
    else
        DoThingTwo();
}

... is more readable than ...

void DoOneOfTwoThings(bool which)
{
    if (which) {
        DoThingOne();
        return;
    }
    DoThingTwo();
}

... because the first one exhibits parallel structure and the second one doesn't. But, when the bulk of the function is devoted to one task and you just need to check some preconditions first, ...

void ProcessInputOfTypeOne(String input)
{
    if (InputIsReallyTypeTwo(input)) {
        ProcessInputOfTypeTwo(input);
        return;
    }
    ProcessInputDefinitelyOfTypeOne(input);

}

... is more readable than ...

void ProcessInputOfTypeOne(String input)
{
    if (InputIsReallyTypeTwo(input))
        ProcessInputOfTypeTwo(input);
    else
        ProcessInputDefinitelyOfTypeOne(input);
}

... because deliberately not setting up parallel structure provides a clue to a future reader that getting input that's "really type two" here is abnormal. (In real life, ProcessInputDefinitelyOfTypeOne would not be a separate function, so the difference would be even more stark.)

When I see this:

if(something){
    return lamp;
}
return table;

I see a common idiom. A common pattern, which in my head translates to:

if(something){
    return lamp;
}
else return table;

Because this is a very common idiom in programming, the programmer who is used to seeing this kind of code has an implicit 'translation' in his/her head whenever he/she sees it, that 'converts' it to the proper meaning.

I don't need the explicit else, my head 'puts it there' for me because it recognized the pattern as a whole. I understand the meaning of the entire common pattern, so I don't need small details to clarify it.

For example, read the following text:

Did you read this?

I love Paris in the springtime

If so, you are wrong. Read the text again. What's actually written is

I love Paris in the the springtime

There are two "the" words in the text. So why did you miss one of the two?

It's because your brain saw a common pattern, and immediately translated it to it's known meaning. It did not need to read the entire thing detail by detail to figure out the meaning, because it already recognized the pattern upon seeing it. This is why it ignored the extra "the".

Same thing with the above idiom. Programmers who've read a lot of code are used to seeing this pattern, of if(something) {return x;} return y;. They don't need an explciit else to understand it's meaning, because their brain 'puts it there for them'. They recognize it as a pattern with a known meaning: "what's after the closing brace will run only as an implicit else of the previous if".

So in my opinion, the else is unnecessary. If I do put it in, I do so in it's own line, like so:

else return table;

Yes, there is an increase in complexity because the else clause is redundant. It's thus better to leave out to keep the code lean and mean. It's on the same chord as omitting redundant this qualifiers (Java, C++, C#) and omitting parenthesises in return statements, and so on.

A good programmer thinks "what can I add?" while a great programmer thinks "what can I remove?".

I noticed, changing my mind on this case.

When asked this question about a year ago, I would have answered something like:

»There should be only one entryand one exit to a method« And with this in mind, I would have suggested to refactor the code to:

bool CanLeaveWithoutUmbrella()
{
    bool result

    if(sky.Color == Color.Blue)
    {
        result = true;
    }
    else
    {
        result = false;
    }

    return result;
}

From a communication viewpoint it is clear, what should be done. If something is the case, manipulate the result in one way, else manipulate it in another way.

But this involves an unnecessary else. The else expresses the default-case. So in a second step, I could refactor it to

bool CanLeaveWithoutUmbrella()
{
    bool result=false

    if(sky.Color == Color.Blue)
    {
        result = true;
    }
    return result;
}

Then the question raises: Why using a temporary variable at all? The next step of refactorization leads to:

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue)
    {
        return true;
    }
    return false;
}

So this is clear in what it does. I would even go one step further:

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue) return true;
    return false;
}

Or for the faint-hearted:

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue) { return true; }
    return false;
}

You could read it like that: » CanLeaveWithoutUmbrella returns a bool. If nothing special happens, it returns false« This is the first read, from top to bottom.

And then you "parse" the condition »If the condition is met, it returns true« You could completely skip the conditional and have understood, what this function does in the default-case. Your eye and mind only have to skim through some letters on the left side, without reading the part on the right.

I'm under the impression the else more directly states intent, by stating the fact that the code in both blocks is directly related.

Yes, that is right. But that information is redundant. You have a default case and one "exception" which is covered by the if-statement.

When dealing with complexer structures, I would choose another strategy, omitting the if or it's ugly brother switch at all.

To make a long story short, in this case, getting rid of the else keyword is a good thing. It's totally redundant here, and depending on how you format your code, it will add one to three more completely useless lines to your code. Consider what's in the if block:

return true;

Therefore if the if block were to be entered into, the whole rest of the function is, by definition, not executed. But if it is not entered into, since there are no further if statements, the whole rest of the function is executed. It would be totally different if a return statement were not in the if block, in which case the presence or absence of an else block would have an impact, but here, it would not have an impact.

In your question, after inverting your if condition and omitting else, you stated the following:

Someone can now add a new if block based on a condition after the first example without immediately correctly recognizing the first condition is placing a constraint on their own condition.

They need to read code a little bit more closely then. We use high-level languages and clear organization of code to mitigate these issues, but adding a completely redundant else block adds "noise" and "clutter" to the code, and we shouldn't have to invert or re-invert our if conditions either. If they can't tell the difference, then they don't know what they're doing. If they can tell the difference, then they can always invert the original if conditions themselves.

I forgot to mention a case in which I agree with the omission of an else block, and is when using an if block to apply a constraint that must be logically satisfied for all following code, such as a null-check (or any other guards).

That's essentially what's happening here though. You're returning from the if block, so that if condition evaluating to false is a constraint that must be logically satisfied for all following code.

Is there an increase in complexity introduced by not including the else block?

No, it will constitute a decrease in complexity in your code, and it may even constitute a decrease in the compiled code, depending on whether certain super trivial optimizations will or will not take place.

In the specific example you gave, it does.

• It forces a reviewer to read the code again to make sure it's not an error.
• It adds (unnecessary) cyclomatic complexity because it adds one node to the flow graph.

I think it couldn't get any simpler that this:

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue);
}