A method should have only one return statement. Discuss.

Anthony.Steele — Mon, 14 Jul 2008 14:56:00 GMT

I heard again recently that "A method should have only one return statement" and of "The single-return law", that more than one return was a poor coding practice. I disagree.

public int IsThisCodeSoBad(int param) {    if (param == 0)      return 0;

   foreach(foo in bar)    {       If foo.fish()       {         return 1;       }    }

   return 2; }

I have never seen any evidence for the belief that that multiple exit points pose a coding risk, or are bad style. I do not know of any formal study that has found this to be the case, on C# code or on code in any other language.

Coding horror recently talked about "Spartan programming", and mentioned "frugal use of control structures, with early return whenever possible."

One of the few links on the topic that I could find was "Return statements and the single exit fantasy":

http://www.leepoint.net/JavaBasics/methods/method-commentary/methcom-30-multiple-return.html

Take a look at some of the comments on the Daily WTF (http://thedailywtf.com/Articles/Refactoring_to_the_Max.aspx ) where this issue is raised, particularly this one:

Re: Refactoring to the Max

2006-04-05 02:25 • by John Hensley

The main reason for the single return "law" in C is to make sure you clean up memory, locks, handles, etc. before you leave a function.

This, on the other hand, is Java code.

Apparently the reason for this "law" was that when coding in C or the like, it was natural to deallocate resources near the end of the function. Thus any extra exit points were an invitation to memory leaks. There are two reasons why this is no longer the case:

Firstly, garbage collected languages make explicit deallocation unnecessary in most cases.

Secondly, try...finally blocks and using statements allow deallocation to happen with greater certainly at the end of any block of code when it is needed.

In any event, a function with multiple exit points is a far lesser issue than a goto. In some cases it is the simplest way to code now that we have control structures to deal with it. But beware of returns in the middle of loops or highly nested code. All other things being equal, avoid them, since they are less readable and predictable.

It is absolutely fine to use it as a first check on the parameters after entering a function, before getting down to the serious work and allocating those resources,– it's more readable than embedding the rest of the function in an if-block. However throwing an exception is the more usual case for this.

My view of the matter is:

There are cases where a single return is more readable, and cases where it isn't. See if it reduces the number of lines of code, makes the logic clearer or makes the control structures less deeply nested.
More than one return style may be a bad habit in C code, but there is no evidence for it being bad in more modern languages.
There is no law requiring only one exit point for a method. Some people have an unsubstantiated belief in the superiority of this style.
Requiring fewer return statements for purposes of centralising resource deallocation is not a common need in modern languages that have garbage collection and try..finally blocks.

Therefore, use as many returns as suits your artistic sensibilities, because it is a layout and readability issue, not a technical one.

There's another issue at play here, about rules being adhered to without people grasping the reason why they are adhering to them, and thus keeping the rule in force after the need for it has evaporated. Rules should be given along with reasons for them.

Addendum:

Watching the DailyWTF commentators (who have a fairly high opinion of their own coding skills, given that they are essentially gathered there to point and laugh) try and repeatedly fail to translate the following code into a single expression underscores that the multiple-exit points version can be in some cases easier to understand, read, and modify:


if (condition1(x)) 
  return true;
if (condition2(x)) 
  return false;
if (condition3(x)) 
  return true;
return false;

You'd think that this is the same as

return condition1(x) || !condition2(x) || condition3(x);,

However the one-liner version returns true if all conditions are false, unlike the original. Furthermore, it evaluates condition3 if condition2 is true, which the original does not.

You could also code it as:

bool result = false;

if (condition1(x))
result = true;
if (condition2(x))
result = false;
if (condition3(x))
result = true;

return result;

However this also evaluates condition3 if condition2 is true, which the original does not. I'm sure that the original logic can be replicated with only one return, but it's not going to be as elegant.

Update: Richard Tobin has sent me this comment by email:

This discussion is a common one that we continually face where I work. I took your example and reworked it into a single return as follows:

private bool LotsOfReturns(int x)
{
    if (Condition1(x))
        return true;
    if (Condition2(x))
        return false;
    if (Condition3(x))
        return true;
    return false;
}

private bool OneReturn(int x)
{
    bool flag = Condition1(x);

    if (!flag)
    {
        if (!Condition2(x))
        {
            flag = Condition3(x);
        }
    }

    return flag;
}

After compiling and looking at the IL with Reflector, the single return routine seemed more compact and efficient, and the C# source seems less tricky.

All I can really say is that he has suceeded in converting the code, and which one "seems less tricky" is going to be a matter of taste.

Anthony Steele's Blog : law

A method should have only one return statement. Discuss.