49

I am using es-lint to clean up the errors in my code. I have come across this error:

Unnecessary 'else' after 'return'. (No-else-return)

} else {

I have always used else statements after a return. Is there something I may be overlooking?

if (cctot <= 3 && cctot > 0) {
    alert('Credit under $3.00 not allowed');
    return Number.MIN_SAFE_INTEGER; // important to return 0 so we can check for these conditions for validation
} else {
    cctot *= -1;
}
return precise(cctot);
Elias Zamaria
  • 96,623
  • 33
  • 114
  • 148
David Brierton
  • 6,977
  • 12
  • 47
  • 104
  • Please post complete snippet, only then we will he able to help – Talha Abrar Oct 22 '17 at 14:51
  • 16
    If the `return` is executed, the function exits. Using an `else` there is redundant and misleading. – Pointy Oct 22 '17 at 14:52
  • 1
    https://eslint.org/docs/rules/no-else-return – JLRishe Oct 22 '17 at 14:56
  • 16
    @Pointy _Using an else there is redundant and misleading._ Huh? Guiding developers through your conditional logic by providing all branches is misleading? The less code, the better? I don't think so! –  Oct 22 '17 at 15:22
  • 1
    @ftor well me and es-lint apparently disagree! – Pointy Oct 22 '17 at 15:36
  • 1
    @pointy There are good abstractions for `if` conditions: Higher order functions, tagged unions. If your code is based on `Boolean` bits, at least be honest and show all cases. Abstracting from `else` branches because they are technically unnecessary is a false understanding of cleverness. I don't care about es-lint. However, disagreeing with you is a new experience for me. So, no offense! –  Oct 22 '17 at 15:58
  • 10
    Let's not forget that ESLint is only there to help us enforce our opinionated conventions. It's not up to ESLint to help the coder determine the difference between right and wrong or "best practices" in the JavaScript community, its merely a tool to help teams and individuals establish their own conventions. In this case, whoever setup the ESLint configurations decided that` else-returns` are redundant, but that doesn't mean they are wrong if someone prefers to use them. – radiovisual Oct 25 '17 at 13:53
  • See also https://stackoverflow.com/questions/1780384/should-if-statement-always-have-an-else-clause and over there on https://softwareengineering.stackexchange.com/questions/375297/if-and-else-or-if-and-return – cachius May 04 '23 at 14:05

4 Answers4

66

What that is basically saying is that the else part of the if statement is unnecessary if there is a return in the if part. Something like this is what it expects: 

if (cctot <= 3 && cctot > 0) {
      alert('Credit under $3.00 not allowed');
      return Number.MIN_SAFE_INTEGER; // important to return 0 so we can check for these conditions for validation
}
cctot *= -1;

In general, this:

if (condition) {
  return something;
} else {
  // do another thing
}

return anotherThing;

is similar to:

if (condition) {
  return something;
}

// do another thing
return anotherThing;

After the if with a return statement, there is no need for the else part as the code below the if will only run when the condition stated is not fulfilled.

codejockie
  • 9,020
  • 4
  • 40
  • 46
  • 3
    This is totally the way to go. – Robert Moskal Oct 22 '17 at 14:57
  • That version **has** the `else`. What does the version with the "parsing error" look like? – Pointy Oct 22 '17 at 15:05
  • 1
    @DavidBrierton: You may want to submit that code to http://codereview.stackexchange.com since there's a good bit of redundancy that can be eliminated. – llama Oct 22 '17 at 15:06
  • @DavidBrierton It doesn't even make sense to use an `else` there. Apparently you want to flip the sign of the credit value right before you return it at the end of the function. This has nothing to do with the `if` that that `else` is attached to. – JLRishe Oct 22 '17 at 15:12
  • @DavidBrierton If you remove an `else {` you need to remove the closing brace at the end of the block. Is that not obvious? – JLRishe Oct 22 '17 at 15:13
  • Would it be better if I be explicit and add the additional 2 or 3 lines for `else`, or remove it all together? –  Feb 10 '21 at 19:54
  • @StackGeek you should remove it altogether if you have a `return` in the `if { }` block otherwise add it. – codejockie Feb 11 '21 at 13:52
  • 0 How is this wrong by having a return on if when it was intended? ``` if (something === true) { return doSomething(); } else if (anotherThing === true) { return doSomethingElse(); } # add more else conditions as necessary // if neither is true doDefaultThing(); ``` why would it be wrong to force termination of the function by returning on each condition? The use case for example is that we want to prevent what is a default implementation and force termination when certain conditions are met. – Adonis Lee Villamor Mar 22 '21 at 01:52
  • @AdonisLeeVillamor, in your use case you can simply just do `if (something === true) return doSomething()` `if (anotherThing === true) return doAnotherThing();` and so on without need to wrap it in else block – codejockie Mar 22 '21 at 08:17
  • what if there are multiple if-else conditions? – Derek Jin Nov 23 '21 at 11:55
  • @DerekJin same rule will apply assuming all the blocks use `return` – codejockie Nov 23 '21 at 11:57
33

It's a code style preference. You don't need the else and instead can put the else code directly below the if. This is because if the if succeeds, that's the end of the function, so the else code will never be reached anyway.

So this:

if (condition) {
  return foo;
} else {
  // do bar
}

return baz

is equivalent to this:

if (condition) {
  return foo;
}

// do bar

return baz

This style seems to vary in different programming communities. Go developers will nearly always omit the else, while I've seen more JS devs include it.

While I prefer to leave off the else, it is again purely a matter of preference. Don't let it worry you too much. People may get dogmatic about this kind of thing, but it's really not that important.

llama
  • 2,535
  • 12
  • 11
  • 3
    I think @llama's advice is spot on, especially when then the if block is more than a few lines. Then the else condition can really help with clarity. – craft Jan 24 '19 at 01:07
  • 1
    If you disassemble the resulting code with the 'else' you'll see a redundant 'jmp' instruction as well. – StureS Oct 25 '22 at 08:14
9

While the rule correctly points out that the else block is unnecessary, and it is a style preference, I would add additional considerations for readability and most importantly scanability.

For the developer writing this code, and to the machine interpreting it, it may be a style point and that's it. But for the developer who needs to fix a bug, enhance a feature, do a code review, etc. the ability to quickly scan through the code and see the else blocks helps to identify branches of logic.

In a few lines of isolated code it is easy to see the intent, but among hundreds of lines of code having if else blocks can serve as useful identifiers, much like other common visual practices like indentation, line breaks, and naming conventions.

jhovanec
  • 796
  • 8
  • 8
1

The return statement stops/terminates the current function. It's just saying that there's no need for 'else' since the execution of the function already stopped and if the 'if' condition doesn't succeed, it will still run any code underneath it.

As for best practice, I won't say it's a big of a deal always but with the code in your example, I won't use the else clause because it's simply not needed. I think it's good to understand what's happening under the hood and the reason behind best practices and not just following them.

dev mamba
  • 676
  • 7
  • 10
  • 5
    Good code transparently reflects developer's intention and is meant to read by engineers first, not by machines. It's obviously bad to skip `else` just "because it's not needed" (by engine). If you don't skip "else" when you talk about what your code does it's already a good reason not to remove "else" from the code. – meandre Jul 01 '20 at 14:06