15

When to use else in conditions?

1)

a)

long int multiplyNumbers(int n)
{
    if (n >= 1) {
        return n*multiplyNumbers(n-1);
    } else {
        return 1;
    }
}

or

b)

long int multiplyNumbers(int n)
{
    if (n >= 1) {
        return n*multiplyNumbers(n-1);
    } 

    return 1;
}

2)

a)

int max(int num1, int num2) {
   int result;

   if (num1 > num2) {
      result = num1;
   } else {
      result = num2;
   }

   return result; 
}

or

b)

int max(int num1, int num2) {

   if (num1 > num2) {
      return num1;
   } else {
      return num2;
   }
}

or

c)

int max(int num1, int num2) {
   if (num1 > num2) {
      return num1;
   } 

   return num2;
}

Is there a rule about when to use else?

Do the if with else statements take more memory? On the one hand, they are more readable, but on the other hand, too much nesting is poor reading.

If I throw exceptions, then it is better not to use else, but if these are ordinary conditional operations like in my examples?

8
  • Both links do not answer directly this question – Billal Begueradj Jan 19 '18 at 19:11
  • 4
    1. The if statement contains a return, so else is redundant. – Robert Harvey Jan 19 '18 at 19:15
  • 3
    2. C is fine, except that you never use int result. – Robert Harvey Jan 19 '18 at 19:16
  • 3
    About redundant elses after returns: It depends. In asynchronous or callbacky code, like node.js, it is fairly common to use both, as a "belt and suspenders" way to avoid bugs. As for this question, I'd probably use a ternary operator for all the case. – user949300 Jan 19 '18 at 19:30
  • 4
    I agree with @user949300, I'd use a ternary because you're peforming the same action for each case (returning a value). return (num1 > num2) ? num1 : num2; simplifies case 2 and shows that you're returning a value regardless of what the condition evaluates to. if else blocks are the better choice when you should perform different logic for each branch. – Wes Toleman Jan 20 '18 at 17:06
42

I can tell you which one I would use, and for which reasons, however I think there will be as many opinions as community members here...

1) Neither. I would write:

long int multiplyNumbers(int n)
{
    if (n < 1) {
        return 1;
    }
    return n*multiplyNumbers(n-1);
}

This is because I like the idea of "early exit" in the special cases, and this reminds me of that scenario. The general rule is: if you need to handle a special case or a general case, handle the special case first. Presumably the special case (e.g. error or a boundary condition) will be handled quickly and the rest of the code is still not too far from the 'if' condition so that you cannot skip a few lines of code to look into it. It also makes reading the general case handling easier, because you need to just scroll down the function body.

In other words, instead of:

void foo(int bar)
{
    if (!special_case_1) {
        if (!special_case_2) {
            return handle_general_case();
        }
        else {
            return handle_special_case_2();
        }
    }
    else {
        return handle_special_case_1();
    }
}

I am advocating writing this:

int foo(int bar)
{
    if (special_case_1) {
        return handle_special_case_1();
    }
    if (special_case_2) {
        return handle_special_case_2();
    }
    return handle_general_case();
}

You will notice that, if it is feasible, I prefer not to introduce an additional variable 'result' here, mostly because then I don't need to care whether it is initialised or not. (If you introduce it, then either you initialise it at the top, only to be overwritten in each branch, or you don't, and then you may forget to initialise it in some branch. Plus, even if you initialise it in every branch, your static code analyser may not see this and may complain for no reason.)

2) I would use (b) (with declaration of 'result' thrown away). This is because in this case I cannot say that either num1 < num2 or num1 >= num2 is a special case. They are equal to my eyes and thus (b) is preferred over (c). (I've already explained why, in this case, I wouldn't introduce an additional variable, so (a) is in my eyes inferior to (b).)

4
  • Note: I used to treat the special case first, however nowadays I've changed tack, and instead treat the smaller case first (that is, the one requiring the least amount of code) whenever practical. This in turns minimizes the distance between if and return, making it more obvious to the reader that it's an early return and the rest of the function no longer has to care for this condition. – Matthieu M. Jan 20 '18 at 13:37
  • 4
    @MatthieuM. If special case handling is taking a distractingly large amount of code, you might want to consider splitting the body of that if block out into a separate helper function. This way you keep the body of the function concentrated on the main task, rather than being overly complicated by something you consider a "special case". – R.M. Jan 20 '18 at 16:08
  • @R.M.: Well... I tend to write performance-oriented code, so rather than putting special-cases out of bands, I put infrequent cases (behind non-inlinable cold functions), so as to preserve the i-cache. Still, a sound advice indeed. Detecting the error condition and just handing it off to a special-purpose function makes for short guard conditions. – Matthieu M. Jan 20 '18 at 17:48
  • I also tend to the "I like the idea of early exit" is great if the most often found condition is first, avoids secondary conditionals in some languages altogether it can be faster, never executing further code that falls after that... – Mark Schultheiss Jan 20 '18 at 18:53
15

There's no hard and fast rule, but here some basic guidelines that I ask my team to follow:

  • Avoid deeply nested ifs
  • Arrange code blocks so they are near to the condition that affects them
  • For most validation checks, null checks, or typical programming logic, you can avoid else statements completely by using the guard pattern.
  • For business logic-- where you are not just checking for edge conditions but are choosing which path to follow-- it's more important to represent the logic in code in a manner than is easily mapped to the specification. So if the specification contains else statements (or arrows, in a flow diagram), it's OK to have these in code.

Simple example. Instead of

if (condition1)
{
    if (condition2)
    {
        DoSomething();
        return result;
    }
    else
    {
        return errorCode;
    }
}
else
{
    return errorCode;
}

It is easier to read:

if (!condition1) return errorCode;
if (!condition2) return errorCode;
DoSomething();
return result;
  1. We reduced nesting level. People reading the code don't have to combine conditions in their head to figure out the logic.

  2. The action associated with each conditional expression is right next to the if statement that determines whether it will run. People reading the code won't have to track the curly brackets to figure out what code is affected by what condition, and won't have to scroll up and down to understand the code.

  3. This approach follows the guard pattern which reduces cyclomatic complexity, which has been shown to reduce defect rates.

See also:

How to avoid if chains

Code pattern to have the least complixity

Flattening arrow code

10

There is no universal rule, and you will find reasonable people disagreeing about whether having multiple returns is ever a good idea.

In cases where you can make the code just as simple with a single return, this is preferable. E.g.:

int max(int num1, int num2) {
   return (num1 > num2) ? num1 : num2;
}

In cases where you can make the code simpler by using multiple returns, I think it is acceptable if it follows the idiom of "early exit":

long int multiplyNumbers(int n)
{
    if (n < 1) {
        return 1;
    } 
    return n*multiplyNumbers(n-1);
}

That is, you have some special cases which can be handled at the top of the function, skipping the more complex logic. But apart from this, I would recommend against having multiple returns or returns inside nested blocks, since it makes the logic harder to follow in more complex functions.

4

Regarding memory use:

  • This is not something you should even worry about.
  • No, in general, there should not really be a difference.
  • The compiler will change things as it sees fit anyway - readability is really the only issue here.

I don't think there is an absolute definitive answer. However, there are some aspects that you might want to consider.

Example 1

I would prefer returning early, if the condition represents some kind of special case.

unsigned factorial(unsigned n)
{
    if (n <= 1) {
        return 1;  // this is a special case
    }

    return n * factorial(n-1);  // this is the "normal" case
}

If you have two cases and you cannot decide which case is special, then an if/else approach might make the most sense.

Example 2

There are going to be exceptions, but I would avoid declaring a variable that is not used until later. So I would choose 2a) over 2b). 2c) makes sense if num1 > num2 is something special, as described above.

1

There's no fixed rule. With experience, you will figure out various rules that you will apply in various cases. In your example, you have two branches that are symmetrical and simple, so you just use if (...) { return x; } else { return y; }

Things to consider: Sometimes you have a largish function with some easy special cases to get rid of first. if (simple condition) return x; if (another simple condition) return y; followed by lots of code.

Another thing to consider: For debugging, you may want to set a breakpoint when the function returns, so you want only one return statement. (If you have a debugger that lets you break when the function returns, even with multiple return statements, you are lucky).

And you don't want deeply nested if / else sequences, with the else of the first if about half a mile away, so you arrange your code accordingly.

1

As others have already mentioned: it's usually preferable to keep the happy path as minimally indented as possible, and to return early when necessary.

On a side note, in both of these simple cases, I would recommend using a conditional expression, as this is pretty much the ideal use case for it:

1) return (n < 1) ? 1 : n*multiplyNumbers(n-1);

2) return (num2 < num1) ? num1 : num2;

I would most certainly avoid method 2.a. It's long, and the temporary variable is extra clutter with no readability benefit (e.g. it doesn't have an informative name).

Not the answer you're looking for? Browse other questions tagged or ask your own question.