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?

zohub
  • 161

6 Answers6

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

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

John Wu
  • 26,955
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.

JacquesB
  • 61,955
  • 21
  • 135
  • 189
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.

doubleYou
  • 2,867
  • 1
  • 13
  • 26
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.

gnasher729
  • 49,096
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).

Alexander
  • 5,185