Why does this line doesnt work

        x > highNum ? highNum = x : y > highNum ? highNum = y : highNum = highNum

In this case this line is in a loop and x and y is different everytime. I tried to find the highest number at the end and thought this would work. In my mind this reads as: If x is higher than high num highnum should get assigned the value of x if not. is y bigger? if yes y should be the new highnum. if not. dont change high num

Why does this line doesnt work

        x > highNum ? highNum = x : y > highNum ? highNum = y : highNum = highNum

In this case this line is in a loop and x and y is different everytime. I tried to find the highest number at the end and thought this would work. In my mind this reads as: If x is higher than high num highnum should get assigned the value of x if not. is y bigger? if yes y should be the new highnum. if not. dont change high num

• javascript
• node.js
• conditional-operator

• 1 If you're not insistent on using ternary operators, highnum = Math.max(x, y, highNum) is the best way to do this. – lejlun Commented Jul 31, 2021 at 17:41
• 1 "doesn't work" is not such a clear problem description. Describe what goes wrong. Better even, include a concrete example in a runnable snippet, highlighting the problem. – trincot Commented Jul 31, 2021 at 17:43

5 Answers 5

short answer:

Yes, they are.

long answer:

Yes, but you should be concerned about readability too. You code does exactly what you are expecting it to do, but other devs (and maybe you in the future), could have a problem understanding that, so I'd strongly advice you to never use nested ternary operators, and only use them when it makes more sense then a simple if else statement. And if even after all this you still wanna use it, at least add a ment explaining what it does. ex:

  let highNum
  for(let line of lines){
     const {x, y} = line; 
     // use bubble sort to find the highest number
     x > highNum ? highNum = x : y > highNum ? highNum = y : highNum = highNum
  }
  

edit: Also, this is also not correctly finding the highest number, as said by "trincot"

Yes, but you'll require brackets, mostly for readability:

(x > highNum) ? (highNum = x) : ((y > highNum) ? (highNum = y) : (highNum = highNum));

In your case, it seems you're better off splitting it into multiple statements to prevent confusion:

if (x > highNum) {
    highNum = x;
} else if (y > highNum) {
    highNum = y;
}

although that doesn't fit in a single expression, but perhaps that's a sign of your code getting a bit too plex/unreadable.

If you're solely looking for the highest number, perhaps Math.max is all you need, i.e. Math.max(x, y, highNum).

There is a potential high value that you could miss: when x > highNum, but also y > x, you will not see that y is really the highest, as the expression will already have decided that highNum should get the value of x.

You can do this quite simple with Math.max:

highNum = Math.max(x, y, highNum);

If x is higher than high num highnum should get assigned the value of x if not. is y bigger? if yes y should be the new highnum. if not. dont change high num

That would be:

highNum = (x > highNum) ? x : ((y > highNum) ? y : highNum)
           [condition] ?[true]:         [false]
                              ([condition] ? [true] : [false]) // Evaluated if the first condition was false

The structure is variable = [condition] ? [value for true] : [value for false]

And you can have a nested ternary instead of a [value] using additional parenthesis.

This is the same as:

if(x > highNum) {
  highNum = x
} else if(y > highNum) {
  highNum = y
} else {
  highNum = highNum // Quite useless... That is for explanation purpose ;)
}

You have to consider y>x instance as well.

highNum = (x > y) ? (x > highNum ? x : highNum) :(y > highNum ? y : highNum);

The above is equal to the below:

if (x>y) {
    if (x>highNum) {
        highNum = x;
    } else {
        highNum = highNum;
    }
} else { //covers both instances of y>x and x=y
    if (y>highNum) {
        highNum = y;
    } else {
        highNum = highNum;
    }
}