how could conditionals be evil?

Posted by Steven Soroka 24 Nov 2009

If you’ve seen the Anti-If Campaign, and overheard talk about the “if” statement being evil, you might be thinking “Why? It’s a core part of any language, and you couldn’t write a program without them”, and you’d be right.

The conditional is not evil itself, it smells of evil. The conditional invites evil not because of what it does, but because of what it shouldn’t be doing. It’s an indication of a code smell, and should be a metric that you track. If you don’t track it, you need to at least know what to watch for.

For example: I was working on my code today, and I found a constant that violated the Single Responsibility Principle. It managed both A) the various pages in a checkout process, and B) which “next” and “previous” buttons displayed on each page.

STATES = {
  :details => [nil, :payment],
  :payment => [:details, :final_review],
  :final_review => [:payment, :confirmation],
  :confirmation => [nil, nil]
}

The subtle problem here is that the confirmation page doesn’t have a back button, but the application can easily send the user back a page if the payment processing fails for some legitimate reason (Maybe the payment gateway is down?). This seemingly innocuous violation of SRP caused an explosion of IF statements throughout the application. One of the best examples of this is:

if !@checkout.valid?
  if @checkout.state.previous.present?
    @checkout.state.previous!
  elsif @checkout.state.is_confirmation?
    @checkout.state.set_state(:final_review)
  end
end

Three conditionals just to move back a page when there are errors. Once I removed the violation, the code nicely cleaned up to a single line with one reasonable conditional:

@checkout.state.previous! if !@checkout.valid?

Conditionals themselves are not evil, they’re needed to get the job done. Take a good look at your code, though; if you see a lot of “if”, “case”, ternary, or other conditionals, you likely have a problem somewhere. Move the conditional in to the class that’s responsible for it, avoid case statements that do something different depending on some enumerator value or state.

Conditionals smell a little. In numbers, they outright stink!