January 10, 2008

Else is evil

What is the purpose of the else statement?
When you write if statement, you are expressing explicit condition, which the code following if will be executed under. You ensure that the code will run, only when the condition holds. What about else? By using the else, you are stating, that you don't mind the condition, potentially grouping multiple conditions under one statement. To illustrate the point, i will use the snippet of code

int deliveryType = order.getDeliveryType();

if(deliveryType == Delivery.NORMAL) {
orderQueue.placeOrder(order);
} else {
orderQueue.placePriorityOrder(order);
}

What's wrong with this code? Everything will work fine, unless there are only two types of order. The code explicitly handles normal order and then groups every other order and treats it the same. As soon as there is new order type added this code would break.

I believe that as the if expresses exactly one condition, the else too should express exactly one condition, or if this is not the case, then it should serve as a guardian. A guardian before the inevitable changes.

int deliveryType = order.getDeliveryType();

if(deliveryType == Delivery.NORMAL) {
orderQueue.placeOrder(order);
} else if(deliveryType == Delivery.EXPRESS) {
orderQueue.placePriorityOrder(order);
} else {
throw new RuntimeException("Unknown delivery type.");
}

The code above illustrates much better approach. The else serves here as a guardian. The application will work, and when there is new delivery type requested it will immediately break unit tests and signal the cause of problem.

if(order.isNew() == true) {
getOrderDao().save(order);
} else {
getOrderDao().update(order);
}

Using else here is appropriate too, as it represents exactly one condition. A boolean can never be anything else than true, or false. True or false, two conditions, two branches and there is no way for third.


5 comments:

paulo said...

Or you could enter the 21º century and use polymorphism in your getDelivery() result, you know grouping data and behavior in classes?

paulo said...

And what is it with the == true

Tomáš Kramár said...

It was not an example of good domain model, I agree, nor was in intended to be. It was an example of if-else chain.

COTOHA said...

Quote: Using else here is appropriate too, as it represents exactly one condition. A boolean can never be anything else than true, or false. True or false, two conditions, two branches and there is no way for third.

it's rather amusing reading this as you might want to know that if statement always evaluates to... boolean :) true or false. if not NullReferenceException of course...

Ivan Assenov said...

Great topic. I never understood the abuse of if/else and I use it very seldom. Without knowing that blog I wrote an article having similar title If Else is evil(http://litun.blogspot.com/)
After I published it I started looking on the Web for something similar and I found this one.
I have found many bugs over the years that came from using if else not in the right place.