Monday, June 13, 2005

OnlyOneReturn Considered Harmful

WARNING: This blog entry was imported from my old blog on blogs.sun.com (which used different blogging software), so formatting and links may not be correct.


The other day I

mentioned PMD
and how it's vital to learn how to customize the ruleset, since by default
it includes some rules that are evil. Evil I tell you!



My favorite rule to hate is the OnlyOneReturn rule. This rule basically
says that


A method should have only one exit point, and that should be the last statement in the method.


Unfortunately, the rule author does not provide supporting documentation
for this point of view, and I personally cannot think of any.
In fact, restricting yourself to a single return statement makes the code
less readable. Take the following method from the JDK's Integer
class for example.


public static String toString(int i) {
if (i == Integer.MIN_VALUE) {
return "-2147483648";
}

int size = (i < 0) ? (stringSize(-i) + 1) : stringSize(i);
char[] buf = new char[size];
getChars(i, size, buf);

return new String(0, size, buf);
}


We have a special case, and it's dealt with right at the beginning of
the method. If we hadn't done that, we would need to slap a big else
block around the remainder of the code, as well as introduce a temporary
variable, e.g. result, to hold the return value that we then
return as the last statement of the method.



The above is a trivial example, but I frequently write code where I have
multiple exit points early in the method that deal with special cases, and
this avoids the need for deeply nested code.



Creator's Visual Page Designer Source Code: Proud OnlyOneReturn Rule Violator
Since 2003.


7 comments:

  1. Amen to that, it was the first rule I blitzed as well. However the standard PMD ruleset has a few other problems as well as that one - see http://bleaklow.com/blog/archive/000157.html

    ReplyDelete
  2. I agree that you the "only one return" rule may increase the complexity of the source code, and I can't think of a single case it might increase the readability. I do however _really_ dislike the goto statement...

    ReplyDelete
  3. It's funny seeing "goto" being brought up here, since I chose a title for this blog entry, "OnlyOneReturn considered harmful", as a play on the title of the classic anti-goto article by Dijkstra: "Go To Statement Considered Harmful".
    http://www.acm.org/classics/oct95/

    ReplyDelete
  4. I use the "finally" structure in Java for that (ensuring that code is run before the method is exited).

    ReplyDelete
  5. Yup, I like doing early exits too... so I put that one in the "controversial" ruleset. Some folks do like it, though, so we keep it around.

    ReplyDelete
  6. Yeah, I think this is mostly a task for the NetBeans plugin developers now, since in the current plugin version, that's where the list of rules to run (which includes OnlyOneReturn) are taken from.
    I'm glad to hear it's in the controversial set :) Sorry if I got a little out of hand here - I guess I take coding style very seriously. But that's also why I love PMD!

    ReplyDelete
  7. Sure, yup, no problem, I definitely agree with you... I certainly don't like all the rules. Actually, I never run all the rules; I just run basic, unusedcode, and pick out a couple from design and whatnot.

    ReplyDelete