nan? #12

Closed
ianstormtaylor opened this Issue Aug 29, 2013 · 15 comments

Comments

Projects
None yet
4 participants
Contributor

ianstormtaylor commented Aug 29, 2013

any thoughts on handling NaN separately from number? am thinking that i never really want NaN to be treated like a number, might be better to 'nan' == type(NaN)?

i can pr if so

Contributor

ianstormtaylor commented Aug 29, 2013

but also could be wrong about this assumption

Contributor

yorkie commented Jan 1, 2014

We could use isNaN function to do this simplely.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isNaN

BUT unfortunately it's not supported on IE/Safari/Opera, hence that a compatible library for this maybe useful.

Owner

dominicbarnes commented Jan 7, 2014

Where isNaN is not available, we can do what underscore does

var isNaN = Number.isNaN || function (val) {
  return typeof val === "number" && val != +val;
};

(haven't tested this at all, that would be for unit tests to ensure of course)

Contributor

yorkie commented Jan 7, 2014

I wanna share another way to do the same stuff, I did take this from node's smalloc:

function isNaN(val) {
  return !(val >>> 0) && val != 0
}
Owner

dominicbarnes commented Jan 7, 2014

I created a JSPerf test with these implementations. I've also created a gist that I used for testing the accuracy of these implementations locally.

Some things I have found:

The native implementation casts it's argument as a Number first, giving inconsistent results imo. (e.g. isNaN("foo") === true) Also, the native method was byfar the slowest.

The second implementation was the fastest, and it caught the 4 cases I set up. (NaN, 1.0, Infinity and "foo")

The third implementation did not catch Infinity, treating it as NaN.

Are there any other inputs we should test? (maybe some expressions in addition to values?)

Owner

Swatinem commented Jan 7, 2014

Even the ES5 spec itself suggests method 2:
http://es5.github.io/#x15.1.2.4

And by definition, a string is "not a number" so ye, if you want your method to have that meaning.

Owner

dominicbarnes commented Jan 7, 2014

Yeah, that's kinda the weirdness, are we looking for "not a number" or the NaN value itself? In the case of this library, I think we want the latter.

Owner

Swatinem commented Jan 7, 2014

Yup I agree. Then just do a return x !== x; as the spec suggests.
The spec itself is not really clear about when it means a non reflexive JS NaN and a reflexive internal NaN.

Owner

dominicbarnes commented Jan 7, 2014

I'm working on a new NaN branch, and I will make a PR for it so anyone who wants to can review it.

Owner

dominicbarnes commented Jan 7, 2014

I'll merge that branch later today or tomorrow unless I hear back from anyone about it.

Contributor

yorkie commented Jan 7, 2014

Hi, @dominicbarnes
why not val !== val directly? It's a simpler and clearer way. I add a test for them: http://jsperf.com/isnan-implementations/2.

I removed a test case fn('foo'), because under this case, val != val is slower than the 1st one.
So we could get typeof operator is faster than !=.

Just my own thought, a clearer way is LGTM.

Owner

Swatinem commented Jan 9, 2014

I'll merge that branch later today or tomorrow unless I hear back from anyone about it.

lgtm. Will you do the honors? :-)

Owner

Swatinem commented Jan 9, 2014

Oh, just noticed, you might also want to add it to the README

Owner

dominicbarnes commented Jan 9, 2014

Good call, I'll do that right now then.

Owner

dominicbarnes commented Jan 9, 2014

Done via 8cbd45b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment