23

This code:

if( someCondition )
    return doSomething();

return doSomethingElse();

versus this code:

if( someCondition )
    return doSomething();
else
    return doSomethingElse();

In essence, they are the same, but what is best style/performance/... (if there is any non-opinionated part to the answer of course)? Also consider the case with multiple "if else's":

if( someCondition )
    return doSomething();
else if( someOtherCondition )
    return doSomethingDifferently();
//...
else
    return doSomethingElse();

Thanks!

Ken Bloom
  • 57,498
  • 14
  • 111
  • 168
rubenvb
  • 74,642
  • 33
  • 187
  • 332

5 Answers5

48

When you have multiple return statements in a function, this is referred to as "early return." If you do a Google search for "early return" you will find link after link that says it is bad.

I say nonsense.

There are two primary reasons and one secondary reason that people claim that early return is bad. I'll go through them and give my rebuttal in order. Keep in mind this is all my opinion and ultimately you have to decide for yourself.

  1. Reason: Early returns make it difficult to clean up.

    Rebuttal: That's what RAII is for. A well-designed program won't allocate resources in such a way that if a the execution leaves scope early those resources will leak. Instead of doing this:

...

int foo()
{
  MyComplexDevice* my_device = new MyComplexDevice;
  // ...
  if( something_bad_hapened )
    return 0;
  // ...
  delete my_device;
  return 42;
}

you do this:

int foo()
{
  std::auto_ptr<MyComplexDevice> my_device(new MyComplexDevice);
  if( something_bad_hapened )
    return 0;
  // ...
  return 42;
} 

And early return won't cause a resource leak. In most cases, you won't even need to use auto_ptr because you'll be creating arrays or strings, in chich case you'll use vector, string or something similar.

You should design your code like this anyway for robustness, because of the possibility of exceptions. Exceptions are a form of early return like an explicit return statement, and you need to be prepared to handle them. You may not handle the exception in foo(), but foo() should not leak regardless.

  1. Reason: Early returns make code more complex. Rebuttal: Early returns actually make code simpler.

It is a common philosophy that functions should have one responsibility. I agree with that. But people take this too far and conclude that if a function has multiple returns it must have more than one responsibility. (They extend this by saying that functions should never be more than 50 lines long, or some other arbitrary number.) I say no. Just because a function has only one responsibility, doesn't mean it doesn't have a lot to do to fulfil that resposibility.

Take for example opening a database. That's one responsibility, but it is made up of many steps, each of which could go wrong. Open the connection. Login. Get a connection object & return it. 3 steps, each of which could fail. You could break this down in to 3 sub-steps, but then instead of having code like this:

int foo()
{ 
  DatabaseObject db = OpenDatabase(...);
}

you'll end up having:

int foo()
{
  Connection conn = Connect(...);
  bool login = Login(...);
  DBObj db = GetDBObj(conn);
}

So you've really just moved the supposed multiple responsibilities to a higher point in the call stack.

  1. Reason: Multiple return points are not object-oriented. Rebuttal: This is really just another way of saying "everybody says multiple returns are bad, though I don't really know why."

Taken another way, this is really just an attempt to cram everything in to an object-shaped box, even when it doesn't belong there. Sure, maybe the connection is an object. But is the login? A login attempt isn't (IMO) an object. It's an operation. Or an algorithm. Trying to take this algorithm and cram it in to an object-shaped box is a gratuitous attempt at OOP, and will only result in code that is more complex, harder to maintain, and possibly even less efficient.

halfer
  • 19,824
  • 17
  • 99
  • 186
John Dibling
  • 99,718
  • 31
  • 186
  • 324
  • 3
    +10 even though it only lets me give 1 – Tergiver Nov 21 '10 at 17:30
  • Although I'm sure this can be seen as personal and ambiguous, I think the effort alone (and of course the fact I agree with every point) deserves the green checkmark! – rubenvb Nov 21 '10 at 19:29
  • But how does this anser the question? Aren't both styles in the question early returns? – xskxzr Aug 14 '21 at 11:38
9

A function should always return as soon as possible. This saves semantic overhead in unnecessary statements. Functions that return as soon as possible offer the highest clarity and the cleanest, most maintainable source code.

SESE-style code was good when you had to manually write to free every resource you had allocated, and remembering to free them all in several different places was a waste. Now that we have RAII, however, it's definitely redundant.

Puppy
  • 144,682
  • 38
  • 256
  • 465
5

It depends, I prefer the same as FredOverflow

return someCondition ? doSomething() : doSomethingElse();

If this is enough. If it isn't, I have wondered for a similar situation - if you have longer code - lets say 30-40 lines or more, should I put return; on a place, that's not necessary. For example, consider the following situation:

if( cond1 )
{
    if( cond2 )
    {
         // do stuff
    }
    else if( cond3 )
    {
        // ..
    }
} 
else if( cond4 )
{
    // ..
}
else
{
    //..
}

What I wondered was - should I put return; (in void function) at the end of each case - is this a good or bad coding style (as it doesn't matter if there's return; or not). Finally I decided to put it, because the developer, who'll read this code later, to know that this is a final state and there's nothing to be done anymore in this function. Not to read the rest of the code, if he/she's interesting only in this case.

Kiril Kirov
  • 37,467
  • 22
  • 115
  • 187
3

It's purely a matter of personal preference or coding standards. Personally, I would prefer a third variant:

return someCondition ? doSomething() : doSomethingElse();
fredoverflow
  • 256,549
  • 94
  • 388
  • 662
  • 4
    If the two function return types which are both convertible to the enclosing scope's return type, but are not identical to each other, the ternary operator will fail to compile. Then you'll need an ugly cast to fix it. I'd be in favor of the ternary here only when the condition involved is trivial and the types are unlikely to change (e.g. the called functions are in the standard library, not application code). – John Zwinck Nov 21 '10 at 17:02
0

It depends.

If you're following a convention of early returns to handle error cases then that's good, if you're just doing arbitrary things then it's bad.

The most serious problem with the code as presented is that you're not using curly braces. That means not quite understanding what clarity's about. If not for that I'd just answer your main question with "aim for clarity", but first you need to understand clarity. As first step on that road, start using curly braces. Try to make your code readable by others, so much so that it can be understood at-a-glance by someone else (or by yourself months/year later).

Cheers & hth.,

Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331
  • 1
    Well, I see curly braces for one-line if-else's as "unclear", and only use them when there are multiple lines in the conditional block, but that is a personal matter and not really relevant to my question... – rubenvb Nov 21 '10 at 13:39
  • 5
    -1: I hate it when people complain about omitting curly braces. It's a perfectly reasonable convention to omit curly braces if you indent your code properly, and everyone knows to go and add curly braces if they add a second line of code. – Ken Bloom Nov 21 '10 at 13:39
  • 8
    Personally, I find adding curly braces for no reason to REDUCE clarity as it adds visual noise and decreases the amount of functional code that will fit in a screen. – swestrup Nov 21 '10 at 13:41
  • @Ken: Yes, except your coworkers... – fredoverflow Nov 21 '10 at 13:41
  • 1
    @Ken: people find the most amazing things reasonable. it all depends on the context in one's head. others may, however, react. cheers & hth. – Cheers and hth. - Alf Nov 21 '10 at 13:43
  • 4
    -1 for claiming that not using curly braces in your prefered style shows a lack of understanding what clarity is about – Sebastian Negraszus Nov 21 '10 at 13:46
  • @Sebastian: it's a an extremely fundamental indicator, like a carpenter swinging a hammer in hollywood style (one knows the guy doesn't know what he's about). i'm sorry, but that's the way it is. cheers & hth., – Cheers and hth. - Alf Nov 21 '10 at 13:49
  • 1
    I always put curly braces, even if there's just one line. Suppose if you want to replace a function call(or operation) with two other operations and do this using `replace` to replace it everywhere in the code (such things happen), then there will be problems if there aren't braces. So, using `replace` will not help you, as you'll have to go through all cases again, to add braces, etc. But yes, it's a **matter of choice**, agree. – Kiril Kirov Nov 21 '10 at 13:52
  • @Fred: That being said, I prefer your way of writing it above in any case, so the point becomes moot for the purposes of answering the question. – Stuart Golodetz Nov 21 '10 at 13:53
  • @Alf: Just to clarify your point of view: would the style the braces use also be an indicator of the understanding of the meaning of clarity? I have seen many style guides suggesting different curly brace styles... – rubenvb Nov 21 '10 at 13:53
  • 2
    That's why i think curlies are evil. Grouping statemens by indentation is way better imo. – Johannes Schaub - litb Nov 21 '10 at 13:56
  • @ruben: Good question! I think the classical curly braces `{` and `}` are too easily overlooked, because they are only one character, so I prefer the trigraphs `??<` and `??>` or define macros `begin` and `end`... just kidding ;) – fredoverflow Nov 21 '10 at 14:00
  • 2
    @rubenvb: well, when someone is hung up on a particular indentation style and thinks that's serious, then it depends on the person's experience whether it's a sign of newbieness or something more serious. an easy check for general incompetence is whether the person also lacks a sense of humor. there were a number of psychological studies around 2000/2001 that concluded that generally incompetent folks seldom have a sense of humor (however, they can get by by memorizing the situations where other people laugh); check out e.g. wikipedia. cheers & hth., – Cheers and hth. - Alf Nov 21 '10 at 14:03
  • 3
    @Johannes: your preference for no-braces might make sense in academia. switch to Python. ;-) – Cheers and hth. - Alf Nov 21 '10 at 14:06