DEV Community

Discussion on: If/else or just if?

Collapse
 
simbo1905 profile image
Simon Massey • Edited

I vote for a mixed style: return early if there is an obvious default case (a guard) then use an if/else for some set of related cases. My reasoning is below.

I first ran into this waaaay too long ago in a code review with someone who said "a method should have one exit point" whereas i was liking the "exit early" or "guard" approach. I argued "but if you use early returns you don't have the cognitive overhead of needing to reading the whole method". Yet to them "single return" was The One True Way so we agreed to differ. My guideline for what to do when two devs disagree on a purely stylist matter is that the code remains as-is. I actually cannot remember whether it was my code or their code that was under review.

Unfortunately early returns is often a rotten code smell of "couldn't be bothered to refactor to add a new feature". In that anti-pattern you have a God Object / God Function and to add new features you add a fresh block at the top of the enormous "doAnything" method to detect your new scenario, call your new feature logic in another file, then return early. Then "ta da!" you never have to refactor the God code or change its unit tests (which are incomplete) nor manually test anything other than your new feature case. This leads to an immediate Broken Glass model where the next dev copies that approach. Pretty soon you have a thousand line method that keeps on growing with every new feature added. Regrettably large OO code bases that have seen any team turnover tend to default to this sort of thing remarkably fast.

If you tempted to return early in more than a single obvious default case then perhaps you should break up the class or method into different units that can be composed where each unit has an if/else set for some closely related cases. You then end up with smaller functions or classes that handle a tight set of cases. Then you can take a mixed approach. Try to only return early if there is a really common default case. Why? Because during debugging you don't want someone to have to read the whole code if there is a very common case. The more early returns you put in the more the code is asking to be broken up. The more complex the if/else starts the more you should break it up into different units that handle a more narrow set of cases.