views:

182

answers:

6

This is a sort of trivial question but something I've been wondering about.

In terms of style (I assume the performance is identical), is it better to keep an 'else' in an if statement where it's not necessary?

For example, which of the following is better:

if (x < 10)
  doSomething();
 else if (x > 20)
  doSomethingElse();

or

if (x < 10)
 doSomething();
if (x > 20)
 doSomethingElse();

Another case:

if (x < 10)
 return;
else doSomething();

or

if (x < 10)
 return;
doSomething();

Thanks,

Luke

+5  A: 

In the first example, definitely keep the else in - it prevents an extra evaluation if the first part is true.

(i.e. if you do:

    if (x < 10) 
     doSomething(); 
   if (x > 20) 
    doSomethingElse();

both ifs are always evaluated.

However, if you do:

if (x < 10)
  doSomething();
else if (x > 20)
  doSomethingElse();

the second part is only evaluated if the first is false. [If x < 10, you don't even want to check if x > 20, it's a waste of an evaluation...])

The second is a personal design decision; choose which appeals more to you, fits the particular logic more, or follows your company's standards if they have any.

froadie
Agreed, though I'd suggest dropping the `else` in the second case since it has no effect. There's also the fact that if you include it then there is an implication that it is needed. It certainly wouldn't confuse everyone, but it might confuse someone.
William
Daniel Earwicker
@Daniel Earwicker - I only said it's a personal design decision for the second example (with the return), where there's no extra evaluation and the flow is the same for both methods.
froadie
+5  A: 

Leave the elses where it enhances code clarity. In the first example, leave the else, since it is subtly different, saving an evaluation. The second case is less clear-cut; typically I use elses after returns for alternate cases, but leave out the else when the return is for error handling.

sreservoir
A: 

As a general rule, always leave them in, and add a comment as to why it is ok, that the else has no code in it. That way those who come in your footsteps won't have to ask the question "What is the implication of the else? Should there be one there or not?"

If you find yourself unable to come up with the correct wording to explain why the else is not significant, then there is a good chance that it is.

Missing elses are generally a code smell.

EvilTeach
...so put a comment as to why it's not needed, not an empty else!
froadie
+1  A: 

Generally I would stay away from anything that isn't necessary. More code means more room for bugs.

However, if code that isn't necessary for the flow of the program, makes it that much clearer to someone reading it, then it may be necessary after all.

In your first example, no number would be less than 10 and greater than 20, but sometimes the logic is not as obvious. The else makes it easy to see that these conditions are part of the same block of code, and therefore should be included.

That else in your second example doesn't actually change the flow of the program so it really isn't necessary. In fact, you might consider reworking the logic a little:

if(x>10){
    doSomething;
}

Now you don't even have to worry about the return statement, or the extra else block.

Edit: added brackets.. For Turing's sake!

Skunkwaffle
+1  A: 

If you're breaking an if statement across more than one line, then for Turing's sake use brackets! So:

if (x < 10) return;

Or

if (x < 10) {
    return;
}

But not

if (x < 10)
    return;

The else issue is subjective, but my view on it is that an if/else is conceptually breaking your code into more-or-less equal cases. Your first example is handling two conceptually similar but mutually exclusive cases, so I find an else to be appropriate. With two separate if statements, it appears at first glance as though the two conditionals are independent, which they are not.

When you return on an if, the circumstances are different. The cases are automatically mutually exclusive, since the return will prevent the other code from running anyway. If the alternative case (without the return) is short, and escecially if it returns as well, then I tend to use the else. If, as is common, the alternative code is long or complex, then I don't use an else, treating the if as more of a guard clause.

It's mostly a matter of clarity and readability, and both of those factors are subjective.

Thom Smith
Agreed. Brackets should always be included.
Skunkwaffle
+1  A: 
supercat