Guard Clauses Considered Harmful
A colleague and I have had a long running debate on Guard Clauses, which is an example of "Alligators versus Crocodiles". A previous post tries to present the "for" case.
Now, it's hammer time.
IMO, the canonical form for a Java method is like so:
public MyObject doSomething(Object a, Object b) {
MyObject result = null;
// stuff goes here
return result;
}
It's old-school: a single point of return. Any checks should concentrate on the positive and relegate error messages, exceptions, etc to the else statements at the bottom of the method.
For example:
if( a != null ) {
if( b != null ) {
doSomething();
} else {
// throw exception
}
} else {
// log error
}
Here are the advantages:
- The logic of the method is easy to understand: set the default case first, then change it if necessary. It quickly becomes a pattern that makes reading and debugging code easier.
- It is easy to enforce. There is no slippery slope as with guard clauses. Guard clauses easily degenerate into "open season" for returning from methods all over the place. This is much harder to maintain, and is more prone to bugs, than the (admitted) indentation issue. IDEs can help match braces etc with respect to indentation: may heaven help you if you need to understand a method with several return methods. Note that the example of guard clauses (here) is pristine. It's not always this way.
- Stating the above in a different way, it's easy to scan and find the "heart" of the logic. The corner cases of exceptions and error logging is at the bottom of the method. Guard clauses puts that stuff right up front, which strikes me as being inverted.
- Using a convention is extremely powerful: note that methods that use this format are amenable to being processed by simple scripts etc. e.g. If one wanted to insert "pre"/"post" logic logging (though admittedly this is surpassed by using AOP).
- In the purest form, guard clauses are brief statements at the top of the method and, yes, they can be thought of as a pseudo-contract. But when scanning through the method, there is something jarring about seeing a return statement. It invariably opens another thread in my head for that case, and as the returns stack up, so too do these threads. It is distracting.
- An oft-cited argument for guard clauses is that methods in Java are shorter. Hmmm.... maybe. Example methods are shorter, and the pristine methods you and I write are shorter, but does the average Java Joe/Jane really write shorter methods than, say, C++? IMO, the above convention does not harm short methods but definitely assists the reader with the dreaded longer methods. The proper use of guard clauses is just too subtle for Java Joe/Jane.
Do you see guard clauses as an alligator or a crocodile?
ps. I fully concede that this convention doesn't have a sexy term like guard clause and does not get press in popular books. Perhaps fans of this idea can give it a snazzy name (Return on Rails?)
