The Redundant ‘Else’
Sometimes, programmers literally translate exactly what they are thinking directly into code. This is especially true for IF/ELSE cases. So, if you see this:
if (someCondition) return DoSomething1(); else return DoSomething2();
…the ‘else’ statement is redundant because if the ‘if’ condition was true the method would return right there. Instead, write that same code like this:
if (someCondition) return DoSomething1(); return DoSomething2();
One case where the “else” does matter is if you buy into the “one way in, one way out” approach to writing methods. If you do, then you would have only one “return” statement in your example above that returns the results of a variable that was set in the “true” side of the “if” statement and (possibly) again in the “false” side of the “if” statement. In some cases, you can simply set the default value of the variable when it is declared and only modify it in the “true” side of the else.
It’s really a matter of style but it does have a bearing in regards to the “Redundant ‘Else’”
@Jeff B
I’m familiar with this model, but I don’t completely see its value. I guess it would tend to make the method slightly more readable, if a little more bulky. Thoughts?
Meh. I thought the point of source code was to capture what the programmer was thinking? In this particular case, I strongly prefer the first approach over the second because it matches the way I think about it (and, I believe, the way most people think about it). The second approach is one line shorter but harder to skim and extract intent from quickly.
The else keyword isn’t hurting anybody and is arguably more explicit about intent. Why get rid of it?
I’m a fan of
return (condition) ? DoSomething1() : DoSomething2();
I think “one way in, one way out” is beneficial when a method is long or has a high cyclomatic complexity. For short methods, I call YAGNI.
@Jef: Amen brother!
@Eric: Why get rid of it? Because it’s redundant. It is not needed. Code analyzers (like ReSharper and CodeRush) will point out the fact that is, in fact, dead code. The “point of source code” is NOT to capture what the programmer is thinking… it’s to capture exactly what that short piece of code is supposed to be accomplishing. It should be readable without being redundant. Like @Jef said… YAGNI… You Aint Gonna Need It!
@Jeff B: “IF” you (not exactly “you”) “buy into” the “one way in, one way out”, Ok. I don’t. I believe we don’t write code like we did a long time ago. Keep in mind that the “multiple return point” rule was founded when 1000-line methods was acceptable… which we all know is not acceptable. So, trade in the “multiple-return-point” rule for the “single responsibility” rule and make your methods short and concise… by the way, I think we were on the same page anyway weren’t we?!
Well, hmm. I still disagree on this one.
Fundamentally, this construct *is* an if-then-else: if the condition is true, then value A will be returned, otherwise, value B will be returned. That’s how you’d describe it in English to a non-programmer, right? Or heck, even a programmer. Can you imagine trying to describe this code over the phone to a fellow programmer? “Ok, at the end of the method, if the condition is true, we return value A. We return value B.” Huh? That doesn’t even make sense until you convert it into C# code in your mind and parse it.
For any action in your branch other than a return statement (like say, calling another method or setting a variable), the else would be necessary. The only reason the else isn’t strictly necessary here is that the return statement is special in that it immediately exits the method and skips all following code. So why are you writing your branch in a way that relies on a very special side-effect of the contents of one of the branches rather than just writing it the way you’d describe it over the phone to someone? Just because the language makes some terse coding trick possible doesn’t mean it’s desirable.
I don’t think YAGNI applies here. YAGNI is supposed to be about avoiding speculative work that you’ll probably never benefit from. In this case we’re talking about writing code in a way that reads the most naturally and common-sensically to people who have to deal with it later. There’s a clear and immediate benefit.
It’s the same reason you should name your variables something like “currentAccountBalance” rather than “cb”. Do you call YAGNI when someone uses 21 keystrokes for a variable name instead of two? I hope not.
Granted, it’s not the end of the world either way but hey, we’re discussing examples of good/bad programming style on a web site dedicated to that topic.