single return statement: yes / no?

[M Easter]

Folks,

A friend and moderator has assured me that this topic might be of interest to this forum. It is simple enough to understand that it can easily fit in the beginner's area.

I'm curious if anyone insists on using a single return statement in a Java method. I believe it is a "best practice" -- a old, timeless technique from languages gone by. Others believe that a single return is an anachronism; there is even a refactoring pattern these days called "guard clause" which explicitly returns early (see http://tinyurl.com/2d6msh)

What do you think?

Mike

ps. I have blogged on this fairly extensively and there is some debate there (see http://codetojoy.blogspot.com). I have dubbed the issue as an "alligators versus crocodiles" because people tend to fall in one camp or the other (and yet it is more substantive than the "religious wars" of trivial things like brace placement).

[Jeanne Boyarsky]

Mike,
Definitely an interesting topic.

What about a third choice of "no more than two return statements" ? I'm in that camp. Having two makes sense - a guard and the "real" one. More than that tends to result in code that is too difficult to refactor because returns are strewn throughout.

[Bear Bibeault]

I'm an old and seasoned graduate of the School of Structure Programming. My first real language (I don't count BASIC) was FORTRAN. So by nature I tend to have one return statement per method, just out of habit if nothing else.

But no hard religion. I don't consider multi-returners blasphemers, and I never try to tell anyone that single-return is the one true way to light and goodness. And if it makes sense for a method to have multiple returns, I'll code it that way without feeling that I need to shower afterwards.

But ask me my opinion of scriplets in JSPs...

[M Easter]

Originally posted by Jeanne Boyarsky:

What about a third choice of "no more than two return statements" ? I'm in that camp. Having two makes sense - a guard and the "real" one. More than that tends to result in code that is too difficult to refactor because returns are strewn throughout.

Jeanne,

IMHO, once there is more than one, then the bubble bursts. Here are some thoughts (all IMHO):

(a) In the Pragmatic Programmer, Dave Thomas talks about the "Broken Windows" theory. (See http://tinyurl.com/6cy39) With that in mind, my concern is that once > 1 return occurs, others are likely to follow. Perhaps not by you or me but by that intern.

(b) Maybe it's me but when I'm debugging or reading code and see more than one return, my brain feels a need to go into "parallel" mode. I know that multiple returns does not equate to multiple threads, but still....

(c) It just seems so much easier! One sets the default value for the result and then only change it as necessary.

(d) This idea _is_ old, and is usurped by AOP: using a convention allows one to do neat things with the codebase via scripting languages. e.g. if you want to add timers/log statements to a set of methods (in an automated way), then at least it is tractable by using a convention. With multiple returns, it's not tractable.

[Henry Wong]

But no hard religion. I don't consider multi-returners blasphemers, and I never try to tell anyone that single-return is the one true way to light and goodness. And if it makes sense for a method to have multiple returns, I'll code it that way without feeling that I need to shower afterwards.

In total agreement. Maybe it is because I deal with a lot of client code, some really horrible, that multiple returns don't bother me.

I have seen some bad code with single returns per method. I have seen some really good code with multiple returns. Personally, I like having a single return, but in the end, if it make sense to have multiple, so be it.


As an anecdote, my first professional language was x86 assembly (I don't count the half dozen or so languages that I learned in school). With assembly, you must have only one return, and at the end.

The reason is because a return doesn't undo the stuff you put on the stack. By guaranteeing that the return is always at the end, it is easier to ensure all the push onto the stack is matched with a pop off of the stack. Of course, now you have a bunch of jumps (gotos) to the pop and return code...

Henry

[Stan James]

I don't mind multiple returns. The least offensive are "guard clauses" (I think I'm using the name right) like a string manipulation method that says if the input is null, return null. I'd rather not put the whole method inside an "if not null" test just to get one return.

Even so, if a method is more than a few lines long, multiple returns can be a Very Bad Thing. But I rather think any time somebody complains that some practice makes it hard to follow a complex method, the method length is more likely the problem.

[Jesper de Jong]

I prefer a single return statement but I'm not very "fundamental" about it. However, I did find a bug recently in the software that I'm currently developing that was caused exactly because there was an extra return statement, it looked like this:

public void someMethod() {
    // Do some work here
    for (int i = 0; i < ...; i++) {
        if (...) {
            // NOTE: Return statement here!
            return ...;
        }
    }
 
    // Some extra code here that was added later
    // NOTE: This would not get executed if the method already returned in the loop above!
    doSomething();
}

The problem was that the extra code below the loop, which was added to the method later, wasn't executed when the condition in the if-statement was true, so that the method returned from the loop immediately. Ofcourse this bug would have been prevented if I wouldn't have allowed multiple return statements.

[Ilja Preuss]

Originally posted by Jesper Young:
I prefer a single return statement but I'm not very "fundamental" about it. However, I did find a bug recently in the software that I'm currently developing that was caused exactly because there was an extra return statement, it looked like this:

public void someMethod() {
    // Do some work here
    for (int i = 0; i < ...; i++) {
        if (...) {
            // NOTE: Return statement here!
            return ...;
        }
    }
 
    // Some extra code here that was added later
    // NOTE: This would not get executed if the method already returned in the loop above!
    doSomething();
}

The problem was that the extra code below the loop, which was added to the method later, wasn't executed when the condition in the if-statement was true, so that the method returned from the loop immediately. Ofcourse this bug would have been prevented if I wouldn't have allowed multiple return statements.

Interesting example. Yes, disallowing multiple returns would have prevented that bug, but it looks to me like it wouldn't have made the code simpler. And the true problem to me is that the method looks too complex, which concealed the bug. So my preferred way of preventing that bug would have been to make the method easier to understand, for example by introducing an abstraction.

[M Easter]

A colleague of mine had a rule: in addition to a single return, also no break or continue statements. They cause bugs.

Often a while loop can simplify things. Consider this (think of a state machine):

 
public void someMethod() {
    // Do some work here
    int i = 0;
    booolean ok = true;
 
    while( ok && i < X ) {
        if(...) { 
            ok = false;
        }
        i++;
    }
 
    if( ok ) {
        doSomething();
    }
}   

One might argue that the while loop can cause an infinite loop if the person forgets the "i++;" but that will fail very quickly and be discovered during testing.

Yes, the same thing can be done with a for statement and multiple predicates, but to me this is the most elegant.
[ May 02, 2007: Message edited by: M Easter ]

[Ernest Friedman-Hill]

I'm also in the "prefer one return, but multiple ones are OK if needed" camp. But actually, it's the kind of code you've just shown here that makes my skin crawl. I can't stand control flags. Variables that represent real decisions previously made are OK, but variables that exist merely to direct control flow bother me a great deal -- too much like assembly, I guess. I would be very quick to rewrite this code to simply return from the loop.

[Ilja Preuss]

Originally posted by M Easter:
A colleague of mine had a rule: in addition to a single return, also no break or continue statements. They cause bugs.

That's a very strong statement that I can't subscribe to.

What I'd agree with is that break and continue statements are a code smell, in the sense that they often hint at code that is doing more in one place than is good for its complexity, and that would benefit from the extraction of a method.

 
public void someMethod() {
    // Do some work here
    int i = 0;
    booolean ok = true;
 
    while( ok && i < X ) {
        if(...) { 
            ok = false;
        }
        i++;
    }
 
    if( ok ) {
        doSomething();
    }
}   

That's a perfect example! Take a look at this refactored version, which I consider to be even more elegant:

 
public void someMethod() {
    // Do some work here
    if( ok() ) {
        doSomething();
    }
}
 
private boolean ok() {
    for (int i = 0; i < X; i++) {
        if(...) { 
            return false;
        }
    }
    return true;
}   

Very often I actually find that a good next step is to move the ok method to a different class, because it smells of Feature Envy.

[Henry Wong]

Would this make your "skin crawl"?...

public void someMethod() {
    // Do some work here
    try {
        for (int i = 0; i < ...; i++) {
            if (...) {
                // NOTE: Return statement here!
                return ...;
            }
        }
    } finally {
        // Some extra code here that was added later
        // NOTE: This will get executed even if the method already returned in the loop above!
        doSomething();
    }
}    

Henry
[ May 02, 2007: Message edited by: Henry Wong ]

[Roseanne Zhang]

If you ever coded in functional language, such as ruby, you will not argue on such topic!!!

Multi-returns are fine to me!

[Roseanne Zhang]

Even you don't see a single return inside the following code, but there can be thousands equivalent. What a beautiful elegant concise code to all possible interpretations of a piece of Morse code.
http://pastie.caboo.se/55742

 
# Ruby Quiz 121
# Donald A. Ball Jr.
# Version 1.1
class String
  def split_at(index)
    [slice(0, index), slice(index, length)]
  end
end
 
class Morse
 
  CODES = {
    :A => '.-',
    :B => '-...',
    :C => '-.-.',
     :D  => '-..',
    :E => '.',
    :F => '..-.',
    :G => '--.',
    :H => '....',
    :I => '..',
    :J => '.---',
    :K => '-.-',
    :L => '.-..',
    :M => '--',
    :N => '-.',
     :o  => '---',
     :p  => '.--.',
    :Q => '--.-',
    :R => '.-.',
    :S => '...',
    :T => '-',
    :U => '..-',
    :V => '...-',
    :W => '.--',
    :X => '-..-',
    :Y => '-.--',
    :Z => '--..'
  }
 
  def self.decipher(cipher)
    CODES.each_pair do |letter, code|
      prefix, suffix = cipher.split_at(code.length)
      next unless prefix == code
      if suffix == ''
        yield letter.to_s
      else
        decipher(suffix) {|result| yield letter.to_s << result }
      end
    end
  end
 
end
 
Morse.decipher(ARGV[0]) {|word| puts word}

[ May 02, 2007: Message edited by: Roseanne Zhang ]

[Joe Wolfe]

It is a complex question without a simple answer. If the mutiple returns make the code hard to follow then use one. If it helps use several. This is my opinion.

With plenty of inexperienced ptrogrammers around an insistance on one return only helps keep the code easy to read. So I like that idea but multiple returns in one method by an experenced programmer is not grounds for dismissal. Forcing them to use one forces good programming pracitices on them so I vote one for the record. But allow two when one makes it too complicated.

Short methods help too. No 300 line methods please.

[Ilja Preuss]

Originally posted by Joe Wolfe:

With plenty of inexperienced ptrogrammers around an insistance on one return only helps keep the code easy to read.

How does it do that?

[Jesper de Jong]

Originally posted by Henry Wong:
Would this make your "skin crawl"?...

Yes, that would make my skin crawl, you're abusing the exception handling mechanism here!

This kind of coding could easily introduce another bug: if you throw an exception inside the try { ... } block that you want thrown out of the method, then the code in the finally { ... } block would be executed even if you don't want that.

So that doesn't look like a good solution.

[Jim Yingst]

Jesper: really? Using finally is abusing exception handling? I really can't agree with that. No exceptions needs to be thrown, and the path through the code is linear. The try/catch/finally blocks do add a bit of visual clutter which I dislike, but otherwise, I don't see a problem.

Well, there is one: if an exception is thrown for any reason from within the finally block, it causes the JVM to lose the stack trace of any previously-thrown exception. That gives a good motivation to try to use finally only for short, "safe" operations. Or to use another try/catch within the finally, which adds to visual clutter...

[Jesper]: This kind of coding could easily introduce another bug: if you throw an exception inside the try { ... } block that you want thrown out of the method, then the code in the finally { ... } block would be executed even if you don't want that.

Hm, I rather thought that was part of the point, that the programmer does want the code executed even if an exception was thrown. Certainly, if they don't, they shouldn't code this way. But if they do, they should.

As for the original question, my response is pretty much exactly the same as Stan's above. I'm very religious about keeping methods short. Many other problems are non-issues if you keep your methods short. And if a method is shorter with multiple returns than with a boolean control variable, that's usually more readable to me. Ilja's earlier example is exactly the sort of thing I would try to do, and the ok() method looks very readable to me.

The main reason I sometimes have to avoid multiple returns is when I want to do something like log a return value before returning. It's nice to only need to do that in one place. And you can't log a return value from a finally block, because there's no way to access the value of an earlier return. So the moment I want to log a return value, I'll refactor to a single return. But again, with short methods, this isn't much of a problem, and I don't see a need to preemptively require single returns everywhere just because I might want to log a return value later.

[Jeanne Boyarsky]

Originally posted by M Easter:
IMHO, once there is more than one, then the bubble bursts. ...
the "Broken Windows" theory. ... my concern is that once > 1 return occurs, others are likely to follow. Perhaps not by you or me but by that intern.

I subscribe to the "Broken Windows" theory, but I disagree with the applicability of the analogy here. The Broken Windows theory has to do with abandonment. If a policy says two return statements (but not more) are allowed, having two doesn't abandon things to get worse. There is still a policy which is addressed when someone decides to add a third. And "that intern" needs to be trained before coding. Otherwise he/she could do all sorts of things to the code base. For example, writing a 300 line method, writing string1 == string2 or writing generally buggy code. We reviewed all the code our summer intern wrote. It was very good (and I only did cursory glances at the end), but you don't want a bug to slip through due to inexperience.

(b) Maybe it's me but when I'm debugging or reading code and see more than one return, my brain feels a need to go into "parallel" mode. I know that multiple returns does not equate to multiple threads, but still....

It depends on how it is used. When the return statements are all over the place or in a long method, I agree. In a 10 line (or 5 line) method, this doesn't seem like a big issue.

(c) It just seems so much easier! One sets the default value for the result and then only change it as necessary.

Sometimes. I find the guard clause to be easier to read. I also find a linear search method to be simpler with two return statements - forgot to mention this one earlier.

[Ricky Clarkson]

There is no good reason to delay returns in Java. Resource cleanup should be done in finally blocks, so that reason is moot. Delaying returns obfuscates code.

If you find that you don't notice returns inside nested loops, etc., then you should read the code more slowly. Perhaps try a bigger font, or get your IDE to make the return keyword a strange colour.

If that fails, try to make the code more readable to yourself in some way. Many for loops with returns are searches - you might be better off writing a general search method that takes a predicate.

[Daniel Platon]

Hello all,
There's definitely a great conversation here. Up till now I never felt the need to more than one return statement. Event when I put two return statements, I fell I could've gotten around with just one.

What attracted me was the tiny piece of code

public void someMethod() {
    // Do some work here
    int i = 0;
    booolean ok = true;
 
    while( ok && i < X ) {
        if(...) { 
            ok = false;
        }
        i++;
    }
 
    if( ok ) {
        doSomething();
    }
}   

now suppose X is a very large number and you have some more work to do in an iteration. Wouldn't be a less burden to do something like this ? -

public void someMethod() {
    // Do some work here
    int i = 0;
    booolean ok = true;
 
    while( ok && i < X ) {
        //some bit of work
        if(...) { 
            ok = false;
            break;
        }
        i++;
    }
 
    if( ok ) {
        doSomething();
    }
}   

because ok could become true in the first 10 iterations out of a hundred for example, so the code executes 90 times for nothing.

Thanks a lot,
Dan

[Joanne Neal]

The while loop is checking the value of ok, so it will exit when it is set to false.

[Daniel Platon]

Ouuups ! Must be tired ! I was thinking about a "for" loop. The solution given by Ilja Preuss is actually ok, but why wouldn't one use "break" (hope i'm not too offtopic here)

Thanks,
Dan

[Ilja Preuss]

Originally posted by Daniel Platon:
why wouldn't one use "break"

I find the refactored version with the two methods to be easier to understand.

[Jim Yingst]

The other issue is that break is generally disliked by the same people who dislike multiple returns. (Except within switch statements, where break is an unfortunate necessity.) It you're working under a coding standard that forbids multiple returns, it probably also forbid break (outside a switch) and continue as well. At least, that's my experience.

Looking at Ilja's refactored ok() method:

private boolean ok() {
    for (int i = 0; i < X; i++) {
        if (...) { 
            return false;
        }
    }
    return true;
}   

This could've used a break instead:

private boolean ok() {
    boolean ok = true;
    for (int i = 0; i < X; i++) {
        if (...) { 
            ok = false;
            break;
        }
    }
    return ok;
}   

That has only one return, but requires two more lines than the original did, and to my mind it's a little less clear. However if I wanted to insert a log statement before the return, this is probably the form I'd use.

And yes, you can avoid the break by putting the ok inside the loop condition:

private boolean ok() {
    boolean ok = true;
    for (int i = 0; ok && i < X; i++) {
        if (...) { 
            ok = false;
        }
    }
    return ok;
}   

The problem there is that it's easy for people to miss that nonstandard loop condition, as Daniel just illustrated. I think that using either break or return jumps out at people more than that "ok &&" does. As usual, others may disagree.
[ May 04, 2007: Message edited by: Jim Yingst ]

[Jesper de Jong]

Jesper: really? Using finally is abusing exception handling? I really can't agree with that. No exceptions needs to be thrown, and the path through the code is linear. The try/catch/finally blocks do add a bit of visual clutter which I dislike, but otherwise, I don't see a problem.

No, ofcourse using 'finally' is not abusing exception handling. But using a try / finally block to avoid a bug caused by an early 'return' statement as I demonstrated in my post above is, in my opinion.

I'll try to explain it again. Suppose you have this:

public int someMethod(List<String> list) throws SomeException {
    int result = 0;
 
    for (String s : list) {
        if (doSomething(s)) 
            return 25;  // Early return!
    }
 
    return result;
}
 
public void doSomething(String s) throws SomeException {
    // ... Might throw SomeException
}

Now I'm going to add some code after the for-loop.

public int someMethod(List<String> list) throws SomeException {
    int result = 0;
 
    for (String s : list) {
        if (doSomething(s)) 
            return 25;  // Early return!
    }
 
    // Extra code here!
    list.add("Jesper");
 
    return result;
}
 
public void doSomething(String s) throws SomeException {
    // ... Might throw SomeException
}

Suppose that the programmer wanted to have that extra code executed before the method returns. In this case it goes wrong because there is an early return in the loop. When the method returns from the loop, the extra code is not executed. Ofcourse this is easy to fix, by removing the early return, like this:

public int someMethod(List<String> list) throws SomeException {
    int result = 0;
 
    for (String s : list) {
        if (doSomething(s)) {
            result = 25;  // No early return!
            break;
        }
    }
 
    // Extra code here!
    list.add("Jesper");
 
    return result;
}
 
public void doSomething(String s) throws SomeException {
    // ... Might throw SomeException
}

Note that if we would fix this with a try-finally block, the code in the finally would be executed even if doSomething(...) throws an exception, and that is not what we want.

public int someMethod(List<String> list) throws SomeException {
    int result = 0;
 
    try {
        for (String s : list) {
            if (doSomething(s)) {
                return 25;
            }
        }
    }
    finally {  // Hack to make sure that code is executed even when there's a return in the loop...
        // But: This code will also be executed when the method exits because doSomething() throws
        // an exception. That's not what you want.
 
        // Extra code here!
        list.add("Jesper");
    }
 
    return result;
}
 
public void doSomething(String s) throws SomeException {
    // ... Might throw SomeException
}

[Jim Yingst]

Jesper, your previous post was clear enough the first time. We're just looking at Henry's example code with different assumptions about what the intent was. As I said earlier:

Hm, I rather thought that was part of the point, that the programmer does want the code executed even if an exception was thrown. Certainly, if they don't, they shouldn't code this way. But if they do, they should.

Considering the original code called "doSomething()" in the finally block, we can't really tell whether the "something" is something that should also be done in the case of an exception, or not. For some situations it's a buggy solution, while for others it's the preferred solution. I certainly wouldn't say it "makes my skin crawl" in general, but it may be wrong for a more specific example.
[ May 05, 2007: Message edited by: Jim Yingst ]

[Nicholas Jordan]

Originally posted by Henry Wong:
Would this make your "skin crawl"?...

Oh, I love your code ! Makes mine crawl, but then I know to look for exactly the type of code that makes my skin crawl.

I originally started coding my project in C++, writing massive switches (where useful) on the assumption that some coder, someday, would try to do things to me like what your snippet does, along with probably several other " hidden gotchas " that were waiting, undiscovered .... making the decision early and clean that I would write whatever code it took, myself, or decline the business.

By now, I template any and all func()'s as int retVal = 0; near the beginning, then place the return retVal; near the closing brace, even going so far as to place an explict return on a void even if the linguistic does not require it.

Occasaionally, there are the unavoidable constraints, but the only solution I have found is to tediously walk through everything. Anything else is going to get you someone elses' malarky assigned to your reputation.

I have been through this so many times, I now laugh at people when I see it coming.

[edit: Sunday, May 20, 2007]:
I have given this additional thought, I always template a func() thus:

int func(){
 if(){
     if(){
        if(){   

With matching elses and do cleanup before vars go out of scope. I am giving consideration at exactly this moment whether to leave by exception in all elses so that the return is only done on successful func() completion.
[ May 20, 2007: Message edited by: Nicholas Jordan ]

[Yegor Bugayenko]

In a pure object-oriented programming, there should be only one exit point from each method, here is why: http://www.yegor256.com/2015/08/18/multiple-return-statements-in-oop.html

[Stephan van Hulst]

I think the article has very poor arguments.

It's based on the fact that "pure OO" (whatever that is) is clean, readable and somehow perfect. However, the example given is less clear than the original code, and it looks like a poor way of functional programming.

public int max(int a, int b) {
  return new If(
    new GreaterThan(a, b),
    a, b
  );
}

Even if this code was better than the original (what does it actually return, an int or an If?), the article argues that since Java doesn't support this, we should use something that looks like it. Where is the logic in that? Is imitating something poorly a better option than the clear and concise piece of code that is not "pure"?

Stick to code that is readable in Java. If you want to use a functional programming style with only one return statement, use Java 8 Streams, or switch to a functional language.

[Campbell Ritchie]

Although I don't like multiple returns myself, I agree that code with return new If… is particularly bad and illegible.
I think a max program is a function and the functional way to write it is this:-

public static final int max(int i, int j)
{
   return i > j ? i : j ;
}

You would write virtually the same in C, so the procedural solution is exactly the same.

[edit]Nobody noticed the missing return type (int) which I have corrected?[/edit]

[Stephan van Hulst]

Campbell Ritchie wrote:[edit]Nobody noticed the missing return type (int) which I have corrected?[/edit]

Hah, I did, but I wasn't in a pedantic mood :P

I was more surprised with why the method was both final and static, since static methods are final by definition.

[Mike. J. Thompson]

Stephan van Hulst wrote:I was more surprised with why the method was both final and static, since static methods are final by definition.

Apparently that isn't true. I just entered the following two classes in Eclipse:

public class Class1
{
    public static final void method()
    {
    }
}

public class Class2 extends Class1
{
    public static void method()
    {

    }
}

And got the compiler error: "Cannot override the final method from Class1". Removing the final modifier resolved the compiler error.

From the Java Language Specification 8 - Section 8.4.3.3

It is a compile-time error to attempt to override or hide a final method.

That was news to me too.

[Stephan van Hulst]

Cool! Have a cow.

[Mike. J. Thompson]

Thanks for the cow!