Issue with similar function for arrays

[Sothatsit]

Hello,

I believe there may be a bug with the similar function for comparing two arrays. However, I could just be reading the code wrong. See the following code snippet that I have copied below: https://github.com/Tixit/odiff/blob/master/odiff.js#L193-L203.

var tenPercent = a.length/10
var notEqual = Math.abs(a.length-b.length) // initialize with the length difference
for(var n=0; n<a.length; n++) {
    if(equal(a[n],b[n])) {
        if(notEqual >= 2 && notEqual > tenPercent || notEqual === a.length) {
            return false
        }

        notEqual++
    }
}

The variable that is incremented is named notEqual, and yet it is used to count the number of equal entries. Should this be changed to check for != instead?

var tenPercent = a.length/10
var notEqual = Math.abs(a.length-b.length) // initialize with the length difference
for(var n=0; n<a.length; n++) {
    if(!equal(a[n],b[n])) {
        if(notEqual >= 2 && notEqual > tenPercent || notEqual === a.length) {
            return false
        }

        notEqual++
    }
}

Thanks, ~ Paddy L.

[fresheneesz]

Hi Paddy, thanks for pointing that out. I think you're correc that this is a logical error. It shouldn't result in any invalid results, but will change how it optimizes structuring the results. Would you be willing to write up a small PR to fix this (with a unit test that would fail on the current incorrect code)?

[Sothatsit]

Hi @fresheneesz, I'm sorry I don't currently have the time to go through fixing this, testing it properly, and double-checking the results. Fortunately, as you point out, it doesn't affect the correctness of the results. After all, odiff has been in use for many years without this being noticed. However, I thought it'd be good to highlight it in case anyone has the time to fix it in the future. I'll keep this in mind to come back to when I have more time.

All the best, ~ Paddy L.

[fresheneesz]

Ok I created a fix. Thanks again for reporting!