I am almost always dismayed when I see a method with too many if statements. More often than not, this is a precursor to similar conditions in other methods in the same or other classes, using the same condition to switch the actions to be called. This is an anti pattern I’ve seen repeated at far too many places. I think the anti if campaign mirrors my thought process about what needs to be done.
Consider a simplistic case of getting price for standard and premium customers for membership of a club.
- It is possible to have a get price method on the customer class with an if statement. The drawback is that the class/method hides complexity, anybody changing the code will have to go around searching for the if statements (and they multiply very quickly) altering behaviour on a boolean.
- Very soon we’ll need a third type of customer, say executive class. So now the booleans will not do, so we have a customer type enum – and the saga continues! Very soon we have spaghetti methods branching off from different if statements – usually in private methods to make the original method look small, again hiding complexity.
- The conditionals will probably have to continue at every place the code is called. The unit tests, if there are any, will have to be numerous and/or huge, creating headaches of their own. Quite often, these get excluded from code coverage when there is no time to sit and work out why they are failing – very likely never to be included again or deleted altogether.
Now repeat this process for EVERYTHING that needs to be done for the customer – ticket price, parking price, areas allowed in etc. Consider their dependents being given memberships based with some similar and other different conditions. And now add a couple of more membership types. Besides the spiralling cost of each of these changes, the architecture of this not so big app will probably resemble a big ball of mud.
The OO way would be to create different customer type classes inheriting from a base customer class and overriding the get price method, or parts of get price method where their implementation is different. The base class can act like a façade for the general behaviour of all the types of customers, which other layers work with.
The behaviour could also be delegated to a new property holding the state of the customer class e.g. in case of account state being in arrears, surplus or up to date. Again, the actions will be called on the state classes after the initial decision of assigning the state is done at the top level.
The biggest benefit is that classes of the app reflects the architecture/complexity, with a number of them in different folders indicating what patterns have been used. The classes themselves can afford to be small, hopefully overriding behaviour from the ones they inherit i.e. encapsulating what varies.
This way of doing things is also likely to have better coverage of unit tests since it is simpler and quicker to create unit tests. The fact that the unit tests are small and don’t require big setups is also a big plus – they are more likely to be updated than huge unit tests which are difficult to work out. The classes and methods are also easier to understand quickly, a big plus in maintenance – which can be 40% – 80% of the cost of a piece of software.
So the if statements are there, but only at the creation of the classes, where the customer type is decided. It is explicit and thereafter all the required changes need to be only in the customer type specific class. Also this should be done from the very beginning, even if there’s just one if statement, as it’s the easiest to do. The person changing it later might not have time, patience or mandate to do it – the last is possible if the resulting methods are being called by other apps which might need change.
To refactor current code with such if statements, I’d recommend http://sourcemaking.com/refactoring/replace-type-code-with-state-strategy and http://sourcemaking.com/refactoring/replace-conditional-with-polymorphism.