1loc icon indicating copy to clipboard operation
1loc copied to clipboard

isEqual is buggy

Open dcaillibaud opened this issue 3 years ago • 2 comments

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') })

dcaillibaud avatar Mar 30 '21 09:03 dcaillibaud

Hi @dcaillibaud, can you mention the related post ?

elkarouani avatar Dec 20 '21 19:12 elkarouani

I believe @dcaillibaud talking about: https://github.com/1milligram/1loc/blob/cecbfa239c1d6c9d527518c157b9ea20c4140c9b/snippets/array/compare-two-arrays.md?plain=1#L8-L14

secondfry avatar Jun 18 '22 17:06 secondfry