Otaku, Cedric's weblog

Exactly the same with me, I prefer single returns in methods but for some time now also use returns for early aborts. Both is consistent with higher readability of source code.

I like to take a pragmatic approach to this question.
Use the style that makes the execution of the operation more readable and understandable.

For example I changed the style, I implement equals()-operations. I used to define a boolean value 'equal = false' at the beginning an return the modified value at the end, but I think it's better understandable to leave the operation as soon as you can say, that the objects are not equal.

I also a big fan of single return functions and I do not give up. In order to combine a single return with early aborting I use the the following trick:

do {

    if (b == null) { // abort
      break;
    }

     ... // do something useful

} while (false);

... // cleanup

Go guard clauses!

I was a big fan of single return a while back... back in the days of no "finally" clause (that was before java).

But with the "finally" clause, I do not care very much for single return. "finally" allow you to have a kind of single return. Sometimes, you want to do some common clean-up whatever the control flow in the method. Then you can use mulptiple returns and have the clean-up code inside the finally clause as it will be executed whenever you return normally or not.

However, I prefer early returns than returns in a middle of block inside of a block inside of a block inside of a ...

It all comes down to code readability and common sense. Sometimes, it's easier to understand a method with a single return, sometime it's better with multiple early returns.

Cedric, for the most part I agree with you. One thing I also like, and always do, is name the 'result' variable "result" like you do in your examples. That way you can easily tell when the function's result is being altered.

I started doing this when I quit writing multi return point functions. When I see:
    result = someVal;
in the function code I can, with some care, mentally replace that with:
    return someVal;
I find this idiom helps combine the advantages of 'multi return point' and 'single return point' functions.

Of course, this only works if the function is returning a value.

I use to be a big single return nazi as well. Originally I adopted this style due to the immense amount of multi-threaded C code I was writing in my OS/2 days. I needed to make sure that I was properly releasing semaphores and whatnot and the only way to do it safely was with single returns. I adopted the same practice for Java for the sake of readability (I don't do as much multi-threaded stuff in Java) but quickly found that it isn't always very readible. I much prefer the short-circuit method. So far I haven't found it to be a problem.

I agree all too much with this.

This is why I am almost always against any 'always' or 'never' rule when the consequence of not following the rule is simply style - sure, always/never rules sometimes have to exist; but in most cases it is a sign that someone hasn't seen a particular scenario enough. If you don't mind, I think I'll begin using the term 'early aborts' as I express my distaste for 'single return nazism' as Stock called it ;).

Well, I'm a fan of multiple return points (multiple =2 or rarely 3). I found it reduces identation :). Leaving joke aside, I find it very easy to follow when you put down your logic as "take down the easy cases first". To find an analogy from game playing, it's like clearing out all corners/small rooms before walking through the main door.

I've seen both approaches taken to the extreme, and I still find multiple (in the ranges of tens!!) exit points easier to follow than if-else...

Also with multiple return points, you usually see what happens in front of your eyes (no need to chase the end of the else if else chain to see if there is extra code involved). I found this also makes it easier to refactor (e.g. extract the code inside the if as a method).You can extract it anyway, but there are better chances the entire code responsible for a certain operation will end up in that method.

However, I agree with both forms as long as the method is short. It's when having a 500+ lines method when you really start to question readability...

I too much prefer guard clauses that cleanly abort early rather than leaving the reader dangling. They instantly explain what conditions need to be met for the method to do useful work.

There is another case where I tend to prefer multiple returns (altough I had a bigger struggle to let go of the single-return on that one): when a simple method has to partially iterate over a collection to make a decision. For example:

public boolean containsEmtpy(List<String> names) {
for (String name : names) {
if (name.length() == 0) {
return true;
}
}
return false;
}

IMO this construct conveys the idea in a much clearer way rather than the convolution of adding in a 'result' temp, especially with the java 5 for loop.

Hi Cedric,

Agree with you whole heartedly. On methods that I modify, I do a javadoc WARNING that "this method has multiple return points" when I don't want(bugs, time, etc) to refactor the code.

We need an intelligent commenting tool that will look at code and insert an automatic javadoc commenting to this effect so people reading it are fore-warned and kinda gives us a TODOLIST for refactoring in the next release.

Note : Eclipse has a way to find out the multiple return points of a method (ALT + SHIFT + O), if you're visiting new code.

BR,
~A

One of the problem that the maxime 'single return' is trying to solve is to minimize the number of possible control flows through a method.
Considering, however, that any exception is a possible exit to a method, this maxime does not seem very helpful. To my tastes, forcing 'single return' often decreases readability (which is very much a matter of taste) and does not really decrease the control flow compexity.
I find short methods, together with finally, much more helpful in that situation.

Early aborts/guard clauses are a great way of specifying preconditions, ala Design By Contract.

In a true DBC system, these guard clauses will be executed for you prior to the method call. Lacking DBC, having guard clauses give you a similar benefit: you know that the main body of the code won't have to worry about bad callers, and the reader of the code is told very clearly what is accepted in the way of incoming parameters (as opposed to having to dig it out of the method, or to find it in the docs)

Mikhail is close. Yes, you can have your shortcut and single exit too. The answer however is not a do-while hack but rather the seldom used labeled block. So rewriting Mikhail example to use a labeled block called “error_check_block” yields:

error_check_block: {
if (null == b) { // abort
break error_check_block;
}
... // do something useful
}
... // cleanup
...// add a trace statement that the method is about to exit

The labeled break statement approach seems more complicated than putting the cleanup and trace statement in a finally block. The only reason I can think of for doing it that way is if you want to log the value being returned.

Easy reading is king. To be forced having a single point of return is not always easy. Many come with deep nested ifs wich a much harder to undestand. Human brain stack size is small

return name.length() == 0;
for one of the above examples seems best. If one needs to be clearer

boolean flag = name.length() == 0;
return flag;

I agree with the comment by Robert Watkins: once you are a user of somebody else's API you learn eventually to code in a very defensive way. Early aborts are one of the techniques I prefer, since the rest of the code has less nasty things to worry about.

(Ah, and you may want to improve your comment filter, as it rejects my last name because 4 out of 5 letters correspond to a product to prevent "grossesses").

Dear Sir,

I would like to renew my request of you to send me some early aborts for doing my final project.

My last request was not honored. This cause a missing deadline and now I have to do my final project for new.

Send it to me!

catbert.L

;-)

Reminds me of an egineering lead who worked with a friend few years ago. He was a big fan of the single-exit style. But he took it to extremes. If someone wrote:

if (b) {
return x;
} else {
return y;
}

he would ask it to be changed to:
return (b? x : y);

even when b, x and y are complex expressions.

In the prior case, I generally say

Object result;
if (b) {
result = x;
} else {
result = y;
}
return result;

I'm not a single-return Nazi. However, the company coding standards (which most employees disregard) call for a single return, and the resulting code is more readable.

I like the human stack hypothesis mentioned by Martin Möbius. It should be possible to figure this out using an experiment. If the hypothesis is correct, you could measure it: a deeper stack means a longer time to solution of the problem. For maximum realism, the problem should be 'find the bug in the following function/method'.

I prefer the multiple return, early abort approach, especially since it calls more attention to the worst condition that can be returned rather than lumping it in with the rest.

In my work, the code more prevalent is the second one (with single return point functions), which I think is the better choice. Because the first one increases your cyclomatic complexity. The cyclomatic complexity (a McCabe metric) increases with the number of executon routes. In the first example, the cyclomatic complexity is 4. In the second one (with single return point functions), this number is 2. So I think the second exemple is the better choice, moreover its legibility and flow of execution is better than the fist example.

I vote for the "early abort idiom". So, you avoid to pollute tests that will consider b as a "legal" value with an initial test just to reject an illegal value.
I also totally agree with Martin Möbius on "Easy reading" and the increased indentation that comes from single return point.
I let imagine everyone how difficult it can be to pair-programm with a "single return point" fan when you are a "early abort" supporter ;-) Believe me, it is sometimes hard !

I don't mind multiple exit points. We gave up single exit points with exceptions. Granted exceptions serve a different role than flow control, but what makes you not do this:

public int foo(String bar) {
RuntimeException e = null;
int rv = ...;
if (bar == null)
e = new NullPointerException();
else
...

if (e != null) throw e;
return rv;
}

instead of this

public int foo(String bar) {
if (bar == null)
throw new NullPointerException();
int rv = ...;
...
return rv;
}

OK this is a bit of an absurd example, but there's a reason why you throw exceptions early: it's cleaner looking and documents exceptional conditions up front.

For methods which have more complex cases than simple parameter validation, it's, of course, less clear. If you're method gets to be too large to follow, you can always refactor or document.

An aside: Rafael suggests that cyclomatic complexity is a factor in his decisions. I know just a little about cyclomatic complexity - what is the difference between multiple exit points and multiple conditional branches within the method itself? I'll probably ask him directly.

I don't mind multiple exit points. We gave up single exit points with exceptions. Granted exceptions serve a different role than flow control, but what makes you not do this:

public int foo(String bar) {
RuntimeException e = null;
int rv = ...;
if (bar == null)
e = new NullPointerException();
else
...

if (e != null) throw e;
return rv;
}

instead of this

public int foo(String bar) {
if (bar == null)
throw new NullPointerException();
int rv = ...;
...
return rv;
}

OK this is a bit of an absurd example, but there's a reason why you throw exceptions early: it's cleaner looking and documents exceptional conditions up front.

For methods which have more complex cases than simple parameter validation, it's, of course, less clear. If you're method gets to be too large to follow, you can always refactor or document.

An aside: Rafael suggests that cyclomatic complexity is a factor in his decisions. I know just a little about cyclomatic complexity - what is the difference between multiple exit points and multiple conditional branches within the method itself? I'll probably ask him directly.

I use early aborts to avoid indention and to better document the exceptions the function has.

Also it is a good coding guideline to early abort with explicite returns because inexperienced programmers are more forced to see the return code, especially if catches are involved.

Bernd

BTW:

if (a==5)
{
if (b==6)
{
rc = 7; break out;
}
if (b==7)
{
rc = 1; break out;
}
}

out: return rc;

PS: http://www.javaspec ialists.co.za/archive/Issue110.html

(need to break the url because of stupid keyword match on "c|al|s"

Greetings
Bernd
--
http://bernd.eckenfes.net

I gave up striving towards single exit points decades ago - after years of struggling with that dogma - for many of the same reasons other folks have cited in the thread above.

However, one thing I often notice in others' "exit in the middle" code - which I have NEVER understood - goes something like:

if (something) {
return 1;
} else {
return 2;
}

Why waste the time writing (and reading) the "else" clause, when the "if" clause will never flow through?

I find:

if (something) {
return 1;
}
return 2;

to be the only reasonable approach - much clearer.

thank