views:

13793

answers:

46

Are there good reasons why it's a better practice to have only one return statement in a function?

Or is it okay to return from a function as soon as it is logically correct to do so, meaning there may be many return statements in the function?

+5  A: 

I would say you should have as many as required, or any that make the code cleaner (such as guard clauses).

I have personally never heard/seen any "best practices" say that you should have only one return statement.

For the most part, I tend to exit a function as soon as possible based on a logic path (guard clauses are an excellent example of this).

Rob Cooper
+3  A: 

One good reason I can think of is for code maintenance: you have a single point of exit. If you want to change the format of the result,..., it's just much simpler to implement. Also, for debugging, you can just stick a breakpoint there :)

Having said that, I once had to work in a library where the coding standards imposed 'one return statement per function', and I found it pretty tough. I write lots of numerical computations code, and there often are 'special cases', so the code ended up being quite hard to follow...

OysterD
+327  A: 

I often have several statements at the start of a method to return for "easy" situations. For example, this:

public void DoStuff(Foo foo)
{
    if (foo != null)
    {
        ...
    }
}

... can be made more readable (IMHO) like this:

public void DoStuff(Foo foo)
{
    if (foo == null) return;

    ...
}

So yes, I think it's fine to have multiple "exit points" from a function/method.

Matt Hamilton
Good argument. This in a way would promote a style of contract, asserting and testing conditions upfront.
Ralph Rickenbach
Agreed. Although having multiple exit point can get out of hand, I definately think it's better than putting your entire function in an IF block. Use return as often as it makes sense to keep your code readable.
Joshua Carmody
This is known as a "guard statment" is Fowler's Refactoring.
Lars Westergren
Having a set of check/returns at the beginning of the function or clustered into a single section is a reasonable use of multiple return statements. In general, the problem is when a developer scatters return statements throughout the function inside of nested if-else's and loops.
James Schek
In the cases where I have placed a return in a deep nesting (ug!) I try to comment it very loudly in code so the next maintainer sees what I did.
Jason Jackson
When functions are kept relatively short, it is not difficult to follow the structure of a function with a return point near the middle.
KJAWolf
I find this form to be much more readable as compared to having } } return; } at the end of the function.Takes its toll on compilers too, every level of nesting costs time and complexity to compile. Used to be that compilers only supported 8. I've seen code that made me wish that was still true.
davenpcj
I am all for pre- and post-conditions, but that's where the buck stops for me. The worst of it is when you come across a huge block of if-else statements each with a return. This kind of code is a nightmare to both change and test.
Rick Minerich
Huge block of if-else statements, each with a return? That is nothing. Such a thing is usually easily refactored. (atleast the more common single exit variation with a result variable is, so I doubt the multi exit variation would be any more difficult.)If you want a real headache, look at a combination of if-else and while loops (controlled by local booleans), where result variables are set, leading to a final exit point at the end of the method. That is the single exit idea gone mad, and yes I am talking from actual experience having had to deal with it.
Marcus Andrén
Imagine: you need to do "IncreaseStuffCallCounter" method at the end of 'DoStuff' function.What will you do in this case? :)Have unique return and enjoy. If you method is too complicated to have only one return - split it.
Budda
@Budda Sure, you could keep a "result" variable 'til the end, or you could wrap the whole thing in a try/finally and call DoStuff in the finally. Then you can return multiple times throughout the method. I don't advocate this way but it'd work.
Matt Hamilton
+38  A: 

Structured programming says you should only ever have one return statement per function. This is to limit the complexity. Many people such as Martin Fowler argue that it is simpler to write functions with multiple return statements. He presents this argument in the classic refactoring book he wrote. This works well if you follow his other advice and write small functions. I agree with this point of view and only strict structured programming purists adhere to single return statements per function.

cdv
Structured programming doesn't say a thing. Some (but not all) people who describe themselves as advocates of structured programming say that.
wnoise
"This works well if you follow his other advice and write small functions."This is the most important point. Small functions rarely need many return points.
tdyen
@wnoise +1 for the comment, so true. All "structude programming" says is don't use GOTO.
ceretullis
@wnoise +1 for a vital observation
Galghamon
@ceretullis: unless it's necesary. Of course it's not essential, but can be usefull in C. The linux kernel uses it, and for good reasons. *GOTO considered Harmful* talked about using `GOTO` to move control flow even when function existed. It never says "never use `GOTO` ".
voyager
Hmm ..., didn't one of the structured programming foremost shamans used to critique object oriented programming as an unnecessary mess, all of whose features could be "easily" implemented with structured programming? Did he not also later eat his words concerning that opinion?Time for another structured programming cud to be spewed out and rechewed again.
Blessed Geek
+3  A: 

I've worked with terrible coding standards that forced a single exit path on you and the result is nearly always unstructured spaghetti if the function is anything but trivial -- you end up with lots of breaks and continues that just get in the way.

Phil Bennett
+112  A: 

I would say it would be incredibly unwise to decide arbitrarily against multiple exit points as I have found the technique to be useful in practice over and over again, in fact I have often refactored existing code to multiple exit points for clarity. We can compare the two approaches thus:-

void string fooBar(string s, int? i) {
  string ret;
  if(!string.IsNullOrEmpty(s) && i != null) {
    var res = someFunction(s, i);

    bool passed = true;
    foreach(var r in res) {
      if(!r.Passed) {
        passed = false;
        break;
      }
    }

    if(passed) {
      // Rest of code...
    }
  }

  return ret;
}

Compare this to the code where multiple exit points are permitted:-

void string fooBar(string s, int? i) {

  if(string.IsNullOrEmpty(s) || i == null) return null;

  var res = someFunction(s, i);

  foreach(var r in res) {
      if(!r.Passed) return null;
  }

  // Rest of code...

  return ret;
}

I think the latter is considerably clearer. As far as I can tell the criticism of multiple exit points is a rather archiac point of view these days.

kronoz
Clarity is in the eye of the beholder - I look at a function and seek a beginning a middle and an end. When the function is small its fine - but when you're trying to work out why something is broken and "Rest of Code" turns out to be non-trivial you can spend ages looking for why ret is
Murph
Even in your first example code you've got an early return (for the foreach loop). Of course the design of your iterator may not be helping.
Tom Hawtin - tackline
@Murph - You'd realise rest of code didn't run as the input didn't conform to requirements...@Tom - Um how is that an 'early return'?!...
kronoz
@kronoz, break/continue are pretty much the same thing as an early return from an extracted loop method.
Dustin Getz
First of all, this is contrived example. Second, Where does :string ret;" go in the second version? Third, Ret contains no useful information.Fourth, why so much logic in one function/method?Fifth, why not make DidValuesPass( type res ) then RestOfCode() separate subfunctions?
Rick Minerich
@Rick 1. Not contrived in my experience, this is actually a pattern I've come across many times, 2. It's assigned in 'rest of code', maybe that's not clear. 3. Um? It's an example? 4. Well I guess it's contrived in this respect, but it's possible to justify this much, 5. could do...
kronoz
@Rick the point is that it's often easier to return early than to wrap code up in a huge if statement. It comes up a lot in my experience, even with proper refactoring.
kronoz
+19  A: 

I've seen it in coding standards for C++ that were a hang-over from C, as if you don't have RAII or other automatic memory management then you have to clean up for each return, which either means cut-and-paste of the clean-up or a goto (logically the same as 'finally' in managed languages), both of which are considered bad form. If your practices are to use smart pointers and collections in C++ or another automatic memory system, then there isn't a strong reason for it, and it become all about readability, and more of a judgement call.

Pete Kirkham
I Could not have written it better. +1.
paercebal
Well said, though I do believe that it is better to copy the deletes when trying to write highly optimized code (such as software skinning complex 3d meshes!)
Grant Peters
What causes you to believe that? If you have a compiler with poor optimisation where there is some overhead in dereferencing the `auto_ptr`, you can use plain pointers in parallel. Though it would be odd to be writing 'optimised' code with a non-optimising compiler in the first place.
Pete Kirkham
+27  A: 

As Kent Beck notes when discussing guard clauses in Implementation Patterns making a routine have a single entry and exit point ...

"was to prevent the confusion possible when jumping into and out of many locations in the same routine. It made good sense when applied to FORTRAN or assembly language programs written with lots of global data where even understanding which statements were executed was hard work ... with small methods and mostly local data, it is needlessly conservative."

I find a function written with guard clauses much easier to follow than one long nested bunch of if then else statements.

Bedwyr Humphreys
+8  A: 

In general I try to have only a single exit point from a function. There are times, however, that doing so actually ends up creating a more complex function body than is necessary, in which case it's better to have multiple exit points. It really has to be a "judgement call" based on the resulting complexity, but the goal should be as few exit points as possible without sacrificing complexity and understanability.

Scott Dorman
+83  A: 

I currently am working on a codebase where two of the people working on it blindly subscribe to the "single point of exit" theory and I can tell you that from experience, it's a horrible horrible practice. It makes code extremely difficult to maintain and I'll show you why.

With the "single point of exit" theory, you inevitably wind up with code that looks like this:

function()
{
    HRESULT error = S_OK;

    if(SUCCEEDED(Operation1()))
    {
        if(SUCCEEDED(Operation2()))
        {
            if(SUCCEEDED(Operation3()))
            {
                if(SUCCEEDED(Operation4()))
                {
                }
                else
                {
                    error = OPERATION4FAILED;
                }
            }
            else
            {
                error = OPERATION3FAILED;
            }
        }
        else
        {
            error = OPERATION2FAILED;
        }
    }
    else
    {
        error = OPERATION1FAILED;
    }

    return error;
}

Not only does this make the code very hard to follow, but now say later on you need to go back and add an operation in between 1 and 2. You have to indent just about the entire freaking function, and good luck making sure all of your if/else conditions and braces are matched up properly.

This method makes code maintenance extremely difficult and error prone.

17 of 26
Or you could just have a list of operations, returning the first error from applying each operation in the list in sequence.
Apocalisp
Luckily a good editor (in my case, I know VIM can do it) will reformat all your code nicely at the touch of a button.
Nathan Fellman
Erm, I disagree about hard to follow, its actually very easy to follow because there is a logic flow. Maintenance? Sir needs a better IDE.
Murph
@Murph: You have no way of knowing that nothing else occurs after each condition without reading through each one. Normally I would say these kinds of topics are subjective, but this is just clearly wrong. With a return on each error, you are done, you know exactly what happened.
Geoffrey Chetwood
@Murph: I've seen this kind of code used, abused and overused. The example is quite simple, as there is no true if/else inside. All that this kind of code needs to break down is one "else" forgotten. AFAIK, this code is in real bad need of exceptions.
paercebal
Good example of abusing good ideas by dogmatizing them! Generally, a single return poing is a good idea and using GOTO is not. Now, one of the simple ways to make this code have a single return point AND be maintainable is exactly to use GOTO (or exceptions). Isn't that ironic?
Yarik
You can refactor it into this, keeps its "purity":if(!SUCCEEDED(Operation1())){}elseerror = OPERATION1FAILED;if(error!=S_OK){if(SUCCEEDED(Operation2())){}elseerror = OPERATION2FAILED;}if(error!=S_OK){if(SUCCEEDED(Operation3())){}elseerror = OPERATION3FAILED;}//etc.
Joe Pineda
This code does not have just one exit point: each of those "error =" statements is on an exit path. It's not just about exiting functions, it's about exiting any block or sequence.
Jay Bazuzi
That's a complexity metric. Creates more edges (paths between blocks of code)
davenpcj
Without objects or method calls for separation, this is exactly what the state of any program ends up looking like. The problem here isn't the single exit point, it's the huge stack of layered if statements without any kind of other flow control.
Rick Minerich
i think this is badly laid out code rather than a bad principal. Joe Pineda's suggestion allows a single exit point and better structure.
Jon Hopkins
I disagree that single return "inevitably" results in deep nesting. Your example can be written as a simple, linear function with a single return (without gotos). And if you don't or can't rely exclusively on RAII for resource management, then early returns end up causing leaks or duplicate code. Most importantly, early returns make it impractical to assert post-conditions.
Adrian McCarthy
Lots of editors support Ctrl+] or Ctrl+[ for multi-line indentations. For others, it's tab and shift+tab.
Wallacoloo
+3  A: 

Multiple exit points are fine for small enough functions -- that is, a function that can be viewed on one screen length on its entirety. If a lengthy function likewise includes multiple exit points, it's a sign that the function can be chopped up further.

That said I avoid multiple-exit functions unless absolutely necessary. I have felt pain of bugs that are due to some stray return in some obscure line in more complex functions.

Jon Limjap
+7  A: 

There are good things to say about having a single exit-point, just as there are bad things to say about the inevitable "arrow" programming that results.

If using multiple exit points during input validation or resource allocation, I try to put all the 'error-exits' very visibly at the top of the function.

Both the Spartan Programming article of the "SSDSLPedia" and the single function exit point article of the "Portland Pattern Repository's Wiki" have some insightful arguments around this. Also, of course, there is this post to consider.

If you really want a single exit-point (in any non-exception-enabled language) for example in order to release resources in one single place, I find the careful application of goto to be good; see for example this rather contrived example (compressed to save screen real-estate):

int f(int y) {
    int value = -1;
    void *data = NULL;

    if (y < 0)
        goto clean;

    if ((data = malloc(123)) == NULL)
        goto clean;

    /* More code */

    value = 1;
clean:
   free(data);
   return value;
}

Personally I, in general, dislike arrow programming more than I dislike multiple exit-points, although both are useful when applied correctly. The best, of course, is to structure your program to require neither. Breaking down your function into multiple chunks usually help :)

Although when doing so, I find I end up with multiple exit points anyway as in this example, where some larger function has been broken down into several smaller functions:

int g(int y) {
  value = 0;

  if ((value = g0(y, value)) == -1)
    return -1;

  if ((value = g1(y, value)) == -1)
    return -1;

  return g2(y, value);
}

Depending on the project or coding guidelines, most of the boiler-plate code could be replaced by macros. As a side note, breaking it down this way makes the functions g0, g1 ,g2 very easy to test individually.

Obviously, in an OO and exception-enabled language, I wouldn't use if-statements like that (or at all, if I could get away with it with little enough effort), and the code would be much more plain. And non-arrowy. And most of the non-final returns would probably be exceptions.

In short;

  • Few returns are better than many returns
  • More than one return is better than huge arrows, and guard clauses are generally ok.
  • Exceptions could/should probably replace most 'guard clauses' when possible.
Henrik Gustafsson
+1 The best answer by far
Trap
The example crashes for y < 0, because it tries to free the NULL pointer ;-)
ammoQ
http://www.opengroup.org/onlinepubs/009695399/functions/free.html"If ptr is a null pointer, no action shall occur."
Henrik Gustafsson
No it will not crash, because passing NULL to free is a defined no-op. It is an annoyingly common misconception that you have to test for NULL first.
hlovdal
+4  A: 

Having a single exit point reduces Cyclomatic Complexity and therefore, in theory, reduces the probability that you will introduce bugs into your code when you change it. Practice however, tends to suggest that a more pragmatic approach is needed. I therefore tend to aim to have a single exit point, but allow my code to have several if that is more readable.

John Channing
Very insightful. Although, I feel that until a programmer knows when to use multiple exit points, they should be constrained to one.
Rick Minerich
Not really. The cyclomatic complexity of "if (...) return; ... return;" is the same as "if (...) {...} return;". They both have two paths through them.
Steve Emmerson
A: 

I'm usually in favor of multiple return statements. They are easiest to read.

There are situations where it isn't good. Sometimes returning from a function can be very complicated. I recall one case where all functions had to link into multiple different libraries. One library expected return values to be error/status codes and others didn't. Having a single return statement can save time there.

I'm surprised that no one mentioned goto. Goto is not the bane of programming that everyone would have you believe. If you must have just a single return in each function, put it at the end and use gotos to jump to that return statement as needed. Definitely avoid flags and arrow programming which are both ugly and run slowly.

Eyal
+19  A: 

In a function that has no side-effects, there's no good reason to have more than a single return and you should write them in a functional style. In a method with side-effects, things are more sequential (time-indexed), so you write in an imperative style, using the return statement as a command to stop executing.

In other words, when possible, favor this style

return a > 0 ?
  positively(a):
  negatively(a);

over this

if (a > 0)
  return positively(a);
else
  return negatively(a);

If you find yourself writing several layers of nested conditions, there's probably a way you can refactor that, using predicate list for example. If you find that your ifs and elses are far apart syntactically, you might want to break that down into smaller functions. A conditional block that spans more than a screenful of text is hard to read.

There's no hard and fast rule that applies to every language. Something like having a single return statement won't make your code good. But good code will tend to allow you to write your functions that way.

Apocalisp
I wish I could upvote this 100 times.
Rick Minerich
+1 "If you find that your ifs and elses are far apart syntactically, you might want to break that down into smaller functions."
Andres Jaan Tack
+2  A: 

The more return statements you have in a function, the higher complexity in that one method. If you find yourself wondering if you have too many return statements, you might want to ask yourself if you have too many lines of code in that function.

But, not, there is nothing wrong with one/many return statements. In some languages, it is a better practice (C++) than in others (C).

hazzen
+1  A: 

You already implicitly have multiple implicit return statements, caused by error handling, so deal with it.

As is typical with programming, though, there are examples both for and against the multiple return practice. If it makes the code clearer, do it one way or the other. Use of many control structures can help (the case statement, for example).

rrichter
+4  A: 

Single exit point - all other things equal - makes code significantly more readable. But there's a catch: popular construction

resulttype res;
if if if...
return res;

is a fake, "res=" is not much better than "return". It has single return statement, but multiple points where function actually ends.

If you have function with multiple returns (or "res="s), it's often a good idea to break it into several smaller functions with single exit point.

ima
+4  A: 

My usual policy is to have only one return statement at the end of a function unless the complexity of the code is greatly reduced by adding more. In fact, I'm rather a fan of Eiffel, which enforces the only one return rule by having no return statement (there's just a auto-created 'result' variable to put your result in).

There certainly are cases where code can be made clearer with multiple returns than the obvious version without them would be. One could argue that more rework is needed if you have a function that is too complex to be understandable without multiple return statements, but sometimes it's good to be pragmatic about such things.

Matt Sheppard
A: 

I use multiple exit points for having error-case + handling + return value as close in proximity as possible.

So having to test for conditions a, b, c that have to be true and you need to handle each of them differently:

if (a is false) {
    handle this situation (eg. report, log, message, etc.)
    return some-err-code
}
if (b is false) {
    handle this situation
    return other-err-code
}
if (c is false) {
    handle this situation
    return yet-another-err-code
}

perform any action assured that a, b and c are ok.

The a, b and c might be different things, like a is input parameter check, b is pointer check to newly allocated memory and c is check for a value in 'a' parameter.

Marcin Gil
What do you do in the future as you need to add more and more cases to handle the branching logic? This is not even a full enumeration of all of the 8 combinations. What if you add d?!, that's 16! This code would be difficult to maintain and grow worse with time.
Rick Minerich
The pattern above is not for any branching logic. It is to assure that when you reach the point to start serious processing all your parameters are checked and ok - and that if something fails you will know exactly at which point.
Marcin Gil
+1 I tend to do this structure a lot, i.e. to let the program test the conditions/prequisites first and return immediately. This can also be done with exception handling, asserts and code contracts if the language supports those things.
Spoike
+3  A: 

If you end up with more than a few returns there may be something wrong with your code. Otherwise I would agree that sometimes it is nice to be able to return from multiple places in a subroutine, especially when it make the code cleaner.

Perl 6: Bad Example

sub Int_to_String( Int i ){
  given( i ){
    when 0 { return "zero" }
    when 1 { return "one" }
    when 2 { return "two" }
    when 3 { return "three" }
    when 4 { return "four" }
    ...
    default { return undef }
  }
}

would be better written like this

Perl 6: Good Example

@Int_to_String = qw{
  zero
  one
  two
  three
  four
  ...
}
sub Int_to_String( Int i ){
  return undef if i < 0;
  return undef unless i < @Int_to_String.length;
  return @Int_to_String[i]
}

Note this is was just a quick example

Brad Gilbert
Ok Why was this voted down? Its not like it isn't an opinion.
Brad Gilbert
+4  A: 

My preference would be for single exit unless it really complicates things. I have found that in some cases, multiple exist points can mask other more significant design problems:

public void DoStuff(Foo foo)
{
    if (foo == null) return;
}

On seeing this code, I would immediately ask:

  • Is 'foo' ever null?
  • If so, how many clients of 'DoStuff' ever call the function with a null 'foo'?

Depending on the answers to these questions it might be that

  1. the check is pointless as it never is true (ie. it should be an assertion)
  2. the check is very rarely true and so it may be better to change those specific caller functions as they should probably take some other action anyway.

In both of the above cases the code can probably be reworked with an assertion to ensure that 'foo' is never null and the relevant callers changed.

There are two other reasons (specific I think to C++ code) where multiple exists can actually have a negative affect. They are code size, and compiler optimizations.

A non-POD C++ object in scope at the exit of a function will have its destructor called. Where there are several return statements, it may be the case that there are different objects in scope and so the list of destructors to call will be different. The compiler therefore needs to generate code for each return statement:

void foo (int i, int j) {
  A a;
  if (i > 0) {
     B b;
     return ;   // Call dtor for 'b' followed by 'a'
  }
  if (i == j) {
     C c;
     B b;
     return ;   // Call dtor for 'b', 'c' and then 'a'
  }
  return 'a'    // Call dtor for 'a'
}

If code size is an issue - then this may be something worth avoiding.

The other issue relates to "Named Return Value OptimiZation" (aka Copy Elision, ISO C++ '03 12.8/15). C++ allows an implementation to skip calling the copy constructor if it can:

A foo () {
  A a1;
  // do something
  return a1;
}

void bar () {
  A a2 ( foo() );
}

Just taking the code as is, the object 'a1' is constructed in 'foo' and then its copy construct will be called to construct 'a2'. However, copy elision allows the compiler to construct 'a1' in the same place on the stack as 'a2'. There is therefore no need to "copy" the object when the function returns.

Multiple exit points complicates the work of the compiler in trying to detect this, and at least for a relatively recent version of VC++ the optimization did not take place where the function body had multiple returns. See Named Return Value Optimization in Visual C++ 2005 for more details.

Richard Corden
If you take all but the last dtor out of your C++ example, the code to destroy B and later C and B, will still have to be generated when the scope of the if statement ends, so you really gain nothing by not have multiple returns.
Eclipse
+1 And waaaaay down at the bottom of the list we have **the REAL reason this coding practice exists** - NRVO. However, this is a micro-optimization; and, like all micro-optimization practices, was probably started by some 50 year-old "expert" who is used to programming on a 300 kHz PDP-8, and does not understand the importance of clean and structured code. In general, take Chris S's advice and use multiple return statements whenever it makes sense to.
BlueRaja - Danny Pflughoeft
A: 

Multiple exit is good if you manage it well

The first step is to specify the reasons of exit. Mine is usually something like this:
1. No need to execute the function
2. Error is found
3. Early completion
4. Normal completion
I suppose you can group "1. No need to execute the function" into "3. Early completion" (a very early completion if you will).

The second step is to let the world outside the function know the reason of exit. The pseudo-code looks something like this:

function foo (input, output, exit_status)

  exit_status == UNDEFINED
  if (check_the_need_to_execute == false) then
    exit_status = NO_NEED_TO_EXECUTE  // reason #1 
    exit

  useful_work

  if (error_is_found == true) then
    exit_status = ERROR               // reason #2
    exit
  if (need_to_go_further == false) then
    exit_status = EARLY_COMPLETION    // reason #3
    exit

  more_work

  if (error_is_found == true) then
    exit_status = ERROR
  else
    exit_status = NORMAL_COMPLETION   // reason #4

end function

Obviously, if it's beneficial to move a lump of work in the illustration above into a separate function, you should do so.

If you want to, you can be more specific with the exit status, say, with several error codes and early completion codes to pinpoint the reason (or even the location) of exit.

Even if you force this function into one that has only a single exit, I think you still need to specify exit status anyway. The caller needs to know whether it's OK to use the output, and it helps maintenance.

Lahur
+2  A: 

What if the function is recursive? Huh? Huh?

then it might exit at a single point by calling itself.
JosephStyons
there is nothing that you cannot handle with an if.
Yar
Well then, I hope you are using a language with optimizing tail recursion!
Rick Minerich
Quick Sort is a great example of when multiple exit points makes recursive code more readable.
Michael Meadows
A: 

As an alternative to the nested IFs, there's a way to use do/while(false) to break out anywhere:

    function()
    {
        HRESULT error = S_OK;

        do
        {
            if(!SUCCEEDED(Operation1()))
            {
                error = OPERATION1FAILED;
                break;
            }

            if(!SUCCEEDED(Operation2()))
            {
                error = OPERATION2FAILED;
                break;
            }

            if(!SUCCEEDED(Operation3()))
            {
                error = OPERATION3FAILED;
                break;
            }
            if(!SUCCEEDED(Operation4()))
            {
                error = OPERATION4FAILED;
                break;
            }
        } while (false);

        return error;
    }

That gets you one exit point, lets you have other nesting of operations, but still not a real deep structure. If you don't like the !SUCCEEDED you could always do FAILED whatever. This kind of thing also lets you add other code between any two other checks without having to re-indent anything.

If you were really crazy, that whole if block could be macroized too. :D

    #define BREAKIFFAILED(x,y) if (!SUCCEEDED((x))) { error = (Y); break; }

    do
    {
        BREAKIFFAILED(Operation1(), OPERATION1FAILED)
        BREAKIFFAILED(Operation2(), OPERATION2FAILED)
        BREAKIFFAILED(Operation3(), OPERATION3FAILED)
        BREAKIFFAILED(Operation4(), OPERATION4FAILED)
    } while (false);
John Gardner
This has proved to be a highly successful way of implementing functions for me. Those who I have taught the technique has embraced it fully and simply loves it. Makes the code clean without unnecessary if's.
Mats Wiklander
and it litters the code with loop constructs that are not really loops - succinct and confusing for the same price ;-)
Steven A. Lowe
How is using goto (masking gotos is all your fake loop does) better than multiple exit points?
mghie
because you have one exit point. If you want to set a breakpoint to see what value is being returned, or other similar things, you only have to place one breakpoint, which is much more common if you're returning a value.
John Gardner
+1  A: 

This is probably an unusual perspective, but I think that anyone who believes that multiple return statements are to be favoured has never had to use a debugger on a microprocessor that supports only 4 hardware breakpoints. ;-)

While the issues of "arrow code" are completely correct, one issue that seems to go away when using multiple return statements is in the situation where you are using a debugger. You have no convenient catch-all position to put a breakpoint to guarantee that you're going to see the exit and hence the return condition.

Andrew Edgecombe
That's just a different sort of premature optimization. You should never optimize for the special case. If you find yourself debugging a specific section of code a lot, there's more wrong with it than just how many exit points it has.
Wedge
+1  A: 

It doesn't make sense to always require a single return type. I think it is more of a flag that something may need to be simplified. Sometimes it's necessary to have multiple returns, but often you can keep things simpler by at least trying to have a single exit point.

JosephStyons
A: 

You should never use a return statement in a method.

I know I will be jumped on for this, but I am serious.

Return statements are basically a hangover from the procedural programming days. They are a form of goto, along with break, continue, if, switch/case, while, for, yield and some other statements and the equivalents in most modern programming languages.

Return statements effectively 'GOTO' the point where the function was called, assigning a variable in that scope.

Return statements are what I call a 'Convenient Nightmare'. They seem to get things done quickly, but cause massive maintenance headaches down the line.

Return statements are diametrically opposed to Encapsulation

This is the most important and fundamental concept of object oriented programming. It is the raison d'etre of OOP.

Whenever you return anything from a method, you are basically 'leaking' state information from the object. It doesn't matter if your state has changed or not, nor whether this information comes from other objects - it makes no difference to the caller. What this does is allow an object's behaviour to be outside of the object - breaking encapsulation. It allows the caller to start manipulating the object in ways that lead to fragile designs.

LoD is your friend

I recommend any developer to read about the Law of Demeter (LoD) on c2.com or Wikipedia. LoD is a design philosophy that has been used at places that have real 'mission-critical' software constraints in the literal sense, like the JPL. It has been shown to reduce the amount of bugs in code and improve flexibility.

There has an excellent analogy based on walking a dog. When you walk a dog, you do not physically grab hold of its legs and move them such that the dog walks. You command the dog to walk and it takes care of it's own legs. A return statement in this analogy is equivalent to the dog letting you grab hold of its legs.

Only talk to your immediate friends:

  1. arguments of the function you are in,
  2. your own attributes,
  3. any objects you created within the function

You will notice that none of these require a return statement. You might think the constructor is a return, and you are on to something. Actually the return is from the memory allocator. The constructor just sets what is in the memory. This is OK so long as the encapsulation of that new object is OK, because, as you made it, you have full control over it - no-one else can break it.

Accessing attributes of other objects is right out. Getters are out (but you knew they were bad already, right?). Setters are OK, but it is better to use constructors. Inheritance is bad - when you inherit from another class, any changes in that class can and probably will break you. Type sniffing is bad (Yes - LoD implies that Java/C++ style type based dispatch is incorrect - asking about type, even implicitly, is breaking encapsulation. Type is an implicit attribute of an object. Interfaces are The Right Thing).

So why is this all a problem? Well, unless your universe is very different from mine, you spend a lot of time debugging code. You aren't writing code that you plan never to reuse. Your software requirements are changing, and that causes internal API/interface changes. Every time you have used a return statement you have introduced a very tricky dependency - methods returning anything are required to know about how whatever they return is going to be used - that is each and every case! As soon as the interface changes, on one end or the other, everything can break, and you are faced with a lengthy and tedious bug hunt.

They really are an malignant cancer in your code, because once you start using them, they promote further use elsewhere (which is why you can often find returning method-chains amongst object systems).

So what is the alternative?

Tell, don't ask.

With OOP - the goal is to tell other objects what to do, and let them take care of it. So you have to forget the procedural ways of doing things. It's easy really - just never write return statements. There are much better ways of doing the same things:

There is nothing wrong with the return concept, but return statements are deeply flawed.

If you really need an answer back - use a call back. Pass in a data structure to be filled in, even. That way you keep the interfaces clean and open to change, and your whole system is less fragile and more adaptable. It does not slow your system down, in fact it can speed it up, in the same way as tail call optimisation does - except in this case, there is no tail call so you don't even have to waste time manipulating the stack with return values.

If you follow these arguments, you will find there really is never a need for a return statement.

If you follow these practices, I guarantee that pretty soon you will find that you are spending a lot less time hunting bugs, are adapting to requirement changes much more quickly, and having less problems understanding your own code.

What is really the difference between returning a value and filling in a data structure that was passed in? The latter just models the former, in an uglier way. And have you read much about functional programming?
Daniel Earwicker
So you would prefer db.CreateNewRecord(newrecord, name, age); over set newrecord = db.CreateNewRecord(name, age)?? -- I think you lost me in there somewhere.
Trevel
Absolutely. The nice thing about this is the callee has full control over the process, as it should. In fact this is the way that COM works, and as much as I dislike MS, COM is surpisingly good at what it does.
Wow, this is just exactly the opposite of how I usually operate. Call it "purely impure" or "side-effect required" programming.
Chris Conway
I don't think I've ever read a post on here that was thought out and disagreed with it so thoroughly. The setnewrecord approach above by Trevel is cleaner and easier than the COM signature, and in many cases can lead to the avoidance of temporary variables to store values. Which is cleaner.
Steve
cont. Could you provide some example of how they break oop? The way I see it, if its a parameter or a return, you're getting the same thing out in the end. Unless you use generics, one way is just as brittle as the other.
Steve
The above post reflects the personal opinions, styles and preferences of the poster and should be taken with quite a few grains of salt with sugar added.
Blessed Geek
The above post is codswallop.
Anthony
A: 

In the interests of good standards and industry best practises, we must establish the correct number of return statements to appear in all functions. Obviously there is consensus against having one return statement. So I propose we set it at two.

I would appreciate it if everyone would look through their code right now, locate any functions with only one exit point, and add another one. It doesn't matter where.

The result of this change will undoubtedly be fewer bugs, greater readability and unimaginable wealth falling from the sky onto our heads.

Daniel Earwicker
If this were funny I would definitely upvote it!
Yar
I was trying to be funny but it just ended up sounding bitter, I know!
Daniel Earwicker
+2  A: 

The only important question is "How is the code simpler, better readable, easier to understand?" If it is simpler with multiple returns, then use them.

martinus
Unfortunately, "understandability" is in the eye of the beholder.
Steve Emmerson
+1  A: 

I prefer a single return statement. One reason which has not yet been pointed out is that some refactoring tools work better for single points of exit, e.g. Eclipse JDT extract/inline method.

starblue
+5  A: 

I lean to the idea that return statements in the middle of the function are bad. You can use returns to build a few guard clauses at the top of the function, and of course tell the compiler what to return at the end of the function without issue, but returns in the middle of the function can be easy to miss and can make the function harder to interpret.

Joel Coehoorn
Pretty much what I was going to say.
Benjol
+1  A: 

Well, maybe I'm one of the few people here old enough to remember one of the big reasons why "only one return statement" was pushed so hard. It's so the compiler can emit more efficient code. For each function call, the compiler typically pushes some registers on the stack to preserve their values. This way, the function can use those registers for temporary storage. When the function returns, those saved registers have to be popped off the stack and back into the registers. That's one POP (or MOV -(SP),Rn) instruction per register. If you have a bunch of return statements, then either each one has to pop all the registers (which makes the compiled code bigger) or the compiler has to keep track of which registers might have been modified and only pop those (decreasing code size, but increasing compilation time).

One reason why it still makes sense today to try to stick with one return statement is ease of automated refactoring. If your IDE supports method-extraction refactoring (selecting a range of lines and turning them into a method), it's very difficult to do this if the lines you want to extract have a return statement in them, especially if you're returning a value.

TMN
+35  A: 

Nobody has mentioned or quoted Code Complete so I'll do it.

16.2 return

Minimize the number of returns in each routine. It's harder to understand a routine if, reading it at the bottom, you're unaware of the possibility that it *return*ed somewhere above.

Use a return when it enhances readability. In certain routines, once you know the answer, you want to return it to the calling routine immediately. If the routine is defined in such a way that it doesn't require any cleanup, not returning immediately means that you have to write more code.

Chris S
+1  A: 

There are times when it is necessary for performance reasons (I don't want to fetch a different cache line kind of the same need as a continue; sometimes).

If you allocate resources (memory, file descriptors, locks, etc.) without using RAII then muliple returns can be error prone and are certainly duplicative as the releases need to be done manually multiple times and you must keep careful track.

In the example:

function()
{
    HRESULT error = S_OK;

    if(SUCCEEDED(Operation1()))
    {
        if(SUCCEEDED(Operation2()))
        {
            if(SUCCEEDED(Operation3()))
            {
                if(SUCCEEDED(Operation4()))
                {
                }
                else
                {
                    error = OPERATION4FAILED;
                }
            }
            else
            {
                error = OPERATION3FAILED;
            }
        }
        else
        {
            error = OPERATION2FAILED;
        }
    }
    else
    {
        error = OPERATION1FAILED;
    }

    return error;
}

I would have written it as:

function() {
    HRESULT error = OPERATION1FAILED;//assume failure
    if(SUCCEEDED(Operation1())) {

        error = OPERATION2FAILED;//assume failure
        if(SUCCEEDED(Operation3())) {

            error = OPERATION3FAILED;//assume failure
            if(SUCCEEDED(Operation3())) {

                error = OPERATION4FAILED; //assume failure
                if(SUCCEEDED(Operation4())) {

                    error = S_OK;
                }
            }
        }
    }
    return error;
}

Which certainly seems better.

This tends to be especially helpful in the manual resource release case as where and which releases are necessary is pretty straightforward. As in the following example:

function() {
        HRESULT error = OPERATION1FAILED;//assume failure
        if(SUCCEEDED(Operation1())) {

            //allocate resource for op2;
            char* const p2 = new char[1024];
            error = OPERATION2FAILED;//assume failure
            if(SUCCEEDED(Operation2(p2))) {

                //allocate resource for op3;
                char* const p3 = new char[1024];
                error = OPERATION3FAILED;//assume failure
                if(SUCCEEDED(Operation3(p3))) {

                    error = OPERATION4FAILED; //assume failure
                    if(SUCCEEDED(Operation4(p2,p3))) {

                        error = S_OK;
                    }
                }
                //free resource for op3;
                delete [] p3;
            }
            //free resource for op2;
            delete [] p2;
        }
        return error;
    }

If you write this code without RAII (forgetting the issue of exceptions!) with multiple exits then the deletes have to be written multiple times. If you write it with }else{ then it gets a little ugly.

But RAII makes the multiple exit resource issue mute.

+2  A: 

I vote for Single return at the end as a guideline. This helps a common code clean-up handling ... For example, take a look at the following code ...

void ProcessMyFile (char *szFileName)
{
   FILE *fp = NULL;
   char *pbyBuffer = NULL:

   do {

      fp = fopen (szFileName, "r");

      if (NULL == fp) {

         break;
      }

      pbyBuffer = malloc (__SOME__SIZE___);

      if (NULL == pbyBuffer) {

         break;
      }

      /*** Do some processing with file ***/

   } while (0);

   if (pbyBuffer) {

      free (pbyBuffer);
   }

   if (fp) {

      fclose (fp);
   }
}
Alphaneo
You vote for single return - in C code. But what if you were coding in a language that had Garbage collection and try..finally blocks?
Anthony
+2  A: 

This is often presented as a false dichotomy between multiple returns or deeply nested if statements. There's almost always a third solution which is very linear (no deep nesting) with only a single exit point. To achieve this, I approach the code with the mindset of checking prerequisites rather than return values.

The single exit point gives an excellent place to check post-conditions with asserts, or to place a breakpoint during debugging. Multiple exit points confound those efforts.

If you're in a language without exceptions or if you don't use RAII, then multiple exits usually requires duplicating clean-up code to avoid leaks.

Adrian McCarthy
+1 post-conditions.
Daniel Daranas
+2  A: 

I force myself to use only one return statement, as it will in a sense generate code smell. Let me explain:

function isCorrect($param1, $param2, $param3) {
    $toret = false;
    if ($param1 != $param2) {
        if ($param1 == ($param3 * 2)) {
            if ($param2 == ($param3 / 3)) {
                $toret = true;
            } else {
                $error = 'Error 3';
            }
        } else {
            $error = 'Error 2';
        }
    } else {
        $error = 'Error 1';
    }
    return $toret;
}

(The conditions are arbritary...)

The more conditions, the larger the function gets, the more difficult it is to read. So if you're attuned to the code smell, you'll realise it, and want to refactor the code. Two possible solutions are:

  • Multiple returns
  • Refactoring into separate functions

Multiple Returns

function isCorrect($param1, $param2, $param3) {
    if ($param1 == $param2)       { $error = 'Error 1'; return false; }
    if ($param1 != ($param3 * 2)) { $error = 'Error 2'; return false; }
    if ($param2 != ($param3 / 3)) { $error = 'Error 3'; return false; }
    return true;
}

Separate Functions

function isEqual($param1, $param2) {
    return $param1 == $param2;
}

function isDouble($param1, $param2) {
    return $param1 == ($param2 * 2);
}

function isThird($param1, $param2) {
    return $param1 == ($param2 / 3);
}

function isCorrect($param1, $param2, $param3) {
    $toret = false;
    if (!isEqual($param1, $param2)
        && isDouble($param1, $param3)
        && isThird($param2, $param3)
    ) {
        $toret = true;
    }
    return $toret;
}

Granted, it is longer and a bit messy, but in the process of refactoring the function this way, we've

  • created a number of reusable functions,
  • made the function more human readable, and
  • the focus of the functions is on why the values are correct.
Jrgns
-1: Bad example. You have omitted the error message handling. If that would not be needed the isCorrect could be expressed just as return xx where xx, yy and z are the isEqual, isDouble and isThird expressions.
kauppi
+4  A: 
Dinah
I disagree with you that multiple returns are the same as gotos. Unfortunately you don't give any reasons for your view. The rest of your post is just guilt by association.
Anthony
A while loop is also "essentially the same thing as a goto" - that doesn't mean that it has the same drawbacks.
Anthony
"essentially the same thing as using a GOTO" - a very very very inaccurate opinion. Why not say that "Using switch-case is the same thing as using a bunch of GOTOs" - you know, break; GOTO End;
Blessed Geek
A: 

I think in different situations different method is better.

For example if you should to process return value before return, you should have one point of exit. But in other situations it is more comfortable to use several returns.

One note. If you should process the return value before return in several situations but not in all, the best solutions (IMHO) to define method like ProcessVal and call it before return.

var retVal = new RetVal();

if(!someCondition) return ProcessVal(retVal);

if(!anotherCondition) return retVal;

Сергій
A: 

If it's okay to write down just an opinion, that's mine:

I totally and absolutely disagree with the `Single return statement theory' and find it mostly speculative and even destructive regarding the code readability, logic and descriptive aspects.

That habit of having one-single-return is even poor for bare procedural programming not to mention more high-level abstractions (functional, combinatory etc.). And furthermore, I wish all the code written in that style to go through some special rewriting parser to make it have multiple return statements!

A function (if it's really a function/query according to `Query-Command separation' note - see Eiffel programming lang. for example) just MUST define as many return points as the control flow scenarios it has. It is much more clear and mathematically consistent; and it is the way to write functions (i.e. Queries)

But I would not be so militant for the mutation messages that your agent does receive - the procedure calls.

Bubba88
A: 

I'm probably going to be hated for this, but ideally there should be no return statement at all I think, a function should just return its last expression, and should in the completely ideal case contain only one.

So not

function name(arg) {
    if (arg.failure?)
        return;

    //code for non failure
}

But rather

function name(arg) {
    if (arg.failure?)
        voidConstant
    else {
        //code for non failure


}

If-statements that aren't expressions and return statements are a very dubious practise to me.

Lajla
Which language is this, what is voidConstant and is this appropriate for a wide range of languages?
Anthony
@Anthony This is pseudocode, and voidConstant can be any of the constants used in languages that traditionally represent 'no useful return value' such as 'null' in Java or 'nil' in Ruby.I guess it is, some languages use it already, where the return value is simply always the value of the last computed expression, if you want to return nothing, you make your last expression void/null/nil/nothing.In these languages, void/null/nil/nothing is also usually part of any type.
Lajla
This is then a language design preference, not a style that is usable in many current languages - C# code where the method should return a value, but there are code paths with no return statement will not even compile. Something similar may happen in java.
Anthony
A: 

I believe that multiple returns are usually good (in the code that I write in C#).

The single-return style is a holdover from C. But you probably aren't coding in C.

More than one return style may be a bad habit in C code, where resources have to be explicitly de-allocated, but in languages that have automatic garbage collection and try..finally blocks (and using blocks in C#) this argument does not apply - in these languages, it is very uncommon to need centralised manual resource deallocation.

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 reduces the number of braces and indents or temporary variables.

There is no law requiring only one exit point for a method. Some people have an unsubstantiated belief in the superiority of this style but this is not backed up by any research.

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

I have talked about this at greater length on my blog.

Anthony
+1  A: 

No, because we don't live in the 1970s any more. If your function is long enough that multiple returns are a problem, it's too long.

(Quite apart from the fact that any multi-line function in a language with exceptions will have multiple exit points anyway.)

Ben Lings
+2  A: 

You know the adage - beauty is in the eyes of the beholder.

Some people swear by netbeans and some by intelli(what?), some by python and some by php.

In some shops you could lose your job if you insist on doing this

public void hello()
{
   if ( ....)
   {
      ....
   }
}

The question is all about visibility and maintainability.

I am addicted to using boolean algebra to reduce and simplify logic and use of state machines. However, there were past colleagues who believed my employ of "mathematical techniques" in coding is unsuitable because it would not be visible and maintainable. And that would be a bad practice. Sorry people, the techniques I employ is very visible and maintainable to me - because when I return to the code 6 months later, I would understand the code clearly rather seeing a mess of proverbial spaghetti.

Hey buddy (like a former client used to say) do what you want as long as you know how to fix it when I need you to fix it.

I remember 20 years ago, a colleague of mine was fired for employing what today would be called agile development strategy. He had a meticulous incremental plan. But his manager was yelling at him "You can't incrementally release features to users! You must stick with the waterfall." His response to the manager was that incremental development would be more precise to customer's needs. He believed in developing for the customers needs but the manager believed in coding to "customer's requirement".

We are frequently guilty for breaking data normalization, mvp and mvc boundaries. We inline instead of constructing a function. We take short cuts.

Personally, I believe that PHP is bad practice but what do I know. All the theoretical arguments boils down to trying fulfill one set of rules

quality = precision, maintainability and profitability.

All other rules fade into the background. And of course this rule never fades:

Laziness is the virtue of a good programmer.

Blessed Geek
+2  A: 

Having a single exit point does provide an advantage in debugging, because it allows you to set a single breakpoint at the end of a file to see what value is actually going to be returned.

Mark