Is an "if(...) return ...;" without "else" considered good style? [closed]

Question

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!

Answer 1

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.

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

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

Answer 2

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.

Answer 3

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.

Answer 4

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

return someCondition ? doSomething() : doSomethingElse();

Answer 5

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