91

I just came from Simple Design and Testing Conference. In one of the session we were talking about evil keywords in programming languages. Corey Haines, who proposed the subject, was convinced that if statement is absolute evil. His alternative was to create functions with predicates. Can you please explain to me why if is evil.

I understand that you can write very ugly code abusing if. But I don't believe that it's that bad.

skaffman
  • 398,947
  • 96
  • 818
  • 769
Vadim
  • 21,044
  • 18
  • 65
  • 101
  • 138
    `if` statements are evil in the way hammers are evil. Some people might misuse them, but they're an essential tool. – Dominic Rodger Oct 12 '09 at 12:08
  • 26
    So some guy throws out an assertion without back up or explaining why? – random Oct 12 '09 at 12:08
  • 11
    I'm very suspicious of sweeping statements like the above. I don't mind preferences, and suggestions as to why you may not want to use a particular tool/feature etc. But a dogmatic stamp of 'evil' turns me off (I don't know if he qualified this at all, btw). – Brian Agnew Oct 12 '09 at 12:12
  • 7
    @e.c.ho - there is never anything wrong with an assertion without justification. ;) – Dominic Rodger Oct 12 '09 at 12:13
  • 1
    Aren't we missing out context in which the statement was made? IMO, it is a language thing. C, for example, does not have the concept of delegates (well, of course, you can manufacture something, but that's besides the point). – dirkgently Oct 12 '09 at 12:21
  • 1
    I'd be interested in finding out if there are any interviews or transcripts of Corey's presentation. I'd like to get the full story of this. – Jason Baker Oct 12 '09 at 12:30
  • @Vadim Are you his publicist? – random Oct 12 '09 at 12:36
  • 2
    @dirkgently: it doesn't have delegates for keeping things orderly, but it does have function pointers. But I don't know *anyone* who would advocate a function pointer over an if statement. – Matthew Scharley Oct 12 '09 at 12:37
  • re "sweeping statements": but how do you qualify your statements without "if"? ;) – Piskvor left the building Oct 12 '09 at 12:54
  • This shows how much attention you can draw by starting with a black&white statement; the truth is generally in many shades of gray. – Adriaan Oct 12 '09 at 13:40
  • 10
    "If" statements don't kill programs: Bad programmers kill programs! – Jay Oct 12 '09 at 14:11
  • @e.c.ho I wonder where your questions come from. Corey's statement kind of shock me. I was wondering what I don't understand. Stackoverflow is great for me to get answers. – Vadim Oct 12 '09 at 14:46
  • @Vadim The publicist thing was facetious. Having never heard of the bloke before, his blanket statement without reason is pretty much like a flame to moths. – random Oct 12 '09 at 15:05
  • 1
    @Dominic, nowadays `if` statements are **much** useful than hammers – flybywire Oct 12 '09 at 15:29
  • 2
    @Vadim What response did you get from Corey Haines when you asked him to explain why? – Matt Lacey Oct 12 '09 at 17:06
  • 1
    -1 for asking a question who's answer is clear: of course they're evil! ;) +1 actually – RCIX Nov 06 '09 at 10:23
  • All sweeping statements are evil. – Vince O'Sullivan Sep 13 '16 at 05:58

18 Answers18

113

The if statement is rarely considered as "evil" as goto or mutable global variables -- and even the latter are actually not universally and absolutely evil. I would suggest taking the claim as a bit hyperbolic.

It also largely depends on your programming language and environment. In languages which support pattern matching, you will have great tools for replacing if at your disposal. But if you're programming a low-level microcontroller in C, replacing ifs with function pointers will be a step in the wrong direction. So, I will mostly consider replacing ifs in OOP programming, because in functional languages, if is not idiomatic anyway, while in purely procedural languages you don't have many other options to begin with.

Nevertheless, conditional clauses sometimes result in code which is harder to manage. This does not only include the if statement, but even more commonly the switch statement, which usually includes more branches than a corresponding if would.

There are cases where it's perfectly reasonable to use an if

When you are writing utility methods, extensions or specific library functions, it's likely that you won't be able to avoid ifs (and you shouldn't). There isn't a better way to code this little function, nor make it more self-documented than it is:

// this is a good "if" use-case
int Min(int a, int b)
{
    if (a < b) 
       return a;
    else
       return b;
}

// or, if you prefer the ternary operator
int Min(int a, int b)
{
    return (a < b) ? a : b;
}

Branching over a "type code" is a code smell

On the other hand, if you encounter code which tests for some sort of a type code, or tests if a variable is of a certain type, then this is most likely a good candidate for refactoring, namely replacing the conditional with polymorphism.

The reason for this is that by allowing your callers to branch on a certain type code, you are creating a possibility to end up with numerous checks scattered all over your code, making extensions and maintenance much more complex. Polymorphism on the other hand allows you to bring this branching decision as closer to the root of your program as possible.

Consider:

// this is called branching on a "type code",
// and screams for refactoring
void RunVehicle(Vehicle vehicle)
{
    // how the hell do I even test this?
    if (vehicle.Type == CAR)
        Drive(vehicle);
    else if (vehicle.Type == PLANE)
        Fly(vehicle);
    else
        Sail(vehicle);
}

By placing common but type-specific (i.e. class-specific) functionality into separate classes and exposing it through a virtual method (or an interface), you allow the internal parts of your program to delegate this decision to someone higher in the call hierarchy (potentially at a single place in code), allowing much easier testing (mocking), extensibility and maintenance:

// adding a new vehicle is gonna be a piece of cake
interface IVehicle
{
    void Run();
}

// your method now doesn't care about which vehicle 
// it got as a parameter
void RunVehicle(IVehicle vehicle)
{
    vehicle.Run();
}

And you can now easily test if your RunVehicle method works as it should:

// you can now create test (mock) implementations
// since you're passing it as an interface
var mock = new Mock<IVehicle>();

// run the client method
something.RunVehicle(mock.Object);

// check if Run() was invoked
mock.Verify(m => m.Run(), Times.Once());

Patterns which only differ in their if conditions can be reused

Regarding the argument about replacing if with a "predicate" in your question, Haines probably wanted to mention that sometimes similar patterns exist over your code, which differ only in their conditional expressions. Conditional expressions do emerge in conjunction with ifs, but the whole idea is to extract a repeating pattern into a separate method, leaving the expression as a parameter. This is what LINQ already does, usually resulting in cleaner code compared to an alternative foreach:

Consider these two very similar methods:

// average male age
public double AverageMaleAge(List<Person> people)
{
    double sum = 0.0;
    int count = 0;
    foreach (var person in people)
    {
       if (person.Gender == Gender.Male)
       {
           sum += person.Age;
           count++;
       }
    }
    return sum / count; // not checking for zero div. for simplicity
}

// average female age
public double AverageFemaleAge(List<Person> people)
{
    double sum = 0.0;
    int count = 0;
    foreach (var person in people)
    {
       if (person.Gender == Gender.Female) // <-- only the expression
       {                                   //     is different
           sum += person.Age;
           count++;
       }
    }
    return sum / count;
}

This indicates that you can extract the condition into a predicate, leaving you with a single method for these two cases (and many other future cases):

// average age for all people matched by the predicate
public double AverageAge(List<Person> people, Predicate<Person> match)
{
    double sum = 0.0;
    int count = 0;
    foreach (var person in people)
    {
       if (match(person))       // <-- the decision to match
       {                        //     is now delegated to callers
           sum += person.Age;
           count++;
       }
    }
    return sum / count;
}

var males = AverageAge(people, p => p.Gender == Gender.Male);
var females = AverageAge(people, p => p.Gender == Gender.Female);

And since LINQ already has a bunch of handy extension methods like this, you actually don't even need to write your own methods:

// replace everything we've written above with these two lines
var males = list.Where(p => p.Gender == Gender.Male).Average(p => p.Age);
var females = list.Where(p => p.Gender == Gender.Female).Average(p => p.Age);

In this last LINQ version the if statement has "disappeared" completely, although:

  1. to be honest the problem wasn't in the if by itself, but in the entire code pattern (simply because it was duplicated), and
  2. the if still actually exists, but it's written inside the LINQ Where extension method, which has been tested and closed for modification. Having less of your own code is always a good thing: less things to test, less things to go wrong, and the code is simpler to follow, analyze and maintain.

Huge runs of nested if/else statements

When you see a function spanning 1000 lines and having dozens of nested if blocks, there is an enormous chance it can be rewritten to

  1. use a better data structure and organize the input data in a more appropriate manner (e.g. a hashtable, which will map one input value to another in a single call),
  2. use a formula, a loop, or sometimes just an existing function which performs the same logic in 10 lines or less (e.g. this notorious example comes to my mind, but the general idea applies to other cases),
  3. use guard clauses to prevent nesting (guard clauses give more confidence into the state of variables throughout the function, because they get rid of exceptional cases as soon as possible),
  4. at least replace with a switch statement where appropriate.

Refactor when you feel it's a code smell, but don't over-engineer

Having said all this, you should not spend sleepless nights over having a couple of conditionals now and there. While these answers can provide some general rules of thumb, the best way to be able to detect constructs which need refactoring is through experience. Over time, some patterns emerge that result in modifying the same clauses over and over again.

vgru
  • 49,838
  • 16
  • 120
  • 201
  • 2
    Nice example for pattern matching, with only a small problem in the result: `sum / people.Count` is incorrect, because it divides partial sum with count of all people. Consider only a single female a in team of 15 programmers. Real age of the female is 45, but when dividing by 15 the average age of females is 3. And that is not a correct average age... – Julo Oct 09 '18 at 06:30
92

There is another sense in which if can be evil: when it comes instead of polymorphism.

E.g.

 if (animal.isFrog()) croak(animal)
 else if (animal.isDog()) bark(animal)
 else if (animal.isLion()) roar(animal)

instead of

 animal.emitSound()

But basically if is a perfectly acceptable tool for what it does. It can be abused and misused of course, but it is nowhere near the status of goto.

flybywire
  • 261,858
  • 191
  • 397
  • 503
  • 7
    I agree with this one. It's the same problem I have with switch statement. In this case we can a strategy pattern. – Vadim Oct 12 '09 at 12:20
  • 1
    I've had a professor who said exactly that - polymorphism should be used to hide type `if`s. He even warned not to use `is(obj is Foo), because this reflection implied a design flaw. – Kobi Oct 12 '09 at 12:36
  • 1
    +1 for replacing if with a more generic inheritance solution, but I think Corey has been talking about FP more than anything else. – Lee B Oct 12 '09 at 12:46
  • Eric mentions that in response to a comment from Aaron G in his as/cast article at http://blogs.msdn.com/ericlippert/archive/2009/10/08/what-s-the-difference-between-as-and-cast-operators.aspx – Brian Oct 12 '09 at 12:59
  • 3
    In dynamic languages, `if(obj is Foo)` is quite sensible, since you don't have typed parameters. Most of the time, you really want to duck-type instead, but sometimes with built-in types, nothing but the real thing will suffice, and in such circumstances, you want to raise an error for anything but `Foo`, and for that you need `if(obj is Foo)`. – Bob Aman Oct 12 '09 at 13:14
  • Don't forget about ToolKit libraries. They have different design needs then business apps. It is common to make toolkits that check an objects type before deciding what to do with the object. – Matthew Whited Oct 12 '09 at 13:22
  • 2
    Originally, when books/people said that "if" was bad, it was exactly in this context of polymorphism; which I totally agree with. At some point later, some people repeated that without knowing the original context nor why, which is a shame. – b.roth Oct 12 '09 at 13:52
  • Absolutely. If you are using the type of an object as a flag that you test, this is just pointless. If for whatever reason you can't execute the appropriate code by overloading the function, that is, if in context what you really need is a flag, then just create a flag and be done with it! About the only time I use "instanceof" is when a parameter can have multiple types that can't be determined at compile time. – Jay Oct 12 '09 at 14:24
39

A good quote from Code Complete:

Code as if whoever maintains your program is a violent psychopath who knows where you live.
— Anonymous

IOW, keep it simple. If the readability of your application will be enhanced by using a predicate in a particular area, use it. Otherwise, use the 'if' and move on.

Nathan
  • 11,814
  • 11
  • 50
  • 93
Jordan Parmer
  • 36,042
  • 30
  • 97
  • 119
18

I think it depends on what you're doing to be honest.

If you have a simple if..else statement, why use a predicate?

If you can, use a switch for larger if replacements, and then if the option to use a predicate for large operations (where it makes sense, otherwise your code will be a nightmare to maintain), use it.

This guy seems to have been a bit pedantic for my liking. Replacing all if's with Predicates is just crazy talk.

Kyle Rosendo
  • 25,001
  • 7
  • 80
  • 118
  • 9
    I'm not fan of switch statements. I prefer to use strategy pattern instead when I can. – Vadim Oct 12 '09 at 12:12
  • 9
    I'm with Kyle, it's just crazy talk. You can't program without conditionals in the way that you can program without GOTOs. Switches, predicates, dispatch tables, etc, are just syntactic sugar for IF. – High Performance Mark Oct 12 '09 at 12:25
  • Beyond that conditional jumps perform better than v-table lookups and method calls – Matthew Whited Oct 12 '09 at 13:22
  • 1
    @High-Performance Mark: I disagree, predicates are **not** syntactic sugar for IF. They allow decoupling of code which would otherwise be placed in a single if/switch block, possibly scattered all over the program. – vgru Oct 12 '09 at 13:34
  • @Matthew: static methods perform better than instance methods, but that is not something that you should consider when programming. @Kyle: I wouldn't say that using predicates would make your code a nightmare, quite the opposite actually, but their use is simply not always justified (due to extra work involved in refactoring when not necessary). – vgru Oct 12 '09 at 14:05
  • 1
    @Groo: why not know how a system really works. It can help find bottle necks before you run into them. I find knowing how the underworkings of the entire system works help me build cleaner and faster code. – Matthew Whited Oct 12 '09 at 15:35
16

There is the Anti-If campaign which started earlier in the year. The main premise being that many nested if statements often can often be replaced with polymorphism.

I would be interested to see an example of using the Predicate instead. Is this more along the lines of functional programming?

Keith Bloom
  • 2,391
  • 3
  • 18
  • 30
  • 2
    Hadn't even heard of the anti-if campaign. Seems a bit extreme, but I agree with the basic tenants of what they're saying. – Jason Baker Oct 12 '09 at 12:25
  • 7
    This looks and sounds like another for-profit subreligion started in hopes of getting coders to buy books about something that they'll better and more completely learn with practice. – Mike Burton Oct 12 '09 at 12:32
  • 3
    Looks like they're not anti-if, they're anti-switch-written-as-ifs. – Alex Feinman Oct 12 '09 at 12:35
  • 1
    I haven't heard of the campaign, but I ripped up some hugely complex code full of if blocks recently, and replaced it with OOP. The code went from incomprehensible buggy rubbish to a solution that was very elegant, clean, easy to understand, and easy to extend. Definitely recommended. if(possible) ;) – Lee B Oct 12 '09 at 12:48
  • @Mike and Alex, I agree that the anti-if campaign is a poor example of what I was trying to express. Thanks to Lee for doing that. Another way is summarised here http://www.refactoring.com/catalog/index.html – Keith Bloom Oct 12 '09 at 13:44
  • 4
    **sigh** So, basically, I have to radically complicate my code just to get rid of an 'If' statement? This is tantamount to employing code obfuscation to satisfy code purists. Thank you, but no. – Mike Hofer Oct 12 '09 at 15:17
  • 1
    @Mike Hoffer - No, you shouldn't complicate your code to get rid of an if statement. It *is* good to simplify your code by getting rid of an if statement though. – Jason Baker Oct 12 '09 at 15:28
  • 1
    @Jason - It is good to simplify your code by any means necessary. If your code actually becomes simpler and easier to understand by using a `goto` then you use a `goto` however "evil" it may be. If it doesn't become simpler with a `goto` (as is so often the case), then don't use it. – Chris Lutz Oct 12 '09 at 17:12
  • 1
    Their example code "The Code Monster" does have 2 `if`s but also Italian variable names. I think I can't take this seriously. – amoebe Apr 22 '16 at 21:03
12

Just like in the bible verse about money, if statements are not evil -- the LOVE of if statements is evil. A program without if statements is a ridiculous idea, and using them as necessary is essential. But a program that has 100 if-else if blocks in a row (which, sadly, I have seen) is definitely evil.

Kaleb Brasee
  • 51,193
  • 8
  • 108
  • 113
10

I have to say that I recently have begun to view if statements as a code smell: especially when you find yourself repeating the same condition several times. But there's something you need to understand about code smells: they don't necessarily mean that the code is bad. They just mean that there's a good chance the code is bad.

For instance, comments are listed as a code smell by Martin Fowler, but I wouldn't take anyone seriously who says "comments are evil; don't use them".

Generally though, I prefer to use polymorphism instead of if statements where possible. That just makes for so much less room for error. I tend to find that a lot of the time, using conditionals leads to a lot of tramp arguments as well (because you have to pass the data needed to form the conditional on to the appropriate method).

Jason Baker
  • 192,085
  • 135
  • 376
  • 510
  • 5
    Martin Fowler says comments are a **sweet smell not a bad smell**. Let me quote from the "bad smells" chapter in *Refactoring*. "...comments aren't a bad smell: indeed they are a sweet smell... [but] they are often used as a deodorant. It's surprising how often you look at thickly commented code and notice that the comments are there because the comments are bad." – MarkJ Oct 13 '09 at 11:52
  • 1
    @MarkJ finally! Someone actually quoting Fowler correctly. +1. – reccles Oct 14 '09 at 23:22
9

if is not evil(I also hold that assigning morality to code-writing practices is asinine...).

Mr. Haines is being silly and should be laughed at.

Paul Nathan
  • 39,638
  • 28
  • 112
  • 212
6

I'll agree with you; he was wrong. You can go too far with things like that, too clever for your own good.

Code created with predicates instead of ifs would be horrendous to maintain and test.

Rap
  • 6,851
  • 3
  • 50
  • 88
5

Predicates come from logical/declarative programming languages, like PROLOG. For certain classes of problems, like constraint solving, they are arguably superior to a lot of drawn out step-by-step if-this-do-that-then-do-this crap. Problems that would be long and complex to solve in imperative languages can be done in just a few lines in PROLOG.

There's also the issue of scalable programming (due to the move towards multicore, the web, etc.). If statements and imperative programming in general tend to be in step-by-step order, and not scaleable. Logical declarations and lambda calculus though, describe how a problem can be solved, and what pieces it can be broken down into. As a result, the interpreter/processor executing that code can efficiently break the code into pieces, and distribute it across multiple CPUs/cores/threads/servers.

Definitely not useful everywhere; I'd hate to try writing a device driver with predicates instead of if statements. But yes, I think the main point is probably sound, and worth at least getting familiar with, if not using all the time.

Lee B
  • 2,137
  • 12
  • 16
4

The only problem with a predicates (in terms of replacing if statements) is that you still need to test them:

function void Test(Predicate<int> pr, int num) 
{
    if (pr(num))
    { /* do something */ }
    else
    { /* do something else */ }
}

You could of course use the terniary operator (?:), but that's just an if statement in disguise...

Matthew Scharley
  • 127,823
  • 52
  • 194
  • 222
  • 2
    With lazy evaluation or if expressions have no side effects, you could also use predicates returning venerable Lambda booleans, where `true = (a,b) => a` and `false = (a,b) => b`, but that's also just an 'if' in disguise. – outis Oct 12 '09 at 12:20
4

Perhaps with quantum computing it will be a sensible strategy to not use IF statements but to let each leg of the computation proceed and only have the function 'collapse' at termination to a useful result.

user229044
  • 232,980
  • 40
  • 330
  • 338
High Performance Mark
  • 77,191
  • 7
  • 105
  • 161
4

Sometimes it's necessary to take an extreme position to make your point. I'm sure this person uses if -- but every time you use an if, it's worth having a little think about whether a different pattern would make the code clearer.

Preferring polymorphism to if is at the core of this. Rather than:

if(animaltype = bird) {
     squawk();
} else if(animaltype = dog) {
     bark();
}

... use:

animal.makeSound();

But that supposes that you've got an Animal class/interface -- so really what the if is telling you, is that you need to create that interface.

So in the real world, what sort of ifs do we see that lead us to a polymorphism solution?

if(logging) {
      log.write("Did something");
}

That's really irritating to see throughout your code. How about, instead, having two (or more) implementations of Logger?

this.logger = new NullLogger();    // logger.log() does nothing
this.logger = new StdOutLogger();  // logger.log() writes to stdout

That leads us to the Strategy Pattern.

Instead of:

if(user.getCreditRisk() > 50) {
     decision = thoroughCreditCheck();
} else if(user.getCreditRisk() > 20) {
     decision = mediumCreditCheck();
} else {
     decision = cursoryCreditCheck();
}

... you could have ...

decision = getCreditCheckStrategy(user.getCreditRisk()).decide();

Of course getCreditCheckStrategy() might contain an if -- and that might well be appropriate. You've pushed it into a neat place where it belongs.

slim
  • 40,215
  • 13
  • 94
  • 127
3

It probably comes down to a desire to keep code cyclomatic complexity down, and to reduce the number of branch points in a function. If a function is simple to decompose into a number of smaller functions, each of which can be tested, you can reduce the complexity and make code more easily testable.

madlep
  • 47,370
  • 7
  • 42
  • 53
3

IMO: I suspect he was trying to provoke a debate and make people think about the misuse of 'if'. No one would seriously suggest such a fundamental construction of programming syntax was to be completely avoided would they?

Gordon Mackie JoanMiro
  • 3,499
  • 3
  • 34
  • 42
  • On the one hand I agree with you, on the other hand i'm sure they were saying that about gotos 10-15 years ago :), +1 – RCIX Nov 06 '09 at 10:13
2

Good that in ruby we have unless ;)

But seriously probably if is the next goto, that even if most of the people think it is evil in some cases is simplifying/speeding up the things (and in some cases like low level highly optimized code it's a must).

Jakub Troszok
  • 99,267
  • 11
  • 41
  • 53
1

I think If statements are evil, but If expressions are not. What I mean by an if expression in this case can be something like the C# ternary operator (condition ? trueExpression : falseExpression). This is not evil because it is a pure function (in a mathematical sense). It evaluates to a new value, but it has no effects on anything else. Because of this, it works in a substitution model.

Imperative If statements are evil because they force you to create side-effects when you don't need to. For an If statement to be meaningful, you have to produce different "effects" depending on the condition expression. These effects can be things like IO, graphic rendering or database transactions, which change things outside of the program. Or, it could be assignment statements that mutate the state of the existing variables. It is usually better to minimize these effects and separate them from the actual logic. But, because of the If statements, we can freely add these "conditionally executed effects" everywhere in the code. I think that's bad.

VB Guy
  • 99
  • 1
  • 2
  • 1
    This is not true... plus ternary operators are impractical in the extreme for even decent sized expressions, they aren't readable. An if statement in a project that is based around side effects is not a bad thing. `if(socket) socket.connect else say "Socket done goofed"` for example – daniel gratzer Oct 22 '12 at 01:03
-3

If is not evil! Consider ...

int sum(int a, int b) {
    return a + b;
}

Boring, eh? Now with an added if ...

int sum(int a, int b) {
    if (a == 0 && b == 0) {
        return 0;
    }
    return a + b;
}

... your code creation productivity (measured in LOC) is doubled.

Also code readability has improved much, for now you can see in the blink of an eye what the result is when both argument are zero. You couldn't do that in the code above, could you?

Moreover you supported the testteam for they now can push their code coverage test tools use up more to the limits.

Furthermore the code now is better prepared for future enhancements. Let's guess, for example, the sum should be zero if one of the arguments is zero (don't laugh and don't blame me, silly customer requirements, you know, and the customer is always right). Because of the if in the first place only a slight code change is needed.

int sum(int a, int b) {
    if (a == 0 || b == 0) {
        return 0;
    }
    return a + b;
}

How much more code change would have been needed if you hadn't invented the if right from the start.

Thankfulness will be yours on all sides.

Conclusion: There's never enough if's.

There you go. To.