Javascript: Nested ternaries aren't bad, just misused

Created on 1 Feb 2018  Â·  30Comments  Â·  Source: airbnb/javascript

The argument against nested ternaries is that they can make code more difficult to understand. But they can make code easier to understand if they are wielded properly. Instead of banning nested ternaries, require that they are formatted such that each line has a single condition and a single result, with the catch-all at the end:

// bad
const foo = maybe1 > maybe2
  ? "bar"
  : value1 > value2 ? "baz" : null;

// good
const foo =
  maybe1 > maybe2 ? "bar"
  : value1 > value2 ? "baz"
  : null;

This is similar to a switch statement that always breaks. With this pattern, it's impossible to get lost in nested contexts. Once you've ruled out a line applying to the case you're considering, you can safely forget it. It's easier to understand than the solutions provided by the style guide right now, and also saves a memory allocation.

Here are some other examples where this pattern improves code:

// bad
let result;
if (operator === ‘+’) {
  result = left + right;
} else if (operator === ‘*’) {
  result = left * right;
} else if (operator === ‘-’) {
  result = left — right;
} else {
  result = left / right;
}

// good
const result =
  operator === ‘+’ ? left + right
  : operator === ‘*’ ? left * right
  : operator === ‘-’ ? left — right
  : left / right;
  • Less code.
  • Prefers const over let.
  • Avoids side effects (i.e. assigning value of variable outside block scope).
// bad
if (conditionA) {
  outcome1();
} else if (conditionB) {
  if (conditionC) {
    outcome2();
  } else {
    outcome3();
  }
} else {
  outcome4();
}

// good
conditionA ? outcome1()
: conditionB && conditionC ? outcome2()
: conditionB ? outcome3()
: outcome4();
  • Less code.
  • Less cognitive load.
question

Most helpful comment

First of all, things that are easily misused / are easy to improperly wield are bad.

A switch statement isn't a good idea to use ever either, because you can easily forget to always break. In the future, switch statements will be discouraged by this guide as well. (Separately, memory allocation is irrelevant; JS is a memory-managed language, which means it's not the developer's business to think about it).

The pattern you're suggesting still has the same issues with intuition about precedence, so it doesn't address the problem. In particular, I don't agree that any of your "good" examples are easier to read; you're also abusing && for control flow. The if/else example is far better.

All 30 comments

First of all, things that are easily misused / are easy to improperly wield are bad.

A switch statement isn't a good idea to use ever either, because you can easily forget to always break. In the future, switch statements will be discouraged by this guide as well. (Separately, memory allocation is irrelevant; JS is a memory-managed language, which means it's not the developer's business to think about it).

The pattern you're suggesting still has the same issues with intuition about precedence, so it doesn't address the problem. In particular, I don't agree that any of your "good" examples are easier to read; you're also abusing && for control flow. The if/else example is far better.

First of all, things that are easily misused / are easy to improperly wield are bad.

This is kind of like saying JavaScript is bad because it is easily misused. That, I think, is the purpose of a style guide -- to make code less unwieldly. I propose enforcing using ternaries in a way which is not bad, which is again, I think, the purpose of a style guide.

A switch statement isn't a good idea to use ever either, because you can easily forget to always break.

Please notice I said this is similar to a switch statement _which always breaks_. The pattern I propose is not a switch statement. It has the good features of a switch statement, but not the bad ones. (For the record, I would not encourage the use of switch statements for the same reasons.)

The pattern you're suggesting still has the same issues with intuition about precedence

Would you mind explaining what you mean about this? I don't see how what you're saying is true.

I don't agree that any of your "good" examples are easier to read

Please consider that your opinion is being influenced by familiarity bias. I feel very strongly that my examples are easier to read.

you're also abusing && for control flow.

I don't understand what you mean by this. This is what && exists for. There are numerous examples of it in the style guide, such as:

// good
if (foo === 123 && bar === 'abc') {
  thing1();
}

The if/else at least should be refactored to remove the nesting in the same way to reduce cognitive load:

if (conditionA) {
  outcome1();
} else if (conditionB && conditionC) {
  outcome2();
} else if (conditionB) {
  outcome3();
} else {
  outcome4();
}

With nested if's you must keep in your mind the inner context and each outer context. But in either case, the ternary statement take less time to write, less time to read, and has less surface area in which bugs can hide.

Separately, memory allocation is irrelevant; JS is a memory-managed language, which means it's not the developer's business to think about it

Just for the record, I work at a JavaScript game studio, and it absolutely is the business of all of our engineers to think about memory allocation.

Can you think of a scenario where nested ternaries are easier to read than a function that returns at the first met condition?

The function might be a memory problem for your gamedev scenario, but an argument about a sub-ideal runtime environment doesn't address ideal style It can merely justify deviation from ideal style, until the technical limitation is overcome.

@okaybenji , it's pretty much against this style guide (i couldn't quote the actual place, but I'm sure it is :-) ) to use anything of the form "x && functionCall()" or "x || functionCall()", which is essentially what your third example does, if I'm reading it correctly. I suspect there's something I'm not thinking of here when lining up several ternaries like that, as well, but I'd probably have to look at it a little longer than I have time for right now. That said, if it works out like it appears to, I like it for variable declarations. Also with THAT said, ljharb and I disagree pretty frequently, and that's OK, because it's his style guide, not mine. I use a modified version of this style guide, and I really enjoy the educational discussions that go on in the Issues here on this one. Sometimes the discussions even help to change my mind on some things.

If tight memory usage is a significant concern of yours, as it probably needs to be with game-type applications, you can probably throw a large portion of this style guide straight out the window. As ljharb stated his views on memory management in Javascript, it's pretty fair to say that your concerns will often be diametrically opposed to ljharb's concerns. His primary concern, or very very high up on the list, is having the code be readable, and execution speed and other efficiencies are way down the list.

(as an aside, if you're looking for the fastest performing code out there, you might want to benchmark all the native iterator functions -- i haven't done it in about a year, but last time I did bench all the native iterator functions, they were significantly slower than using for/while, which makes no sense to me, but i guess there was, at the time, some pretty massive overhead in setting up function calls, or whatever)

See #1729; the guide now explicitly warns against abusing value selection operators for control flow.

In general, when memory management is a concern (as is often the case with games) one must indeed throw best practices out the window - as the priority there isn't good code, but rather fast code (which tend to overlap often, but not always).

@mk-pmb Sure, you could write:

const getResult = (operator) => {
  if (operator === ‘+’) {
    return left + right;
  }
  if (operator === ‘*’) {
    return left * right;
  }
  if (operator === ‘-’) {
    return left — right;
  }
  return left / right;
};

const result = getResult(operator);

But this code is still much better:

const result =
  operator === ‘+’ ? left + right
  : operator === ‘*’ ? left * right
  : operator === ‘-’ ? left — right
  : left / right;

It's easier to read and understand, takes less time to read and maintain, and has less surface area in which bugs can hide. The fact that it's also much more performant is just the cherry on top.

@ericblade @ljharb I appreciate your feedback, but none of my examples do what you're saying. This issue has been closed for a reason that doesn't even apply.

conditionA ? outcome1()
  : conditionB && conditionC ? outcome2()
  : conditionB ? outcome3()
  : outcome4();

is exactly the same as

if (conditionA) {
  outcome1();
} else {
  if (conditionB && conditionC) {
    outcome2();
  } else {
    if (conditionB) {
      outcome3();
    } else {
      outcome4();
    }
  }
}

Nowhere does anything resembling x && functionCall() or x || functionCall() occur. Additionally, even if you did have a reason to dislike this example, the first two examples still stand.

(Just as an aside, I'm with @ljharb on erring on the side of readable and maintainable code over worrying about performance. And I love the Airbnb style guide, I just noticed a couple of places it could be improved with ideas I had not seen elsewhere that I felt needed to be fairly evaluated.)

But this code is still much better: […] It's easier to read and understand, takes less time to read and maintain,

Looks like a matter of what you're used to, then. For me, those chained ternaries are harder to read. They also evoke self-doubt about whether I remember all of the operator precedence rules correctly, which slows down my work or even prevents me working on that part when my brain isn't at full energy.

I consider some table-style layout way more intuitive:

const result = (() => {
  if (operator === '+') { return left + right; }
  if (operator === '*') { return left * right; }
  if (operator === '-') { return left - right; }
  return left / right;
})();

Would be even clearer with a real table lookup:

const res2 = ({
  '+': (a, b) => (a + b),
  '*': (a, b) => (a * b),
  '-': (a, b) => (a - b),
  '/': (a, b) => (a / b),
})[operator](left, right);

Free debug bonus: % and ^ throw an exception instead of just using division.

Maybe there could even be some performance gains in factoring out that anonymous table into one object shared between all invocations of the function that uses it.

@mk-pmb

Looks like a matter of what you're used to, then. For me, those chained ternaries are harder to read. They also evoke self-doubt about whether I remember all of the operator precedence rules correctly

What I'm proposing isn't even something I'm used to, (or as far as I know, that anyone is used to) just something that occurred to me a couple of weeks ago and instantly seemed better than what everybody else was doing.

The beauty of it is you don't have to remember anything at all. You simply find the first line that has a true condition, and on that line you'll find your result. If you've made it all the way to the end without encountering a true condition, there at the end, you'll find your result.

The idea was inspired by reading about other languages with similar control structures, and this article by Eric Elliot which in my estimation is very good, but still gets it wrong.

I appreciate the examples you've provided, but anything that looks like })()); is already very difficult to read. The second example is slightly better, but there's still a lot more to parse, just more densely packed.

The beauty of it is you don't have to remember anything at all.

How do you derive that claim? My quality criteria demand that I can trust the code that I ship in my libraries. The chained ternaries look like a footgun to me, so I'd have to understand them in order to ensure they do the thing that I expect.

anything that looks like })()); is already very difficult to read.

Indeed. In my code I'd use a named IIFE or maybe even a regular named function. The exampled was meant to lean towards the ternaries' terseness.

If the 2nd example was a real use case, I'd probably factor out the table as some library module, then write

import { bySymbol as mathOperFuncs } from 'math-oper-funcs';
const res2 = mathOperFuncs[operator](left, right);

@mk-pmb

How do you derive that claim?

If you take a look at my original proposal re: nested ternaries, it is to:

require that they are formatted such that each line has a single condition and a single result, with the catch-all at the end

This could be enforced however you choose, such as linting during intergration testing.

So I'd have to remember the formatting requirement. In shared projects I'd also have to verify no other collaborator messed it up into misleading code. Nah, still on Crockford's side about footguns.

One conflict your proposal has is that we now require (in the next release of the linter config) that operators begin a line in multiline constructs. In other words, the ? and the : must never end a line.

(also, in any multiline ternary, each of the three parts must go on its own line)

@mk-pmb Please remember that this is a style guide. You have to either remember all of its requirements (which I'm pretty sure no one does) or rely on a linter (which I'm pretty sure everyone does).

@ljharb

One conflict your proposal has is that we now require (in the next release of the linter config) that operators begin a line in multiline constructs. In other words, the ? and the : must never end a line.

Sorry, I'm really confused... my proposal follows this requirement.

Not the second part; you have ?’s embedded in individual lines, instead of always starting the line (in a multiline ternary)

@ljharb

(also, in any multiline ternary, each of the three parts must go on its own line)

This is exactly what makes nested ternaries confusing. Would you mind explaining the reasoning behind this decision?

In my opinion, that is exactly the opposite way I would want to read multiline constructs -- I want the operator at the end of the line, so that I know reading the first line, that it continues into the next line. It would otherwise feel like reading English with punctuation belonging to the next sentence instead of the one it applies to.

Think of it more like a bulleted list than sentences.

@okaybenji no, they’re confusing because it’s multiple conditionals chained together. The formatting can’t address that.

@ljharb

they’re confusing because it’s multiple conditionals chained together. The formatting can’t address that.

Okay, I can see that this is where we have a difference of opinion. I feel very strongly that the formatting I propose not only addresses it but totally resolves it. Anyhow, thanks for taking a look.

@ericblade @ljharb If you don't mind, I would still like to understand why you thought my code was violating the rule against x && functionCall()

Probably because you used function calls in

// good
conditionA ? outcome1()
: conditionB && conditionC ? outcome2()
: conditionB ? outcome3()
: outcome4();

whereas this style guide seems to¹ disencourage possible side effects in value picking expressions.

(¹ haven't fully memorized it yet. :P)

@mk-pmb It seems not.

  1. They specifically said I was abusing &&
    2) My example is no more a "value picking expression" than a simple function call, such as functionCall();

I read that example to basically be the same as
conditionA && outcome1() || (conditionB && conditionC) && outcome2() ....
You're using what is normally used as an assignment, instead as flow control.

@ericblade Thanks for the response. I appreciate the insight.

@okaybenji I agree with your assessment that ternaries when properly used are a great tool for code legibility. the real issue is stoping the linter from giving the user a lengthy error message because somebody does not like the use of nested ternaries. God forbid one has to update the tool...

Was this page helpful?
0 / 5 - 0 ratings

Related issues

CWSpear picture CWSpear  Â·  167Comments

ljharb picture ljharb  Â·  139Comments

martpie picture martpie  Â·  28Comments

PandaWhisperer picture PandaWhisperer  Â·  44Comments

JacksonGariety picture JacksonGariety  Â·  43Comments