Monday June 13, 2005
OnlyOneReturn Considered Harmful 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.
(2005-06-13 10:52:23.0) Permalink Comments [9]Post a Comment:
Comments are closed for this entry.
Posted by Alan Burlison on June 13, 2005 at 01:52 PM PDT #
int func(int param) { int result; if( precondition ) { return somesimpleresult; } /* mainbody */ goto CommonExit; ErrorExit: /* Do error cleanup */ Commonexit: /* common cleanup */ return result; }<tt>goto</tt>s are only ever forward branching and towards the eventual <tt>return</tt>. In Java code <tt>finally</tt> clauses are roughly equivalent.Posted by Mike Duigou on June 13, 2005 at 08:30 PM PDT #
Posted by Trond Norbye on June 14, 2005 at 05:56 AM PDT #
Posted by Tor Norbye on June 14, 2005 at 06:07 AM PDT #
{ T result = <defaultValue>; ... return result; }is that you can watch the result variable in the debugger. Not sure whether you can watch the method's return value. <p/> There may be other reasons that are more important to C than to Java developers. At least I have met a lot of these girls and guys that passionately hated code that uses multiple exit points. <p /> For method bodies that are 10 lines or fewer (like your sample) it is hard to see why one style should be easier to comprehend than the other.Posted by George on June 23, 2005 at 04:40 AM PDT #
Posted by Tor Norbye on June 23, 2005 at 06:21 AM PDT #
Posted by Tom Copeland on July 10, 2005 at 12:10 AM PDT #
Posted by 192.18.98.64 on July 10, 2005 at 12:59 AM PDT #
Posted by Tom Copeland on July 10, 2005 at 10:30 AM PDT #