CODEWITHSUNDEEP
javascriptloops
Matt
Matt

Reputation: 23

Ridiculous javascript bug: have to call a function multiple times for it to behave correctly

I have no doubt at all that my own idiocy is responsible for this. I'm not a programmer, but a scientist, and I just generally hack away at something until it works which is how I end up with such weird bugs. Basically, any help would be really appreciated.

Okay, so my function is this:

function discardDuplicates(threshold) {
    for (var m = 0; m < xCo2.length; m++){
        var testX = xCo2[m];
        var testY = yCo2[m];
        for (var n = 0; n < xCo2.length; n++){
            if (m != n) {
                if ((Math.abs(xCo2[n] - testX) < threshold)
                    && (Math.abs(yCo2[n] - testY) < threshold)
                    && deltas[m] > deltas[n]){

                    xCo2.splice(n,1);
                    yCo2.splice(n,1);
                    deltas.splice(n,1);
                }
            }
        }
    }
}

I'm detecting features which have co-ordinates (x,y) stored in xCo2 and yCo2 arrays, each co-ordinate also has a property known as "delta." And I want to check to see if I've identified several features in basically the same place--if I have, they're probably duplicates, so I remove all but the one with the highest delta from the list.

Right, basically, this doesn't work!

At the moment I have to do this:

//ugly hack 
var oldLength = 0;
var newLength = 1;
while (oldLength != newLength) {
    oldLength = xCo2.length;
    discardDuplicates(10);
    newLength = xCo2.length;
}

Because the first time the function's called it doesn't remove any duplicates. The second time it removes most of them. The third time it usually has them all... So I just keep on calling it until it stops removing duplicates. The function definitely works though; it's removing the right points.

The reason I'm asking this question is that this same bug has now happened a second time, with a second function, which this time tries to remove any co-ordinates with a delta value which is too low.

Enlightenment would be gratefully accepted!

Upvotes: 2

Views: 301

Answers (1)

Ivo Wetzel
Ivo Wetzel

Reputation: 46735

Calling splice on the array will remove an entry and move the next entry to this position, that's very common mistake.

Let's take a look:

xCo2 = [1, 2, 3, 4, 5];
n = 0;

xCo2.splice(n, 1); // n = 0, removed 1
>> [2, 3, 4, 5];
n ++;

xCo2.splice(n, 1); // n = 1, removed 3
>> [2, 4, 5];
n++;

See the problem? You're skipping entries, what you need to do is to reduce n each time you remove entries from the arrays. Since you're always removing 1 entry you need to reduce n by once after you've spliced your arrays.

if ((Math.abs(xCo2[n] - testX) < threshold)
     && (Math.abs(yCo2[n] - testY) < threshold) && deltas[m] > deltas[n] ){

    xCo2.splice(n,1);
    yCo2.splice(n,1);
    deltas.splice(n,1);
    n--;
}

PS: Some whitespace greatly increases the readability of your code.

Upvotes: 10

Related Questions