DEV Community

Discussion on: Rethinking JavaScript: The if statement

View post
Collapse
 
mortoray profile image
edA‑qa mort‑ora‑y •

This is terrible use of the ternary operator. You're mixing multiple imperative statements into a logical expression value.

  return isCustomerValid(customer)
    ? database.save(customer)
    : alert('customer is invalid')
Enter fullscreen mode Exit fullscreen mode

It's hard to image how database.save and alert resolve the same logical type, even if the actual type happens to be the same.

You're hiding an imperative conditional inside this ?: syntax. When I see ?: I think of conditional evaluation with multiple possibilities. I would absolutely not think of a branching imperative flow.

This hurts readability and is not clean code.

Collapse
 
joelnet profile image
JavaScript Joel •

This is a contrived example that was made up to demonstrate specific functionality. The biggest problem with contrived examples is that there are always better ways to solve the problem. A perfect example is the validation, there are much better ways to validate inputs (but I am not writing about how to validate inputs).

The goal of the article is also not to demonstrate how to save things to the database. It is to demonstrate a ternary. So, yes this is a terrible example of how to save something to a database. And actually, the code itself doesn't even make sense! I mean why is a database.save function, which suggests this might be a node application co-exist with an alert which is only available in the browser? The whole things is nonsensical! But I didn't think anyone would get caught up on this.

I could have created an example that made more sense, something like...

const saveCustomer = customer =>
  doesCustomerExist(customer)
    ? database.update(customer)
    : database.insert(customer)
Enter fullscreen mode Exit fullscreen mode

... but then I could continue to run into problems when it could be suggested to write like this...

const saveCustomer = customer =>
  database[doesCustomerExist(customer) ? 'update' : 'insert'](customer)
Enter fullscreen mode Exit fullscreen mode

... which is not what I wanted to demonstrate.

Maybe I should leave more up to the imagination with something like this...

const doSomething = thing =>
  checkTheThing(thing)
    ? doOneThing(thing)
    : doOtherThing(thing)
Enter fullscreen mode Exit fullscreen mode

It's clearly a failure in the article that is detracting from the overall message.

Also, there is nothing imperative OR functional about this expression. It is simply an expression.

I am recommending the ternary as one of many ways to replace the if statement, which is definitely exclusively imperative.

I understand your opinion regarding the readability and I have a differing opinion; I absolutely prefer this syntax. There is nothing complicated about this code, it's actually incredibly simple, though you may be less exposed to this (because everyone is used to imperative codebases).

Even the basic examples on Mozilla's ternary page are in alignment with this article developer.mozilla.org/en-US/docs/W...

I'll try to work on better examples for future articles, I do agree that mixing database.save and alert is shitty and takes far too much attention away from the article.

Cheers!

Collapse
 
mortoray profile image
edA‑qa mort‑ora‑y •

It's this pattern I disagree with:

const doSomething = thing =>
  checkTheThing(thing)
    ? doOneThing(thing)
    : doOtherThing(thing)

In particular I disagree with the do... functions. Ternary is meant for evaluation, such as:

const result = thing =>
  checkTheThing(thing)
    ? evalOneThing(thing)
    : evalOtherThing(thing)

There is a subtle, but important difference. This clearly results in a value that we want to use, whereas the do... form is branching to perform some action, it's unclear what result is desired.

Consider if the functions don't return anything. Then you'd have:

var voidResult = check ? voidFoo() : voidBar()

This won't even be allowed in many statically type languages, as they can't evaluate void. It also means the result cannot be used, implying that the expression itself isn't being used. Logically if a value isn't being used you'd expect you can skip the evaluation of it. But instead you have code where despite not using the result, you require the code to be evaluated nonetheless.

Moreso, you're implicitly relying on a short-circuiting of this expression, which may not actually be case in all languages. Consider that operators are short-forms for functions, in this case you have a function like this:

ternaryOp( cond, eval1, eval2 )

But you can't translate your code to this:

var result = ternaryOp( checkTheThing(thing), evalOneThing(thing), evalOtherThing(thing) );

This always evaluates both of the arguments, which is not correct for your code. It's failed a logical substituion test.

The form that I would allow is this:

var result = (checkTheThing(thing) ? evalOneThing : evalOtherThing)(thing);

This is okay because the ternary is a proper evaluation. It doesn't require compiler short-circuiting, it isn't hiding any code branch. It works with void return as well (dropping the assignment).

Thread Thread
 
joelnet profile image
JavaScript Joel •

var voidResult = check ? voidFoo() : voidBar()
This won't even be allowed in many statically type languages, as they can't evaluate void

That is absolutely correct. Though this article is exclusive to JavaScript. There is quite a bit of JavaScript that doesn't translate over to other languages.

You can't even do this in C#

// ERROR
var result = voidFoo();
Enter fullscreen mode Exit fullscreen mode

You still always need to follow order of operations. For example:

// this function...
const result = ternaryOp( checkTheThing(thing), evalOneThing(thing), evalOtherThing(thing) );

// ...is equivalent to
const val1 = checkTheThing(thing)
const val2 = evalOneThing(thing)
const val3 = evalOtherThing(thing)
const result = ternaryOp(val1, val2, val3)
Enter fullscreen mode Exit fullscreen mode

...when compared to...

// this thing...
const result =
  checkTheThing(thing)
    ? doOneThing(thing)
    : doOtherThing(thing)

// ...is equivalent to
const val1 = checkTheThing(thing)
let result
if (val1) {
  result = doOneThing(thing)
} else {
  result = doOtherThing(thing)
}
Enter fullscreen mode Exit fullscreen mode

Moreso, you're implicitly relying on a short-circuiting of this expression

For sure. Though again, this article is exclusively about JavaScript. I'm still not completely sure this is short-circuiting, I have only heard that in reference to the && and || operators.

Another example:

const result = obj ? obj.name : 'unknown'
Enter fullscreen mode Exit fullscreen mode

The reason why this works is because both branches are not evaluated. This is not exclusive to my examples, but fundamental to how the ternary operator works in JavaScript.

The ternary works differently than VB.net's IIf operator, which evaluates both sides (this is close to your example). VB.net later added the ability to do Dim foo as String = If(bar = buz, cat, dog), which will only evaluate one side.

I would expect an understanding to how any operator works in your language prior to using it to be mandatory. JavaScript's ternary operator works in a fairly common way, for example, it works the same way in C and Java.

I will consider updating the article to clarify this is JavaScript as well as more examples on how the fundamentals of the ternary work.

All very good feedback.

Cheers!