Should I use an else when an if contains a return?

Question

I am following some examples in the book, and I noticed two different conventions for having various return conditions. Is there any difference between the two?

//example 1
if(someCondition)
{
   return (someValue);
}
return (someOtherValue);

//example 2
if(someCondition)
{
   return (someValue);
}
else
{
   return (someOtherValue);
}

Personally, I like the second example better, because it's more explicit, and I feel that it's more readable.

Answer 1

If your language contains a conditional operator I would recommend using that.

return condition ? ifTrue : ifFalse;

Answer 2

There is no difference other than the look. Both will return the same, regardless of which you choose.

Answer 3

Use whatever is the one you enjoy, and looks best with your code, or whatever your team understands most.

There isn't really a convention for this, as far as I am aware.

Answer 4

Personally I think that it always good to have one return statement in a method, otherwise code can be a bit hard to read and hence unmaintainable. so do something like

def returnVal = default;

if (cond) {
 returnVal = whatever
}

return returnVal

Answer 5

Personally the case where this comes up a lot is when you are eliminating invalid states from your function/method call. For example:

sqrt(x) {
    if(x < 0)
        return 0;

    answer=math;
        return(answer);

If you use elses, you can end up with a lot of really horrific nesting.

This also involves the "Single return" theory--lots of people think a method should only have one return statement. This also leads to mess in some cases.

In your specific example it's 50/50 IMO.

Do what you like, but I highly suggest giving the "Test immediately and return for simple cases" theory a try.

Answer 6

Avoiding else and taking advantage of early return can keep your code from becoming too deeply nested. Which example looks more readable?

function doStuff (thing) {
  if (thing.foo) {
    alert ("thing.foo is alive and well");
    if (thing.foo.bar) {
      alert ("thing.foo.bar is alive and well");
      if (thing.foo.bar.baz) {
        alert ("thing.foo.bar.baz is alive and well");
        // TODO: stuff with thing.foo.bar.baz
      } else {
        alert ("thing.foo.bar.baz doesn't exist!");
      }
    } else {
      alert ("thing.foo.bar doesn't exist!");
    }
  } else {
    alert ("thing.foo doesn't exist!");
  }
}

or

function doStuff (thing) {
  if (!thing.foo) {
    alert ("thing.foo doesn't exist!");
    return;
  }
  alert ("thing.foo is alive and well");
  if (!thing.foo.bar) {
    alert ("thing.foo.bar doesn't exist!");
    return;
  }
  alert ("thing.foo.bar is alive and well");
  if (!thing.foo.bar.baz) {
    alert ("thing.foo.bar.baz doesn't exist!");
    return;
  }
  alert ("thing.foo.bar.baz is alive and well");
  // TODO: stuff with thing.foo.bar.baz       
}

... I think the second is much more readable!