Although if/else blocks definitely have their uses, I think it’s mostly seen in procedural code. In today’s code, I find it mostly on the edges of the OO-realm, e.g. when checking for null pointers. In pure object-oriented code polymorphism will be used in places where traditionally an if/else block would be used.

I find this even more true when looking at that extended if/else block, the switch-case statement. Whenever I see it in code, it smells, and it usually can be replaced with something much more elegant using some form of polymorphism.

We should be carefull not to try to eliminate Switch statements just because we think they are evil. Because actually, they are not. I don’t use many Switch statements and I too sense the smell when I use them or when I come across them in other peoples code.

BUT they do serve a purpose. You should use a switch statement, when there is little or no reason to suspect, that the code will have to change. If you are always using polymorphism instead of a Switch, there’s a good chance that your doing a kind of “premature optimization” and that IS, without question, an evil.

Of course if it is easier and faster (sometimes it is and sometime it most definately isn’t) to do the polymorphism it’s preferrable. But don’t use polymorphism if it’s not validated by volatile code or the fact that it is easier/faster to do.

I too find that else statments are sort of an evil. I use guard clauses to the extreme, because it keeps my methods clean, from a single purpose perspective. I’d much rather have two methods calls, where one does nothing due to a guards clause, than if/else statements in one method. I find it much easier to express my intentions with the two method approach.

Hmmm… This made me remember this piece by Kevin Rutherford:

http://silkandspinach.net/2004/07/16/if/

Summary: What about keeping the conditional code where it is really needed (i.e. on the boundaries to the rest of the world)?

You know in functional programming the use of pattern matching is very common due to it’s close relationship to recursiveness. First define the ADT that represents for example a binary tree and then traverse the tree by using recursive function that forks using pattern matching (generalized switch / if else). And with more complex ADT, it just becomes more obvious how elegant, easy-to-read and how easy it is to prove correct it really is. How could it be bad?

I think there is something sublime there. I know that code I write well tends to have few conditionals, but I’ve never thought about the elses.

I just checked a little tool I wrote a while back called ‘Vise.’ It has 12 classes and no elses. Another tool I’m working on called ‘Glaze’ has 17 classes and only one else.

Side note:

Years ago, Kent Beck wrote ‘99 bottles of beer on the wall’ in Smalltalk. He kind of pushed it to the limit and used conditionals only in a factory. It’s neat code: http://www.smalltalkconsulting.com/html/99bottlesofbeer.html

Alright, that’s it. We’ve officially run out of things to legitimately call “evil.” Good work, everyone.

I wouldn’t say they are the root of all evil, but they do tend to waste alot of screen real estate with all the extra indentation required for the nested if/else blocks, and the maze of {} blocks gets confusing when they’re too big to fit on your screen at once.

The use of ‘if’ without ‘else’ implies bad code. This (by implication) is called an uncontrolled side-effect. It is what many Blub Programmers (Java, C, C++ etc.) are trying to avoid anyway; they just don’t know it, because well, they aren’t very clever.

There is a reason that, for example, the Haskell programming language fails to compile if without else, since Haskell disallows uncontrolled side-effects. In fact, every pure functional language would fail to compile if without else. By posting what you have, you have implicitly refuted the legitimacy of some of the most powerful programming languages available. In light of this, don’t you think you need to reconsider?

Look up the term “referential transparency” and have a deeper think. Though, don’t do it while stoned this time :)

Elses aren’t evil. Ifs are, though. They directly contribute to higher cyclomatic complexity by adding extra code paths.

When I refactor, I do it with an emphasis on removing “if” statements. The result is almost always much less code and many fewer “you forgot to check …” bugs.

Many ifs are plain unnecessary (e.g., “if (p) delete p”). Very often I can replace “if (foo) DoSomething()” in ten places with “void DoSomething() { if (foo) { ... } }” in one place. The null object pattern regularly comes in handy, and replacing switch statements with polymorphism can have a big impact as well.

The lack of elses is actually a code smell in my view. Okay, so “if (foo) DoSomething”. But what if not foo? Should something else happen instead? Or are you just being defensive and foo should actually never happen?—in which case, I’d argue an assertion or exception is more appropriate.

What about languages that don’t have a ‘return’ statement?

hmm, I’d say nested ternaries are the root of all evil…

seriously though, I think there is something to avoiding the else clause. Every time you hit one in your code you have to write another unit test that takes that branch… the whole arrange, act, and assert cycle gets repeated.

As mentioned in some of the comments above, the evil starts with if as it has an implicit else statement: do nothing.

Kevin Rutherford wrote something on the topic a few years ago.

Hmmm… links don’t work much, do they? Here’s another attempt in plain text:

http://silkandspinach.net/2004/07/16/if/

Indeed I did, starting with a post called if…

My comment was in the context of C#/java type languages, where it’s IS a good practice to use guard clauses:

Simple example: void DomainObject someMethod(string someParam) { if(null someParam) return null; if(string.Empty someParam) return null; ... }

Clearly ther’s no use for the else clause here, so my point should be valid enough; no uncontrolled sideeffects, and no confusion about what happens.

I totally agree with Alyosha about moving the conditionals inside the methods affected by them. This is in clearly in line with the “Tell, don’t ask” principle.

Thanks for all the good comments.

Next, I plan to prove that whitespace is evil… Some of you read too much into my provocative title ;)

Clearly, any construct can be abused. Of course, I don’t believe that else clauses are evil, because conditional logic exists all programs. The question is how we handle it. Big blocks of conditional logic are almost always a smell. The original quote observed that well-structured code, which makes effective use of encapsulation, polymorphism, etc., tends to use few else clauses and the ones that appear are usually small and narrowly-focused. This is the preferred way of handling conditional logic in OO languages. As some of you observed, functional languages offer other approaches to conditional logic.

Also, following up on Jan Daniel Andersen’s observation, doing the simplest thing that can possibly work means using a straightforward else clause when the “complication” of a polymorphic alternative solution isn’t justified.

I’m not sure I can pretend that either “if” or “else” are evil.

Ifs are necessary, because conditional code is necessary. If you want to remove them, you’ll end up with over-engineered solutions which will be quite hard to maintain (think strategies all over the place). Sure, you’ll get a low average cyclomatic complexity – but I’m not sure I want to pay the price for this. And will the next point be: how to get rid of these loops?

Else clauses are not necessary. Most of the decisions we have to make don’t imply a else clause – because it’s simply not needed (this is my answer to the “else clause missing code smell”: a necessary if clause doesn’t imply a necessary else clause. Of course, this is just my opinion, and I may be wrong).

I also don’t agree with the simplistic approach of the “use a straightforward else clause when the complication of a polymorphic solution isn’t justified”. You use a else clause when it’s justified, and most of the time, a polymorphic solution is not the best way to handle the same problem (I tend to think that inheritance shall NOT be used to tackle with implementation issues). Conclusion: your else clause might not be straightforward. Anyway, what does “straightforward” mean in this context? 2 lines of code? 3? 10? What if I replace ten lines of code by a function call?

And to answer Deans first question (So, if a method has an if/else block, is it doing two (or more) things and therefore violating the SRP?): no, it does one thing in two different ways, based on a conditional. If the two clauses are completely unrelated, then yes, it does two things. But most of the time, this is not the case (except, of course, in the code base I’m working on; it seems they thought that 1500+ LOC constructors were teh kewl).

@ Jan Daniel Andersen

Yes, there is an uncontrolled side-effect. I challenge you to spot it, without my having to give it away. It is ALWAYS implied that if without else involves an uncontrolled side-effect.

And these uncontrolled side-effects are the single biggest nasty thing that plagues the world of programming. This leaves the original article a little bit out for criticism don’t you think? There is a reason it was immediately downvoted on programming.reddit.com. Think about it some more, please, for the sake of programming :)

@ Tony Morris

I don’t see how the if’s in my example can leave any uncontrolled sideeffects. Not inside the methods. It does imply that the caller, handles a possible null. I’m guessing that is what you’re hinting at. The code in my example, could have used a NullObject instead of returning null. I would probably have used this, if the caller, was an another class, but since this method is private, I would argue, that it’s a fair to assume that I (being the one who made the class) will handle the null-situation.

... or did I miss your point?

I misread your code. You have declared a void/unit return type, when you are in fact returning (null), which led to my confusion.

So then, your code is better (correctly) written as if(null someParam) return null; else if(string.Empty someParam) return null; else ...

Your code is imperative – it specifies an order of execution. This is nasty, but unavoidable in many crappy languages. My code does not i.e. the compiler is free to decide evaluation order. I even have a project dedicated to demonstrating this fact.

http://code.google.com/p/pure-functional-java/ Please feel free to ask questions.

Always a pleasure to see Tony promoting FP :).

@Tony. I see your point. But I don’t agree. I don’t see a problem, with my “exit or continue” approach. To me it’s very logical. There are always only 2 choices: either the function works as expected or else it tells me that there, something wrong (returning null or throwing an exception.).

I don’t know much about functional programming, so I might still miss your point :-).

I too prefer the “exit or continue” approach, and I find the guard clause very readable. And every time I see a guard clause, I then ask myself if it can be removed. Where did that null parameter come from?

If it came from some other code in my application, perhaps I can re-design so that the pieces communicate more clearly?

I don’t see how ???? the if’s in my example can leave any uncontrolled sideeffects. Not inside the methods. It does imply that the caller, handles a possible null.

Also, following up on Jan Daniel Andersen’s observation, doing the simplest thing ?? ???? that can possibly work means using a straightforward else clause when the “complication” of a polymorphic alternative solution isn’t justified.