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

isEqual is buggy #414

Open
dcaillibaud opened this issue Mar 30, 2021 · 2 comments
Open

isEqual is buggy #414

dcaillibaud opened this issue Mar 30, 2021 · 2 comments

Comments

@dcaillibaud
Copy link

Hi,

You should alert that

const isEqual1 = (a, b) => JSON.stringify(a) === JSON.stringify(b)

isn't similar to

const isEqual2 = (a, b) => a.length === b.length && a.every((v, i) => v === b[i])

and both are valuable (and similar) only if all array elements are boolean|finite number|string

// buggy if NaN
isEqual1([NaN], [NaN]) // => true
isEqual2([NaN], [NaN]) // => false

// buggy if object
// plain object
isEqual1([{a: 42}], [{a: 42}]) // => true
isEqual2([{a: 42}], [{a: 42}]) // => false
// array
isEqual1([[42]], [[42]]) // => true
isEqual2([[42]], [[42]]) // => false
// Date
isEqual1([new Date('2021-03-30')], [new Date('2021-03-30')]) // => true
isEqual2([new Date('2021-03-30')], [new Date('2021-03-30')]) // => false

And the json comparaison can be dangerous

isEqual1([NaN], [null]) // => true
isEqual1([Number.POSITIVE_INFINITY], [Number.NEGATIVE_INFINITY]) // => true
isEqual1([Number.POSITIVE_INFINITY], [null]) // => true

The only correct way to compare would be to test type of each element then value, in a recurse manner, but it needs a very long line of code ;-) (eg lodash implementation

You can limit to the area where the code is valid with something like

// returns undefined when we can't decide in a such easy way
const isEqual = (a, b) => a.every((v, i) => (['boolean', 'string'].includes(typeof v) || (typeof v === 'number' && !Number.isNaN(v)))) ? a.length === b.length && a.every((v, i) => v === b[i]) : undefined

// or throw (which seems better in real life, because undefined is falsy and previous function can leads to bugs hard to detect)
const isEqual = (a, b) => a.length === b.length && a.every((v, i) => { if (['boolean', 'string'].includes(typeof v) || (typeof v === 'number' && !Number.isNaN(v))) return v === b[i]; throw Error('isEqual can only compare array of boolean|string|number') })
@elkarouani
Copy link
Contributor

Hi @dcaillibaud, can you mention the related post ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants