share
Stack OverflowShould a function have only one return statement?
[+564] [49] David
[2008-08-31 09:26:55]
[ language-agnostic coding-style ]
[ http://stackoverflow.com/questions/36707/should-a-function-have-only-one-return-statement ]

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?

(16) I don't agree that the question is language agnostic. With some languages, having multiple returns is more natural and convenient than with others. I'd be more likely to complain about early returns in a C function than in a C++ one that uses RAII. - Adrian McCarthy
This is closely related and has excellent answers: programmers.stackexchange.com/questions/118703/… - Tim Schmelter
[+593] [2008-08-31 09:31:40] Matt Hamilton [ACCEPTED]

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.


Good argument. This in a way would promote a style of contract, asserting and testing conditions upfront. - malach
(65) 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
(134) This is known as a "guard statement" is Fowler's Refactoring. - Lars Westergren
(1) 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
(5) 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
(10) 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
(2) 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
(9) 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
(2) 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
(1) @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
@Matt try is very expensive to set up. If you're doing it but you don't expect an exception, I'd say there's probably a better way to do it. - Michael Blackburn
(2) @LarsWestergren Guards pre-date even the great Martin Fowler :) - Chris S
(3) Of course, in most cases it is preferable to throw an exception if an argument is null while you expect some value. Having an argument that can take on null can be useful, but it should be used sparingly IMHO. An exception of course is as much as an exit point as a return statement, and most IDE's nowadays highlight all exit points when you e.g. point to the method's return type. - owlstead
(5) 'Imagine: you need to do "IncreaseStuffCallCounter" method at the end of 'DoStuff' function. What will you do in this case? :)' -- DoStuff() { DoStuffInner(); IncreaseStuffCallCounter(); } - Jim Balter
I generally find guard statements unappealing, as they tend to invert the logic and sometimes introduce double negatives. "If the prerequisite is met, then do the operation" reads better to me than "If the prerequisite is NOT met, then abort; (otherwise) do the operation." That said, I find simple guard statements less objectionable that returns in the middle of a function. - Adrian McCarthy
Everyone seems to be defending multiple returns by talking about not wanting to wrap their code in if blocks etc. When you return to your code in 2 months, or if you're reading someone's code, it'll be so much easier to see what code depends on what condition. Code readability doesn't mean that your code looks pretty, it means that the logic is more readable. - nenchev
1
[+214] [2009-04-09 11:47:43] Chris S

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

17.1 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 returned 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.

[1] http://www.cc2e.com/

(31) +1 for the nuance of "minimize" but not prohibit multiple returns. - Raedwald
+1 for the very same reason. - foo
(8) "harder to understand" is very subjective, especially when the author does not provide any empirical evidence to support the general claim... having a single variable that must be set in many conditional locations in the code for the final return statement is equally subjected to "unaware of the possibility that the variable was assigned somewhere above int he function! - Heston T. Holtmann
(11) Personally, I favour returning early where possible. Why? Well, when you see the return keyword for a given case, you instantly know "I'm finished" -- you don't have to keep reading to figure out what, if anything, occurs afterwards. - Mark Simpson
(9) @HestonT.Holtmann: The thing that made Code Complete unique among programming books was that the advice is backed by empirical evidence. - Adrian McCarthy
(4) This should probably the accepted answer, since it mentions that having multiple return points is not always good, yet sometimes necessary. - Rafid
Multiple returns is never really good, I think its a crutch and it reduces the readability of your logic. Reducing the number of lines of code or indents, doesn't mean that your code is more readable. - nenchev
2
[+194] [2008-08-31 09:46:19] ljs

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:-

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:-

string fooBar(string s, int? i) {
  var ret = "";
  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 archaic point of view these days.


(8) 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'?!... - ljs
@kronoz, break/continue are pretty much the same thing as an early return from an extracted loop method. - Dustin Getz
(5) 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
(22) @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... - ljs
(5) @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. - ljs
the code you are showing for multiple returns violates another principal: you shouldn't place multiple statements on the same line. - inor
(2) The point of not having multiple return statements is make debugging easier, a far second it is for readability. One breakpoint at the end allows you to see the exit value, sans exceptions. As for readability the 2 shown functions do not behave the same. The default return is an empty string if !r.Passed but the "more readable" one changes this to return null. The author misread that earlier there was a default after only a few lines. Even in trivial examples it is easy to have unclear default returns, something a single return at the end helps enforce. - Mitch
(1) +1 - But string ret = ""should be string ret = null to both functions be equivalent. - André Figueiredo
3
[+155] [2008-08-31 12:50:37] 17 of 26

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.


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
(5) @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. - GEOCHET
(6) @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
(5) 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
(10) You can refactor it into this, keeps its "purity": if(!SUCCEEDED(Operation1())) { }else error = OPERATION1FAILED; if(error!=S_OK){ if(SUCCEEDED(Operation2())) { } else error = OPERATION2FAILED; } if(error!=S_OK){ if(SUCCEEDED(Operation3())) { } else error = OPERATION3FAILED; } //etc. - Joe Pineda
(5) 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
(1) That's a complexity metric. Creates more edges (paths between blocks of code) - davenpcj
(2) 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
(3) 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
(5) 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
(4) Usually if you are considering more than one return statement in a function the function can be rewritten or broken down into smaller functions. Although it is more of a guideline than a rule, it is often an indication that you are taking the wrong approach. - J.Money
Not to beat a dead horse, but this code is awful for a different reason: the complexity is certainly increased by the function's size. It's doing too many things at once. At the very least, the most-inner if statement is useful because it can be inverted (! SUCCEEDED). But most importantly, this function is doing multiple things, and those results can be broken apart: function doStep1() { int r = ERROR1; if (SUCCESS(step1())) r = doStep2(); return r; } where inevitably the last step finally returns OK upon success. In object oriented design, this could even be made into objects... - pickypg
(2) But that’s just a bad implementation, it’s not been imposed by the single-exit point itself. A single exit point does not automatically imply deep nesting, it’s just that’s how a lot of devs deal with it. Check out the various design-by-contract implementations as something that has often historically been solved with deep nesting or multi-exits but nowadays tends to have more elegant solutions. - Vman
I can’t help thinking that if you are going to impose a single exit point rule then if you also have a suggested maximum nesting level coding practice in place as well, as it will force devs to think about writing clean code rather than this sort of stuff. Two rules make a right, so to speak. - Vman
I think this is a poor example; in all cases of the operation failing an exception being thrown would invariably be better than a return code. The code would be much improved by having each operation sub routine throw an exception when they fail instead of relying on the caller to do so. - Andy
4
[+62] [2008-08-31 09:38:53] cdv

Structured programming [1] 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 [2] 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.

[1] http://en.wikipedia.org/wiki/Structured%5Fprogramming
[2] http://rads.stackoverflow.com/amzn/click/0201485672

(39) Structured programming doesn't say a thing. Some (but not all) people who describe themselves as advocates of structured programming say that. - wnoise
(12) "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
(5) @wnoise +1 for the comment, so true. All "structude programming" says is don't use GOTO. - paxos1977
@wnoise +1 for a vital observation - Galghamon
(1) @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
@wnoise goto, break, return and continue (did I miss any?) are all about ignoring the 'structure' of the code. Some programs may be too hard to structure well, and these tricks can be useful, but it makes sense to say they should be avoided (in structured programming). - Thomas Ahle
5
[+55] [2008-08-31 09:48:40] blank

As Kent Beck notes when discussing guard clauses in Implementation Patterns [1] 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.

[1] http://rads.stackoverflow.com/amzn/click/0321413091

Of course, "one long nested bunch of if-then-else statements" is not the only alternative to guard clauses. - Adrian McCarthy
6
[+43] [2008-09-07 18:06:17] Apocalisp

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.


(1) I wish I could upvote this 100 times. - Rick Minerich
(3) +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
(3) +1, if this is a problem, it usually means you are doing too much in a single function. It really depresses me that this is not the highest voted answer - Matt Briggs
7
[+35] [2008-08-31 09:48:38] Pete Kirkham

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.


(3) 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
This makes for an interesting exception to the rule: If your programming language does not contain something that is automatically called at the end of the method (such as try ... finally in Java) and you need to do resource maintenance you could do with a single return at the end of a method. Before you do this, you should seriously consider refactoring the code to get rid of the situation. - owlstead
8
[+32] [2009-03-20 13:03:21] Joel Coehoorn

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.


(2) Pretty much what I was going to say. - Benjol
9
[+22] [2010-07-23 17:29:19] Mark

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.


(3) BRAVO! You are the only person to mention this objective reason. It's the reason I prefer single exit points vs. multiple exit points. If my debugger could set a break point on any exit point I'd probably prefer multiple exit points. My current opinion is the people who code multiple exit points do it for their own benefit at the expense of those others who have to use a debugger on their code (and yes I'm talking about all you open-source contributors who write code with multiple exit points.) - MikeSchinkel
(2) YES. I'm adding logging code to a system that's intermittently misbehaving in production (where I can't step through). If the previous coder had used single-exit, it would have been MUCH easier. - Michael Blackburn
(1) True, in debugging it is helpful. But in practice I was able in most cases to set the breakpoint in the calling function, just after the call - effectively to the same result. (And that position is found on the call stack, of course.) YMMV. - foo
This is unless your debugger provides a step-out or step-return function (and each and every debugger does as far as I know), which shows the returned value right after is returned. Changing the value afterwards may be a bit tricky if it is not assigned to a variable though. - owlstead
10
[+19] [2009-05-14 22:04:28] Adrian McCarthy

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

Yes, there are:

  • The single exit point gives an excellent place to assert your post-conditions.
  • Being able to put a debugger breakpoint on the one return at the end of the function is often useful.
  • Fewer returns means less complexity. Linear code is generally simpler to understand.
  • If trying to simplify a function to a single return causes complexity, then that's incentive to refactor to smaller, more general, easier-to-understand functions.
  • If you're in a language without destructors or if you don't use RAII, then a single return reduces the number of places you have to clean up.
  • Some languages require a single exit point (e.g., Pascal and Eiffel).

The question is often posed 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.

Update: Apparently MISRA guidelines promote single exit [1], too.

[1] http://stackoverflow.com/questions/17177276/c-c-conditional-return-statements/17177315#17177315

(4) +1 post-conditions. - Daniel Daranas
another good reason, probably the best these days, to have a single return statement is logging. if you want to add logging to a method, you can place a single log statement that conveys what the method returns. - inor
How common was the FORTRAN ENTRY statement? See docs.oracle.com/cd/E19957-01/805-4939/6j4m0vn99/index.html. And if you're adventurous, you can log methods with AOP and after-advice - Eric Jablow
11
[+16] [2008-08-31 10:29:18] Scott Dorman

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.


12
[+9] [2008-08-31 09:30:18] Rob Cooper

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

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).

[1] http://www.refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html

13
[+9] [2008-09-17 18:01:13] Richard Corden

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 [1] for more details.

[1] http://blogs.msdn.com/aymans/archive/2005/10/13/480699.aspx

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
(3) +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
14
[+9] [2009-08-14 09:40:24] Jrgns

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.

(4) -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 && yy && zz; where xx, yy and z are the isEqual, isDouble and isThird expressions. - kauppi
15
[+9] [2010-07-14 19:20:52] Ben Lings

No, because we don't live in the 1970s any more [1]. 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.)

[1] http://en.wikipedia.org/wiki/Structured_programming

16
[+8] [2008-08-31 14:13:10] Henrik Gustafsson

There are good things to say about having a single exit-point, just as there are bad things to say about the inevitable "arrow" [1] 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 [2] article of the "SSDSLPedia" and the single function exit point [3] article of the "Portland Pattern Repository's Wiki" have some insightful arguments around this. Also, of course, there is this post [4] 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 [5] are generally ok.
  • Exceptions could/should probably replace most 'guard clauses' when possible.
[1] http://c2.com/cgi/wiki?ArrowAntiPattern
[2] http://ssdl-wiki.cs.technion.ac.il/wiki/index.php/Spartan%5Fprogramming
[3] http://c2.com/cgi/wiki?SingleFunctionExitPoint
[4] #36711
[5] http://www.refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html

+1 The best answer by far - Trap
The example crashes for y < 0, because it tries to free the NULL pointer ;-) - ammoQ
(1) 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
The "arrow" pattern is not an inevitable alternative. This is a false dichotomy. - Adrian McCarthy
17
[+8] [2010-06-27 17:32:49] Anthony

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.

There is no law requiring only one exit point for a method in all programming languages. Some people insist on the superiority of this style, and sometimes they elevate it to a "rule" or "law" but this belief is not backed up by any evidence or research.

More than one return style may be a bad habit in C code, where resources have to be explicitly de-allocated, but languages such as Java, C#, Python or JavaScript that have constructs such as automatic garbage collection and try..finally blocks (and using blocks in C#), and 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.

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 [1].

[1] http://www.anthonysteele.co.uk/the-single-exit-point-law

18
[+6] [2008-09-02 21:17:52] John Channing

Having a single exit point reduces Cyclomatic Complexity [1] 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.

[1] http://en.wikipedia.org/wiki/Cyclomatic_complexity

Very insightful. Although, I feel that until a programmer knows when to use multiple exit points, they should be constrained to one. - Rick Minerich
(4) Not really. The cyclomatic complexity of "if (...) return; ... return;" is the same as "if (...) {...} return;". They both have two paths through them. - Steve Emmerson
19
[+6] [2010-07-15 05:27:18] Blessed Geek

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

Some people swear by NetBeans [1] and some by IntelliJ IDEA [2], some by Python [3] and some by PHP [4].

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 six 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 [5] 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 [6]." 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 [7] and MVC [8] boundaries. We inline instead of constructing a function. We take shortcuts.

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.

[1] http://en.wikipedia.org/wiki/NetBeans
[2] http://en.wikipedia.org/wiki/IntelliJ_IDEA
[3] http://en.wikipedia.org/wiki/Python_%28programming_language%29
[4] http://en.wikipedia.org/wiki/PHP
[5] http://en.wikipedia.org/wiki/Agile_software_development
[6] http://en.wikipedia.org/wiki/Waterfall_model
[7] http://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93presenter
[8] http://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93controller

"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." Problem: it's often not you who 'fixes' it. - Dan Barron
20
[+6] [2011-02-17 20:42:44] David Clarke

I lean towards using guard clauses to return early and otherwise exit at the end of a method. The single entry and exit rule has historical significance and was particularly helpful when dealing with legacy code that ran to 10 A4 pages for a single C++ method with multiple returns (and many defects). More recently, accepted good practice is to keep methods small which makes multiple exits less of an impedance to understanding. In the following Kronoz example copied from above, the question is what occurs in //Rest of code...?:

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 realise the example is somewhat contrived but I would be tempted to refactor the foreach loop into a LINQ statement that could then be considered a guard clause. Again, in a contrived example the intent of the code isn't apparent and someFunction() may have some other side effect or the result may be used in the // Rest of code....

if (string.IsNullOrEmpty(s) || i == null) return null;
if (someFunction(s, i).Any(r => !r.Passed)) return null;

Giving the following refactored function:

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

  if (string.IsNullOrEmpty(s) || i == null) return null;
  if (someFunction(s, i).Any(r => !r.Passed)) return null;

  // Rest of code...

  return ret;
}

Doesn't C++ have exceptions? Then why would you return null instead of throwing an exception indicating that the argument is not accepted? - owlstead
(1) As I indicated, the example code is copied from a previous answer (stackoverflow.com/a/36729/132599). The original example returned nulls and refactoring to throw argument exceptions wasn't material to the point I was trying to make or to the original question. As a matter of good practice, then yes I would normally (in C#) throw an ArgumentNullException in a guard clause rather than return a null value. - David Clarke
21
[+5] [2008-08-31 09:39:25] Phil Bennett

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.


Not to mention having to tell your mind to skip the if statement in front of each method call that returns success or not :( - owlstead
22
[+5] [2008-08-31 13:38:46] Jon Limjap

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.


23
[+5] [2008-09-11 07:39:18] ima

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.


24
[+5] [2008-09-11 07:48:45] Matt Sheppard

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.


25
[+4] [2008-08-31 09:31:16] OysterD

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...


That does not really make a difference. If you change the type of the local variable that is returned, then you will have to fix all the assignments to that local variable. And it is probably better to define a method with a different signature anyway, as you will have to fix all the method calls as well. - owlstead
26
[+4] [2008-09-07 18:19:18] hazzen

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).


27
[+4] [2008-09-15 15:53:45] Brad Gilbert

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


Ok Why was this voted down? Its not like it isn't an opinion. - Brad Gilbert
28
[+4] [2009-04-27 06:07:00] Alphaneo

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);
   }
}

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
29
[+3] [2008-09-23 23:47:34] Andrew Edgecombe

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.


(5) 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
Depends on your debugger too. - owlstead
30
[+3] [2008-09-26 14:19:18] JosephStyons

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.


31
[+3] [2010-02-04 23:42:37] Dinah

Having multiple exit points is essentially the same thing as using a GOTO. Whether or not that's a bad thing depends on how you feel about raptors.


(16) 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
(8) A while loop is also "essentially the same thing as a goto" - that doesn't mean that it has the same drawbacks. - Anthony
(3) "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
(2) Isn't a function call the same thing as using a GOTO? BAD programmer. - ErikE
(1) @ErikE: No, a function call is the same as using a GOSUB which is a structured programming concept. GOTO is not. - Adrian McCarthy
@AdrianMcCarthy Please don't miss that my prior comment was intended to be wrong: it is a parody of this answer. I don't by any means think functions are the same as GOTO. And by the same token, neither is return the same as GOTO! Return means "abandon current block of code (for reasons obvious--there is no more work to do) and resume execution after the calling statement". It is not arbitrary or absolute. GOTO on the other hand forces execution to jump around to an arbitrary, unknown point for purposes unknown. Ultimately, it is all about human comprehension of code. - ErikE
(1) @AdrianMcCarthy And some of us believe that better human comprehension can be achieved in code through abandoning the arbitrary rule of "one exit point". For any human who accepts and believes this rule, then return in the middle of a function is jarring, and confusing. So change! Those humans should change their minds. In the final analysis what I think we're really arguing about is convention, not sense. - ErikE
(1) @ErikE: The fact that your comment was parody was not obvious to me. I apologize for misunderstanding it. I disagree that the rule (I'd call it a guideline) is arbitrary. Whenever I've forced myself to eliminate early returns from a function, the function has always come out better (better == more comprehensible, simpler, more robust, better focused on doing one thing, etc.). - Adrian McCarthy
(1) @AdrianMcCarthy Those are valid points. I think that you and I can at least agree that we don't want to multiply return statements unnecessarily--that will indeed cause confusion. But I for one am sold on "guard statements" as they make coding very simple in many cases--sometimes highly improving even very short functions. And I think there may be use cases for other non-final return usage. But we can agree to disagree. I'm sorry if I was too harsh in my comments. - ErikE
32
[+2] [2008-09-07 18:31:07] rrichter

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).


33
[+2] [2008-12-26 18:31:15] martinus

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.


(2) Unfortunately, "understandability" is in the eye of the beholder. - Steve Emmerson
34
[+2] [2009-03-20 14:03:42] TMN

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.


35
[+2] [2010-05-08 05:29:04] Bubba88

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.


36
[+1] [2008-09-11 09:06:41] Marcin Gil

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.


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
(1) 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) +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
37
[+1] [2008-12-24 09:19:40] Daniel Earwicker

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.


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
38
[+1] [2008-12-30 07:58:32] starblue

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.


39
[+1] [2010-11-30 18:20:22] BJS

I always avoid multiple return statements. Even in small functions. Small functions can become larger, and tracking the multiple return paths makes it harder (to my small mind) to keep track of what is going on. A single return also makes debugging easier. I've seen people post that the only alternative to multiple return statements is a messy arrow of nested IF statements 10 levels deep. While I certain agree that such coding does occur, it isn't the only option. I wouldn't make the choice between a multiple return statements and a nest of IFs, I'd refactor it so you'd eliminate both. And that is how I code. The following code eliminates both issues and, in my mind, is very easy to read:

    public string GetResult()
    {
        string rv = null;
        bool okay = false;

        okay = PerformTest(1);

        if (okay)
        {
            okay = PerformTest(2);
        }

        if (okay)
        {
            okay = PerformTest(3);
        }

        if (okay)
        {
            okay = PerformTest(4);
        };

        if (okay)
        {
            okay = PerformTest(5);
        }

        if (okay)
        {
            rv = "All Tests Passed";
        }

        return rv;
    }

(3) Adding a flag to code is, from an analytical standpoint, equivalent to having two copies of the code--one were the flag is assumed to be false and one where it is assumed to true--and jumping between them every time the flag is changed. Adding flags may sometimes make code less bulky, but it does not reduce the analytical complexity. Note that in cases like the above example, adding a flag will yield an executable that is larger and slower than could be obtained without. - supercat
(1) Why not okay = PerformTestOne() && PerformTest2() && PerformTest3() ... IIRC, '&&' will short-circuit on the first of these to return false, so even if the tests are expensive, you aren't going to perform all of them. - Michael Blackburn
i agree with Michael. i was thinking the same thing... - inor
@MichaelBlackburn I think that has something to do how you think. if you do it in your way you cannot debug in a certain debug style where you just want to see during debug what every method returns without entering it. - Offler
40
[0] [2008-09-03 21:49:33] Eyal

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.


41
[0] [2008-09-23 22:24:23] John Gardner

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);

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
(1) and it litters the code with loop constructs that are not really loops - succinct and confusing for the same price ;-) - Steven A. Lowe
(3) 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
42
[0] [2009-04-26 00:55:40] pgast

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.


43
[0] [2010-02-10 11:07:26] Сергій

I think in different situations different method is better. For example, if you should process the 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 a method like ProcessVal and call it before return:

var retVal = new RetVal();

if(!someCondition)
    return ProcessVal(retVal);

if(!anotherCondition)
   return retVal;

44
[0] [2010-05-19 06:53:09] Zorf

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.


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. - Zorf
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
45
[0] [2014-02-15 04:59:39] TaylorMac

One might argue... if you have multiple conditions that must be satisfied before the tasks of the function are to be performed, then don't invoke the function until those conditions are met:

Instead of:

function doStuff(foo) {
    if (foo != null) return;
}

Or

function doStuff(foo) {
    if (foo !== null) {
        ...
    }
}

Don't invoke doStuff until foo != null

if(foo != null) doStuff(foo);

Which, requires that every call site ensures that the conditions for the invocation are satisfied before the call. If there are multiple call sites, this logic is perhaps best placed in a separate function, in a method of the to-be-invoked function (assuming they are first-class citizens), or in a proxy.

On the topic of whether or not the function is mathematically provable, consider the logic over the syntax. If a function has multiple return points, this doesn't mean (by default) that it is not mathematically provable.


46
[0] [2014-08-08 10:56:39] Tom Tanner

This is mainly a hang over from Fortran, where it was possible to pass multiple statement labels to a function so it could return to any one of them.

So this sort of code was perfectly valid

       CALL SOMESUB(ARG1, 101, 102, 103)
C Some code
 101   CONTINUE
C Some more code
 102   CONTINUE
C Yet more code
 103   CONTINUE
C You get the general idea

But the function being called made the decision as to where your code path went. Efficient? Probably. Maintainable? No.

That is where that rule comes from (incidentally along with no multiple entry points to a function, which is possible in fortran and assembler, but not in C).

However, the wording of that looks like it can be applied to other languages (the one about multiple entry points can't be applied to other languages, so it's not really a program). So the rule got carried over, even though it refers to a completely different problem, and isn't applicable.

For more structured languages, that rule needs to be dropped or at least thought through more. Certainly a function spattered with returns is difficult to understand, but returning at the beginning isn't an issue. And in some C++ compilers a single return point may generate better code if you're returning a value from only one place.

But the original rule is misunderstood, misapplied. and no longer relevant.


47
[-1] [2008-09-18 03:01:48] Lahur

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.


48
[-1] [2008-12-24 04:36:34] Mike 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 [1] (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.

[1] http://en.wikipedia.org/wiki/Law_of_Demeter

(7) 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. - Mike A
(2) Wow, this is just exactly the opposite of how I usually operate. Call it "purely impure" or "side-effect required" programming. - Chris Conway
(1) 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
(3) 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
(4) The above post is codswallop. - Anthony
So, when I am asking a function to select a result from a given set of items for me, what "state information" is leaked, exactly? What allocation is made? What dependecy has been intruduced? -- I can see the cases you're aiming at, but for general use, above is a load of bollocks, sorry to say so. - foo
How would you implement Math.Max(int, int)?? - Stefan Steinegger
Heh, I knew when I posted I'd upset a few people with this. Sure I've studied functional programming (Haskell), and used it. I wouldn't come here making 'radical' statements otherwise. FP is fine for small problems, but the fact is that the typed lambda calculi are not even Turing-complete. That means you simply can't write things like databases, file systems etc, without stepping outside the 'pure' world, i.e. something has to deal with the side effects. - Mike A
I use continuation-passing style (CPS) and write things like db.CreateNewRecord(name, age, use_new_record); (by the way, anything FP can be automatically translated to CPS, and that's what functional language compilers do). Note that this has a nice effect of tying up all the onward side-effects into the continuation. Great if you need to be asynchronous, i.e. Other stuff can happen whilst waiting for the database. - Mike A
(1) Seems confusion exists about what I want to say here. I'll try to be clearer, and please try to read clearly. I emphasised Tell-don't-ask, i.e. stop asking for state. This stops state moving away from where it is managed. The main counter-examples involve "it's simple to do x = f(y) using a return statement, but it's hard to do that without a return statement.". So the "asking" (function call) is still there, but the "reply" (return statement) part is gone. So of course that is hard. But that's not my point. My point is that we shouldn't be asking at all. We should be sending messages. - Mike A
Also, for the FP programmers, consider this: unless you are using a pure FP language, you can't rule out side-effects, so in any case you are fighting a losing battle. And guess what? Side-effects are really useful! One generally useful example is profiling code. Do I really worry that the profiler is going to update its log? Of course not, I want that. Do I have to create a Monadic type system for profiling? Yeuch! Sure the logging might upset my other code, but I'll live with that tiny chance. - Mike A
Stefan S.: the maximum example is an interesting one, as it is also the example picked by Alexander Stepanov to criticise OOP versus generic programming. He basically implied it couldn't be done. However, again this is an example of thinking in terms of functions. How would I do it? The CPS way: max(int, int, use_result) where use_result takes the maximum. However, I'd also look at the code, and usually the wider picture is that of finding the maximum of a bunch of values. The OOP version is a class that simply keeps a running maximum of any things that can be ordered. So much for generics. - Mike A
Final note for now: I note there are several comments that simply dismiss these concepts out-of-hand, lobbing the odd insult (and some are just insults). I would point out that these concepts are not my own. I discovered them by reading widely on C2 and many other sites. I would invite these masterminds to construct reasoned arguments or suggest they develop their commenting abilities starting at forums more appropriate to their level, such as YouTube or Yahoo Answers. - Mike A
How would you get data? Getters are an important part of OOP. Otherwise everything would have to be public. - J.Money
Ha, this definitely feels a little like Nietzsche's madman -- it might sound mad, but it's largely because what's being said is potentially well ahead of its time. And though I'm pro-single-return, Mike A has nearly convinced me this is the future/what we'd do if we started over without our C roots. Well done. Great thought experiment. - ruffin
@J.Money: Public getters can be avoided by keeping the property in the class that uses it (i.e. passes it as an argument to another class or a system function.) E.g. a class that reads & parses a config file has no need for a "getPath" method - its job is to output the parsed config, not the filename. (It will probably have a "setPath" method though.) - finnw
"The procedural programming days" are not over and probably never will be. - finnw
This CPS style is kind of similar to rack/stack where you pass in something handle the next step. Following this idea, basically the entire software could be drawn as a food chain where functions contain functions contains functions... etc. Thus the many onion layers fulfill request/response tasks. Ultimately the initial caller still needs a result, in some cases it needs the result itself, in other cases the result can be a side effect, but the caller will often need some sort of feedback to know whether things worked out or not. @MikeA can you show any software written strictly like this? - CMCDragonkai
Here's some interesting information to help with CPS based callback hell: eamodeorubio.github.io/tamingasync/# and github.com/baconjs/bacon.js - CMCDragonkai
49