phuocng/1loc

isEqual is buggy

dcaillibaud opened this issue · 2 comments

dcaillibaud commented

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 commented

Hi @dcaillibaud, can you mention the related post ?