Continued Slander against the Noble If
I’ve often said that if statements are good indicators of poorly designed code. To illustrate my point, here is some code I recently stole from a major auto insurance firm1:
def Money calculateAutoInsurance
if (person is-a Man)
Man man = (Man)person
if (man.canGrowBeard)
return $50
else
return $2000
end
else if (person is-a Woman)
return $10
else
kaboom!
end
end
When casting or using any is-a
operator such as instanceof
in Java or kind_of?
/is_a?
in Ruby, you should stop to think about whether this is really the best solution. Sometimes it is necessary such as in Java when implementing the equals method or when hacking around a poorly designed third party library. If you are able to modify the classes you are calling though, a simpler solution is to push the conditional logic into the derived classes.
The Liskov Substitution Principle and substitutability are helpful in understanding how to design proper polymorphic classes. When a caller is acting upon an interface, one should be able to substitute any type that implements that interface. In this case, the caller is breaking substitutability by casting to a specific derived type and basing logic on what the derived type is.
Code that follows the substitutability principle is better for several reasons:
- Higher cohesion. We can leave the polymorphic dispatch logic up to the language itself so that the calling code doesn’t have to worry about the specifics of manipulating derived types.
- Better encapsulation. If canGrowBeard was only exposed for auto insurance calculations, it can be rehidden when we push the auto insurance calculation into the derived Persons.
- Safer/Easier to extend. For example, it will be easier to add a third gender for Person if we know the caller always follows the substitutability principle. I would not advocate removing simplicity to add this extensibility, but given that it is simpler, the extra extensibility is a nice bonus.
class Man extends Person
...
def Money autoInsurance
if (canGrowBeard)
return $50
else
return $2000
end
end
...
end
class Woman extends Person
...
def Money autoInsurance
return $10
end
...
end
BONUS POINTS for anyone that can tell me what is still highly suspicious about the design for the final solution.
- Any illegal activity presented here is fictional and any resemblance to illegal activity past, present, or fictional is purely and completely coincidental.