Discuss
  • [NaN].indexOf(NaN) === -1 VS Object.is(NaN, NaN)

[NaN].indexOf(NaN) === -1 VS Object.is(NaN, NaN)

# Andrea Giammarchi (13 years ago)

Hello everybody, I am not sure about the status of this subject but I find quite inconsistent the fact we cannot trust Array#indexOf when it comes to NaN ... I am updating my es6-collections shim accordingly with what Paul Millr said: my shim does not consider NaN as a valid key, being unable to find a match through indexOf, so each NaN key will result into a new one for both Map and Set ( broken WeakMap too )

I wonder if in the next future Array#indexOf will use internally Object.is to compare values or if I have to use my own function to loop over all values and do this kind of comparison.

Any thought on this will be appreciated and apologies if this has been discussed already ( so a quick answer on "how it's going to be" would be more than enough )

Best , Andrea Giammarchi

# Mark S. Miller (13 years ago)

Please replace your use of indexOf in your collection implementations instead. If indexOf is more efficient than one written in JS using Object.is, then use the fast one when the argument is anything other than NaN, 0, and -0.

# Andrea Giammarchi (13 years ago)

This is basically what I am doing except I find Array#indexOf as it is pointless ... or better, something that should be fixed in the next version of ECMAScript.

Right now I am feature detecting the inconsistent behavior ... but a feature detection that will never possibly return true is also pointless ... thanks in any case :-)

br

# Andrea Giammarchi (13 years ago)

just in case ... this is my internal function

// var is = Object.is || properShim // var indexOf = [].indexOf || shortShim // var i; function betterIndexOf(value) { if (value != value || value === 0) { for (i = this.length; i-- && !is(this[i], value);); } else { i = indexOf.call(this, value); } return i; }

maybe somebody else should implement something similar to avoid N Object.is(a, b) checks within the loop ...

# Herby Vojčík (13 years ago)

Andrea Giammarchi wrote:

just in case ... this is my internal function

// var is = Object.is || properShim // var indexOf = [].indexOf || shortShim // var i; function betterIndexOf(value) { if (value != value || value === 0) { for (i = this.length; i-- && !is(this[i], value););

I always has the impression that indexOf is firstIndexOf not lastIndexOf... sometimes it matters.

# Andrea Giammarchi (13 years ago)

it doesn't in my case ... arrays are "sandboxed" so none can access them and set twice the same value ;-)

anyway, that was a proof of concept, not a fully implemented indexOf with strict comparison, apologies I should have specified that.

br

# Brandon Benvie (13 years ago)

I don't think an array is the right solution for this particular problem though. The performance penalty is too much to make it worth it.

# Mark S. Miller (13 years ago)

On Thu, Jun 14, 2012 at 7:59 PM, Andrea Giammarchi <andrea.giammarchi at gmail.com> wrote:

This is basically what I am doing except I find Array#indexOf as it is pointless ... or better, something that should be fixed in the next version of ECMAScript.

I agree. I argued that it should be fixed as of ES5, but lost the argument on compatibility grounds. No one had measurements either way, so in the absence of evidence we properly made the conservative choice. The other place where this occurs, and IMO is an even worse problem is switch:

function sw(x) {
  switch (x) {
    case NaN: return 'match';
    default: return 'default';
  }
}
sw(NaN);    // returns: 'default'

It's too late for ES6. In order to fix these for ES7, someone would need to gather evidence that the web corpus does not depend on the current broken behavior.

# Michael Haufe (13 years ago)

The opposite seems to be true:

gist.github.com/gists/search?q="case+NaN"&x=0&y=0 < search?q="case+NaN"&type=Everything&repo=&langOverride=&start_value=1

# Allen Wirfs-Brock (13 years ago)

On Jun 14, 2012, at 3:25 PM, Mark S. Miller wrote:

On Thu, Jun 14, 2012 at 7:59 PM, Andrea Giammarchi <andrea.giammarchi at gmail.com> wrote:

This is basically what I am doing except I find Array#indexOf as it is pointless ... or better, something that should be fixed in the next version of ECMAScript.

I agree. I argued that it should be fixed as of ES5, but lost the argument on compatibility grounds. No one had measurements either way, so in the absence of evidence we properly made the conservative choice. The other place where this occurs, and IMO is an even worse problem is switch:

function sw(x) { switch (x) { case NaN: return 'match'; default: return 'default'; } } sw(NaN); // returns: 'default'

It's too late for ES6. In order to fix these for ES7, someone would need to gather evidence that the web corpus does not depend on the current broken behavior.

I disagree about the too lateness. Specification-wise this would be a minor change and the absolute cut-off date is about a year away. If we actually had evidence that it was a safe breaking change we could do it. Of course, getting that evidence is not a trivial matter.

Regarding switch statements. We could make case NaN: be an early error. It wouldn't take care of computed NaN case expression values but it would take care of the obviously buggy literal cases like your example.

We would probably still need some evidence that an early error on case NaN wouldn't be disruptive.

# Allen Wirfs-Brock (13 years ago)

On Jun 14, 2012, at 4:30 PM, Michael Haufe wrote:

The opposite seems to be true:

gist.github.com/gists/search?q="case+NaN"&x=0&y=0

this only has case "NaN": not case NaN:

search?q="case+NaN"&type=Everything&repo=&langOverride=&start_value=1

And the above isn't only looking at JS code. If you limit the search to javascript code the only result that actually matches "case NaN:" is what appears to be a SpiderMonkey test case that is replicated several places

// No test case, just make sure this doesn't assert. function testNegZero2() { var z = 0; for (let j = 0; j < 5; ++j) { ({p: (-z)}); } } testNegZero2();

function testConstSwitch() { var x; for (var j=0;j<5;++j) { switch(1.1) { case NaN: case 2: } x = 2; } return x; } assertEq(testConstSwitch(), 2);