Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basis for `no-self-compare`? #374

Closed
mitranim opened this issue Jan 7, 2016 · 9 comments

Comments

@mitranim
Copy link

commented Jan 7, 2016

Comparing to self is the best way to test for NaN:

const nan = NaN
nan !== nan  // => true

The no-self-compare rule included into standardjs complains about this being "potentially pointless". I think it's wrong.

@rstacruz

This comment has been minimized.

Copy link
Member

commented Jan 7, 2016

Use isNaN(). 'x === x' is confusing.

On Friday, January 8, 2016, Nelo Mitranim notifications@github.com wrote:

Comparing to self is the best way to test for NaN:

const nan = NaN
nan !== nan // => true

The no-self-compare rule included into standardjs complains about this
being "potentially pointless". I think it's wrong.


Reply to this email directly or view it on GitHub
#374.

@mitranim

This comment has been minimized.

Copy link
Author

commented Jan 7, 2016

Questionable advice. isNaN is so broken, the language now includes a second version (ES2015 only). It's usable in combination with typeof, but verbose. Self-compare is shorter and cleaner (arguably).

@dcousens

This comment has been minimized.

Copy link
Member

commented Jan 8, 2016

@mitranim how is isNaN broken?
Works for me. Maybe !isFinite?

@bcomnes

This comment has been minimized.

Copy link
Member

commented Jan 8, 2016

@dcousens the mdn link that @mitranim posted has details on the issue I presume is the reference here. I don't have any opinion on this matter though.

EDIT: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isNaN

@dcousens

This comment has been minimized.

Copy link
Member

commented Jan 8, 2016

Right, so Number.isNaN is just more strict, sure. I wouldn't call isNaN broken though, as isFinite has the same behaviour.

@mitranim

This comment has been minimized.

Copy link
Author

commented Jan 8, 2016

By broken I mean it doesn't tell you if a value is exactly NaN. Here's an example where you need this.

Let's say you're implementing a deep tree comparison algorithm for something like emerge. Can't use === for primitives because of NaN, so you'll need a SameValueZero comparator. Here's how it looks with isNaN:

function sameValueZero (one, other) {
  return one === other ||
    typeof one === 'number' && isNaN(one)
    typeof other === 'number' && isNaN(other)
}

Verbose! With self-compare:

function sameValueZero (one, other) {
  return one === other || one !== one && other !== other
}
@bcomnes

This comment has been minimized.

Copy link
Member

commented Jan 8, 2016

Seems clever enough to warrant a function/module with some semantic value but maybe there are performance concerns. I'm ¯\_(ツ)_/¯ on this one. For weird edge cases like this, I've used modules or ignore comments. Are there any other situations where you would want to self compare other than this?

@feross

This comment has been minimized.

Copy link
Member

commented Jan 8, 2016

See the rule page for a more complete explanation of the rationale. This is a good default for 99%+ of users.

If you still want to use a === a in your code, you can just disable the rule with a comment at the top:

/*eslint-disable no-self-compare */

@feross feross closed this Jan 8, 2016

@dcousens

This comment has been minimized.

Copy link
Member

commented Jan 9, 2016

Are there any other situations where you would want to self compare other than this?

Nope, and in this case, you don't even need a comment, you can just use the isNaN or Number.isNaN functions. Less bike shedding, instantly readable, no floating point rules to lookup/remember.

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
5 participants
You can’t perform that action at this time.