Browsing through the code base of one of our customers I frequently stumbled over methods that were roughtly structured like this:
void theMethod { if (some_expression) { // rest of the method body // ... } // no more code here! }
And most of the time I was tempted to refactor the method using a guard clause, like so:
void theMethod { if (!some_expression) { return; } // rest of the method body // ... }
because this is far more readable for me. When I noticed that the methods were written all by the same guy I told him about by refactoring ideas in absolute certainty that he would agree with me. It came as quite a surprise when, in fact, he didn’t agree with me, at all. Even something like this:
void theMethod { if (some_expression) { // some code // ... if (another_expression) { // some more code // ... } // no more code here .. } // ... and here }
was in his eyes far more readable than the refactored version with guard clauses. His rational was that guard clauses make it harder for to see the program flow through the method. And a nested if(…) structure like above was very suitable to express slightly more complicated flows.
All my talks about crappy methods and the downsides of highly indented code were not able to change his mind.
I admit that I can somewhat understand his point about the visibility of the program flow through the method. And sure, the (nested) ifs increase indentation and the number of possible code paths but since there are no elses and no code after the if-blocks, does that really increase the overall complexity?
Well, I still would prefer smaller methods with guard clauses but as you can see, to a great extend readability lies in the eyes of the beholder.
What do you find readable?
I had this discussion myself a couple of times (maybe with the same guy), and I wonder why the return count didn’t come up as argument. “Seeing the control flow” and “Single point of return” imho is a construct that was inherited from the good old C days. It is readable to someone who a) never did anything else and b) isn’t willing to see guard clauses as a pattern that makes it easy to see the method bodies invariants. I have coded with both styles in separate products for years and the only thing I see positive in the not-guard-clause-style is that it makes “extract method” easier and a bit safer. But once you do that, your method again is small and the remaining single if-statement can easily be converted to a guard clause.
Another problem that I see with nested ifs is that you always have to check that the “else” part is empty or not. It is far too easy to overlook such a case. It is far more difficult to check and understand such a solution than to use guard clauses which clarify the code coming after the checks. I like the separation of checking the invariants and the “flesh” of the method.
You are not alone… Last week we refactored a part of an older project – it was full of such constructs. In my opinion it is even simpler to refactor a method if you use guard clauses because your code tends to contain small (often independent) blocks.
If you are so worried about readability, why do inject the grammatical production into all of your “if” statements? The curly braces that group a statement list in conditional and iterative control structures are not part of these control structures. Curly braces belong to a grammatical production often referred to as . The grammatical production for is “if” “(” “)” “else” . The reason why one can use curly braces in a or statement is because the production is on the right-hand side of the production. The grammatical production for is “{” “}”.
old post but I recently published an open source library offering fluent guard clauses that might help with this. It eliminates nesting, increases readability, and reduces code needed for guard clause implementation. You might check it out and let me know what you think https://github.com/chrisdostert/guard-clauses-for-java