Early returns (no else after return)

[rcky]

I prefer to use early returns but i know @eventEmitter has a really strong opinion on this (which contradicts the current coding guidelines). I think we should clarify that and - if necessary - adjust the linting i.e:

Early Return (no else after return)

if (condition) {
    return ;
}
// implicit else code, guarded by the return

Pro: easier to read, less indentation Con: if the return is removed (e.g. during refactorings) both branches will be executed

Explicit Else Clause

if (condition) {
    return ;
} else {
    // explicit else code
}

Pro: Safely protected branches (important in cases of low test coverage) Con: Harder to read, based on the assumption that the programmer introduces bugs

In general we could add some other metrics to avoid the code getting less readable (ie. nesting & cyclomatic complexity of methods)

@fxstr @sandrokneubuehl @eventEmitter @lkappeler feel free to comment

Readings

• https://softwareengineering.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement
• https://news.ycombinator.com/item?id=16678209

[linaGirl]

@rcky thanks. i indeed have a strong opinion on this one. I had to fix several nasty bugs because of the implicit else statement. Counting on the test coverage is a bit bold :)

[lkappeler]

I prefer the the variation without the else statement, for me it make the code more readable and easier to understand as well as the code has less branches. I think it's also very handy if it comes to multiple error cases you want to handle.

For Example:

if (error condition 1) {
  return new Error('error one')
}

if (error condition 2) {
  return new Error('error two')
}

// the main function logic if the input was valid
return result;

vs.

if (error condition 1) {
  return new Error('error one')
} else {
  if (error condition 2) {
    return new Error('error two')
  } else {
    // the main function logic if the input was valid
  }
}

return result;

The example above could be handled by an else if in this simple case but in general I think the code has more unnecessary branches with the else method.

[rcky]

I'd throw Errors anyway ;) But its exactly the same case actually! @eventEmitter to be consistent, the same rule should apply for throw

[linaGirl]

Thats correct @rcky . Lets vote: :+1: for early returns, :-1: for the explicit else case :)

@fxstr @lkappeler @rcky

[rcky]

@eventEmitter What about leaving it open to the developer and qualify the corresponding rule as a warning?

[linaGirl]

@rcky i'm completely ok with using early returns after this vote :) please keep the blocks with early returns short and consider using an else statement if there are many lines in it.