I usually code with the first one since that is the way my brain works while coding, although I think I prefer the 2nd one since it takes care of any error handling right away and I find it easier to read

19 Answers
19

I prefer the second style. Get invalid cases out of the way first, either simply exiting or raising exceptions as appropriate, put a blank line in there, then add the "real" body of the method. I find it easier to read.

The advantage of single a exit point is that... there's a single exit point! With your example, there's several points which could return. With a more complex function, that could turn into a hunt-the-exit-point when the format of the return value changes. Of course, there's times when forcing a single exit point doesn't make sense.
–
JohnLNov 11 '10 at 21:17

28

@JohnL big functions are the problem, not multiple exit points. Unless you're working in a context where an additional function call will slow down your code immensely, of course...
–
YarNov 11 '10 at 21:23

3

@Yar: True, but the point stands. I'm not going to try and convert anyone, and I was just pointing out the advantage of going for as few exit points as possible (and Jason's example was kind of straw man anyway). Just next time you're pulling your hair out trying to figure out why SomeFunction sometimes returns odd values, maybe you'll wish you could add a logging call just before the return. Much easier to debug if there's only one of the buggers! :)
–
JohnLNov 11 '10 at 23:11

26

@Jason Viers As if a single return value; helps!?! Then one has to hunt half a dozen value = ..., with the disadvantage that you are never sure that value will not be changed between this assignment and the final return. At least an immediate return is clear that nothing will change the result anymore.
–
SjoerdFeb 18 '11 at 17:26

2

@Sjoed: I second Sjoerd here. If you want to log the result, you can log at the caller site. If you want to know the reason, you have to log at each exit point/assignment so both are identical in this aspect.
–
Matthieu M.Feb 18 '11 at 18:46

It depends - In general I am not going to go out of my way to try and move a bunch of code around to break out of the function early - the compiler will generally take care of that for me. That said though, if there are some basic parameters at the top that I need and can't continue otherwise, I will breakout early. Likewise, if a condition generates a giant if block in function I will have it breakout early as a result of that as well.

That said though, if a function requires some data when it is called, I'm usually going to be throwing an exception (see example) as opposed to just returning.

If you have one entry point and one exit point then you always have to track the entire code in your head all the way down to the exit point (you never know if some other piece of code bellow does something else to the result, so you have to track it up until the exist). You do that no mater which branch determines the final result. This is hard to follow.

With one entry and multiple exists, you return when you have your result and don't bother tracking it all the way down to see that nobody does anything else to it (because there won't be anything else since you returned). It's like having the method body split into more steps, which each step with the possibility to return the result or let the next step try its luck.

Early return if there is some obvious dead end condition to check for right away that would make running the rest of the function pointless.*

Set Retval + single return if the function is more complex and could have multiple exit points otherwise (readability issue).

*This can often indicate a design problem. If you find that a lot of your methods need to check some external/paramater state or such before running the rest of the code, that's probably something that should be handled by the caller.

When I write code that may be shared, my mantra is "Assume Nothing. Trust No One." You should always validate your inputs and any external state you depend on. Better to throw an exception than possibly corrupt something because somebody gave you bad data.
–
TMNNov 11 '10 at 20:59

2

Key point here is that in OO you throw an exception, not return. Multiple returns can be bad, multiple exception throws aren't necessarily a code smell.
–
Michael KNov 12 '10 at 13:59

2

@MichaelK: A method should an exception if the post-condition can't be met. In some cases, a method should exit early because the post-condition has been achieved even before the function started. For example, if one invokes a "Set control label" method to change a control's label to Fred, the window's label is already Fred, and setting the control's name to its present state would force a redraw (which, while potentially useful in some cases, would be annoying in the one at hand), it would be perfectly reasonable to have the set-name method early-exit if the old and new names match.
–
supercatJan 13 at 18:15

In C programming where you have to manually clean-up there is a lot to be said for one-point return. Even if there is no need right now to clean something up, someone might edit your function, allocate something and need to clean it up before return. If that happens it will be a nightmare job looking through all the return statements.

In C++ programming you have destructors and even now scope-exit guards. All these need to be here to ensure the code is exception-safe in the first place, so code is well guarded against early exit and therefore doing so has no logical downside and is purely a style issue.

I am not knowledgeable enough about Java, whether "finally" block code will get called and whether finalizers can handle the situation of needing to ensure something happens.

C# I certainly can't answer on.

D-language gives you proper built-in scope-exit guards and therefore is well-prepared for early exit and therefore should not present an issue other than style.

Functions should of course not be so long in the first place, and if you have a huge switch statement your code is probably also badly factored.

C++ allows something called return value optmization which permits the compiler to essentially omit the copy operation that would normally occur when you return a value. However, under various conditions that's hard to do, and multiple return values is one of those cases. In other words, using multiple return values in C++ can actually make your code slower. This certainly hold on MSC, and given that is would be quite tricky (if not impossible) to implement RVO with multiple possible return values, it's likely a problem in all compilers.
–
Eamon NerbonneMay 20 '14 at 14:27

In Don Knuth's book about GOTO's I read him give a reason for always having the most likely condition come first in an if statement. Under the assumption that this is still a reasonable idea (and not one out of pure consideration for the speed of the era). I'd say early returns aren't good programming practice, especially considering the fact that they're more often than not used for error handling, unless your code is more likely to fail than not fail :-)

If you follow the above advice, you'd need to put that return at the bottom of the function, and then you might as well not even call it a return there, just set the error code and return it two lines hence. Thereby achieving the 1 entry 1 exit ideal.

Delphi Specific...

I'm of the mind that this is a good programming practice for Delphi programmers, although I don't have any proof. Pre-D2009, we don't have an atomic way to return a value, we have exit; and result := foo; or we could just throw exceptions.

If you had to substitute

if (true) {
return foo;
}

for

if true then
begin
result := foo;
exit;
end;

you might just get sick of seeing that at the top of every one of your functions and prefer

With newer Delphis, it could be abbreviated to if true then Exit(foo); I use often the technique to first initialize result to nil or FALSE respectively, then checking all the error conditions and just Exit; if one is met. The success-case result is then (typically) set somewehere at the end of the method.
–
JensGMay 21 '14 at 8:13

I'm personally a fan of guard clauses
(the second example) as it reduces the
indenting of the function. Some people
don't like them because it results in
multiple return points from the
function, but I think it's clearer
with them.

If this is run in a public method and SomeFunction() is a private method in the same class it could be ok. But if anyone else is calling SomeFunction() you would have to duplicate the checks there. I find it better to make each method take care of what it needs to do its work, no one else should have to know that.
–
Per WiklanderNov 11 '10 at 23:34

2

This is definitely the style that Robert C. Martin proposes in "Clean Code". A function should only do one thing. However in "Implementation Patterns", Kent Beck suggests, of the two options proposed in the OP, the second is better.
–
Scott WhitlockNov 13 '10 at 1:20

As others say, it depends. For little functions that return values, I may code early returns.
But for sizeable functions, I like to always have a place in the code where I know I can put something that will get executed before it returns.

The conditions on the top are called "preconditions". By putting if(!precond) return;, you are visually listing all preconditions.

Using the large "if-else" block may increase indent overhead (I forgot the quote about 3-level indents) and generate more bytecode.

Also, what language type is this question talking about? In Java and other strongly-typed languages, you cannot do early returns (but you can throw exceptions). In JavaScript, early returns may generate warnings.