Which one is a better style?

[Vishal Saxena]

Style1
private PaymentSchedule getPaymentSchedule()
{
//...blah blah...
if(<some condition>
{
//..blah..blah
return paymentSchedule.prorata(partialPaymentAmount);
}
else if(<some other condition>
{
// ..blah..blah
return new PaymentSchedule(partialPaymentAmount, principalPayment, interestPayment,
bankInterestAccruedSinceLastPayment, dealerInterestAccruedSinceLastPayment);
}

//if the program flow reaches here; it's a bug.
return null }
}
Style2
private PaymentSchedule getPaymentSchedule()
{
PaymentSchedule paymentSchedule = null;
if(<some condition>
{
//..blah..blah
paymentSchedule = paymentSchedule.prorata(partialPaymentAmount);
}
else if(<someothercondition>
{
// ..blah..blah
paymentSchedule = new PaymentSchedule(partialPaymentAmount, principalPayment, interestPayment,
bankInterestAccruedSinceLastPayment, dealerInterestAccruedSinceLastPayment);
}

return paymentSchedule }
}

[M Beck]

it's a toss-up. i tend to use a mixture of the two, myself; i often toss in a final "else" at the bottom to handle any bugs that may crop up somehow, too. the second style is a bit clearer, since there's only one "return" to worry about, but the first one can sometimes be more concise when the thing to be returned is just a primitive and not a complex object. whichever makes your code easier to read and understand is likely better.

[Stan James]

There are many good arguments for setting a single value and having a single return statement at the end. It's a bit more work but makes it easier to make certain changes or apply refactorings. If you find that this makes the method too long or clumsy it's probably a sign to shorten the method, perhaps by extracting the blocks inside each condition to their own methods.

Multiple returns are easier and I sometimes code that way, but I consider it lazy and almost always go back and fix up a single return after the code passes tests.

[David Harkness]

I tend to prefer the early return method, but I can see the reasons for the single return. I often have to deal with other people's code, and nine times out of ten that means several-page methods. The problem with a single return is that you have to read the entire method to make sure no other code is modifying the value before the return at the bottom.

With an early return, when you see "return new Foo(...)" you don't have to ask yourself, "Gee, I wonder if that Foo is modified before being returned" because it can't be modified. (Hmm, what happens if that's inside a try block with a finally block that returns something else. I know the JLS covers it so I guess I'll deal with that if I ever see it ... *shudder*!)

As well, when you get down to really tiny methods, which can you grok more quickly?

public int find ( int [] a , int x ) {
    int found = -1;
    for ( i = 0 ; found == -1 && i < a.length ; i++ ) {
        if ( a[i] == x ) {
            found = i;
        }
    }
    return found;
}

or this

public int find ( int [] a , int x ) {
    for ( i = 0 ; i < a.length ; i++ ) {
        if ( a[i] == x ) {
            return i;
        }
    }
    return -1;
}

Why the wonderful variable names? That's what I get to deal with! (I know, "Boo hoo, poor Dave" ) But why the "found == -1 && i < a.length". Well, I figure if you're against early returns, you're probably against break as well.

[ Added this: ]

Since they're so similar, I should probably point out why I believe the second form is much clearer. While looking over the first form, my first question upon seeing "return found" is "What value will found have at this point?" To answer that, I have to understand the entire method. In the second form, when I see "return -1" I immediately understand that if the method makes it through the loop, -1 is returned. In the first form, found might be changed inside the loop, so it's unclear under what circumstances -1 (the initial value) is returned.

Next, in the second form it's very clear that as soon as a value is found, the index is returned. In the first form, you need to catch the "found == -1" in the loop condition to see that the first index is returned. If you miss that, you'll think that index of the last occurrence of x is returned.

For me it comes down to pattern matching, the thing our brains were designed to do. I instantly match the entire for-loop pattern of the second form because it doesn't deviate from the norm. The first form as a different condition, and that instantly kicks me out of pattern matching as I have to analyze exactly what's going on.
[ April 26, 2005: Message edited by: David Harkness ]

[M Beck]

Originally posted by David Harkness:

public int find ( int [] a , int x ) {
    for ( i = 0 ; i < a.length ; i++ ) {
        if ( a[i] == x ) {
            return i;
        }
    }
    return -1;
}

Why the wonderful variable names? That's what I get to deal with! (I know, "Boo hoo, poor Dave" )

eh, what's so odd about those? you've got an array named "a", a thing to go looking for named "x", and an iteration index named "i". clear as day!

...or have i just been reading too much MIT-style Scheme code of late, and become inured? it has made me wish there was a convenient, succinct way to do first-class functions in Java... again...

[Stan James]

After I argued for a single return at the end I went and wrote an extra return at the top of a method. Maybe I'm evolving a position for immediate return on argument validation. What I put as the first statement of a String formatting method was:

if ( argument == null )
   return null;

cause it's not worth doing the rest of the method.

[David Harkness]

That's a great example. Another reason I like early returns (and exceptions for that matter) is that it decreases the amount of context you need to keep in mind while reading a method. You humans can really only hold about seven things in your head at any one time, and each level of nesting is another thing to keep in mind.

String result = null;
if ( arg != null ) {
   ...
}
 
return result;

Inside the if block you already have two pieces of context in your mind: the initial value of the result is null and the argument isn't null. In the same early return method

if ( arg == null )
    return null;
 
...

at the ... (the real work of the method) you really have no context to keep in mind. For the most part you can forget that the argument isn't null because that case is handled by exiting the method -- it won't pop up at the end of an if block, so you can mostly forget it.

As for the argument that early returns make proving code and refactoring more difficult, I guess I can only say that the tools are pretty simplistic today. As they mature, cases like this should be very simple to handle. And since I can count the number of times I've used a code-proving tool without any fingers, I can confidently say that it hasn't affected me or anyone I know yet.

[Ilja Preuss]

Originally posted by Stan James:
After I argued for a single return at the end I went and wrote an extra return at the top of a method. Maybe I'm evolving a position for immediate return on argument validation. What I put as the first statement of a String formatting method was:

if ( argument == null )
   return null;

cause it's not worth doing the rest of the method.

That's called a Guard Clause - and I really like it.

I regularly refactor code with one return to use guard clauses instead, because I find such code much easier to understand.

Now and then I need to refactor to a single return statement to enable an automated extract method refactoring, but I think it's still worth the effort - worth enough that typically after the extraction, I will again refactor to guard clauses, if still appropriate.

[Warren Dew]

Stan James:

There are many good arguments for setting a single value and having a single return statement at the end. It's a bit more work but makes it easier to make certain changes or apply refactorings.

My experience is just the opposite. I'm doing a port project right now, and my experience is that retaining a return value - or any parameter - for the entire method makes refactoring quite difficult, because I can't extract a portion of the code without having to take into account interactions with the entire rest of the function. That can be a real pain when dealing with 400 line functions.

You could argue that those functions are too long, and I would agree. However, I think part of the way they got that way is that it was too tempting to overmanipulate the return value variable when making additions to the code. Without the variable, there's no temptation to include it in the logic; as a result, the added code isn't as entangled with the existing function and can more easily be added in the form of a new function call.

[miguel lisboa]

That's called a Guard Clause - and I really like it.

someone in these forums once told me: i wanna leave the method as soon as i can

i guess his idea was something like this:

 void onlyfForHumans(Living x) {
if (x.isAlien) {
return;
}
// human code here
} 

[Henry Wong]

I have never been a fan of the "one return" statement rule. I just don't see the benefit. IMO, if it can cause that much confusion, then maybe the method is too long or complex -- and some refactoring may be in order.

Henry

[Ilja Preuss]

Originally posted by Henry Wong:
I have never been a fan of the "one return" statement rule. I just don't see the benefit.

It is often important to academics, because it makes mathematical proves about the code much simpler.