If/else or just if?

Today in a code review we had a statement like this:

Statement A

if (typeof val === 'string') {
    return 'A'
} else if (val === null || val === undefined) {
    return 'B'
} else {
    return val
}

And a suggestion was made to switch to the following:

Statement B

if (typeof val === 'string') return 'A'
if (val === null || val === undefined) return 'B'
return val

I won't tell you where we came out 😜, but which do you feel is a better way?

Was the suggestion to be concise and avoid if/else logic a good one, or was the original way better?

Comments

[JasperHorn]

I wouldn't do sample B as written because of the lack of braces for the if, which I'd always add (as well as spreading it out over more lines).

With that change, though, it's something that I do wonder about myself from time to time. With the exact sample given (where the else is just another return statement), I would usually go with sample A because the if-else-chain (which has a known mental model for me) nicely describes what happens. However, when the "else" portion is much larger (i.e. it's the "real implementation" after the corner cases have been taken care of) I'd probably go with something like sample B because it limits the levels of indentation that don't add information.

Ultimately, though, I'm not entirely consistent in this.

[apisurfer]

Statement B in general, because of indentation and context, even though you need to be careful about conditional cases. It's generally easier to follow through. IT REQUIRES MUCH LESS COGNITIVE LOAD because you don't need to think about all the cases that might have happened before or might happen afterwards if the specific condition is true or false. It's mostly used when you want to quickly check for a very bad usage of a function. Parameters that might break your logic. In that case you'd have 1-2-3 if clauses that check for bad params and immediately return/throw error. Alternative to that is to let the bad param checking leak to ALL of you NESTED conditional cases and make your code very hard to read. In that case you need to check for valid params inside of nested code and it's quite possible that similar code will occur a couple of times in the same function, which is crazy.
If you at any point end up thinking that it's not enough, that you need to take care of a bunch of complex cases, your function needs to be broken down to a smaller and composable functions that each do 1 thing and then revise all of the surrounding logic.

[Paul]

I do like the if/else case. Where sometimes when you are actually reading the codes it gives a better insight on the app and just like piece of orchestration or a music it makes sense (it rythms). It does sound and it is a good coding practice.

[Renato Byrro]

I find this version even more difficult to read...

[PatricNox]

Is the if and elseif statement about same scenario?

Yes.

Is the if and elseif statements not about same scenario OR is taking up more than 10 lines?

Switch case.

[Likhita]

I either user Ternary operator or the statement A type of syntax. But after reading this thread definitely will stick to statement B or ternary operator depending on the situation.

[Savas Vedova]

In Golang if the if statement is returning, the linter will complain about the else statement. In short,

if statement == true {
  return 
}

// This is the else statement

[Sebastian Vargr]

I tend to use a small map, or a ternary in these situations.

If’s without brackets breaks the bracket consistency for me, and ‘if else’ can be hard to read.

[Kevin Sullivan]

Why is val not already typed? It could be anything?!? I'd rather look into avoiding either of these situations and make sure the type of val is already known.

[The Jared Wilcurt]

There isn't much value in the else since the first conditional results in a return, however having it aids in readability in this case by grouping these two similar conditions. but having a final return outside of a condition is a best practice to ensure you don't mistakenly return undefined. The trailing else should be removed if it is just a return statement.

function (value) {
  if (typeof(value) === 'string') {
    return 'A';
  } else if (value === null || value === undefined) {
    return 'B';
  }
  return value;
}

Though in many cases, a map works better than a bunch of if/else's.

function (state) {
  const states = {
    indiana: 24,
    ohio: 36,
    florida: 19
  };

  return states[state] || 0;
}

Using the mapped approach eliminates any need for switch statements.

Also, don't abbreviate your code, there is no benefit gained from shaving off a few characters. Let the uglifier worry about that. Your goal should always be readability above all else until something is noticeably slow.