Monday, December 06, 2004

Multiple Return Statements

Ted Hill writes:


Hi Bruce,

I attended your design seminar in Crested Butte this summer. Thanks for hosting a great seminar, I thoroughly enjoyed the entire experience.

On my current job we work in Java and are putting together a list of "best practices" for our company.

The idea is that we will run the PMD inspection tool on our Java code.

One inspection that some of our developers favor is:
"There must only be a single return statement within each method"

PMD classifies this as a "controversial" inspection. They do not comment on why it is considered controversial.

I notice that in your book "Thinking in Java" you have several examples of multiple returns within a method.

I personally favor using it for 'breaking' out of loops as you do on the top of pg 490 (Thinking in Java 2nd Edition) in the code for demonstration hashed Map.

Here is the method:


public Object get(Object key) {
int index = key.hashCode() % SZ;
if(index < 0) index = -index;
if(bucket[index] == null) return null;
LinkedList pairs = bucket[index];
MPair match = new MPair(key, null);
ListIterator it = pairs.listIterator();
while(it.hasNext()) {
Object iPair = it.next();
if(iPair.equals(match))
return ((MPair)iPair).getValue();
}
return null;
}

So I am wondering:

1. What is your personal feeling about the "single return rule"?

2. Do you know anything about the history/background of this rule/inspection?

I happened to be thinking about this recently when refactoring some code, noticing that taking some kinds of inline code and turning it into a method with multiple return statements can really clean things up. It is definitely an idiom that I find valuable for that very thing.

I believe the history of this may come from Dijkstra's famous "goto considered harmful" statement, which has been massively misunderstood, in my opinion. I think the problem that Dijkstra was talking about was wild jumps from any part of the code to any other part, without constraint. The closest thing we have in Java is the labeled break and labeled continue, which you'll notice are jumps, but are very constrained.

I think that multiple return statements only seem to be jumps. But they all have to go through the "return chute" and they must return the specified type, the stack gets cleaned up, finally clauses are executed, etc. So not only do I find them completely safe, but their benefits in terms of clearer code are significant.

The counter-argument to this may be that it could be hard to see all the places where you exit a method if they are distributed throughout the method. My feeling is that if this is the case then your method is probably too large and complex and that you should probably refactor it to make it cleaner. Or you should choose not to use multiple returns for that method. Enforcing "no multiple returns" because you might end up creating a confusing method is a bad policy, I think. If someone is going to create a confusing method, preventing them from using multiple returns probably won't do any good, and it will certainly harm the clarity of other methods.

Guidelines and tools to point them out are helpful, but at some point programmers need to develop a sense of good code. I don't think there's any way around that. The only way to enforce it, I believe, is code reviews, ideally in a group so that everyone can learn from them (as noted in a previous entry, this can be tricky because you want to focus on improvement rather than making people feel bad).