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?