CODEWITHSUNDEEP
javascriptjquery
pdlol
pdlol

Reputation: 389

Javascript splice is breaking on jQuery .each()?

var results = ['one', 'two', 'one hundred', 'three'];
var removal = [];
$.each(results, function(i) {
    removal.push(i);
    if (results[i].indexOf('one') == -1){
        console.log('Removing:' + results[i] + '(' + removal[i] + ')');
        results = results.splice(removal[i], 1);
    }
});

I have the following code, but it is breaking after it removes the first result.

I want it to remove anything that does not contain the word 'one'.

I am guessing it is breaking because the removal order changes once one has been removed.

What am I doing wrong?

Upvotes: 7

Views: 7839

Answers (4)

Arun Pratap Singh
Arun Pratap Singh

Reputation: 3646

var myArr = [0,1,2,3,4,5,6];

Problem statement:

myArr.splice(2,1);

  \\ [0, 1, 3, 4, 5, 6];

now 3 moves at second position and length is reduced by 1 which creates problem.

Solution: A simple solution would be iterating in reverse direction when splicing.

var i = myArr.length;
while (i--) {
    console.log(myArr);
    myArr.splice(i,1);
}

Output:

[0, 1, 2, 3, 4, 5, 6]
[0, 1, 2, 3, 4, 5]
[0, 1, 2, 3, 4]
[0, 1, 2, 3]
[0, 1, 2]
[0, 1]
[0]

Upvotes: 3

user1106925
user1106925

Reputation:

You shouldn't splice the Array while you're iterating it with $.each().

Since you're changing the length of the Array, you're going beyond the final index since.

Just use a for loop and adjust i when you remove an item...

var results = ['one', 'two', 'one hundred', 'three'];
var removal = [];
for(var i = 0; i < results.length; i++) {
    removal.push(i);
    if (results[i].indexOf('one') == -1){
        console.log('Removing:' + results[i] + '(' + removal[i] + ')');
        results.splice(i, 1);
        i--;
    }
};

Note that I changed this...

results = results.splice(removal[i], 1);

to this...

results.splice(removal[i], 1);

You don't want that since splice() modifies the original, and returns the item(s) removed.

Upvotes: 19

pete
pete

Reputation: 25081

Yes, modifying the array structure when you are looping through will wreak havoc aplenty. Try using a temporary arrary:

var results = ['one', 'two', 'one hundred', 'three'];
var removal = [];
var newResults = [];
$.each(results, function(i) {
    removal.push(i);
    if (results[i].indexOf('one') > -1){
        newResults.push(i);
    } else {
        console.log('Removing:' + results[i] + '(' + removal[i] + ')');
    }
});
results = newResults;

Upvotes: -1

nnnnnn
nnnnnn

Reputation: 150020

The problem is that the $.each() function iterates through the array in order from the first element, and internally it sets the index range it will be using at the beginning. So when you remove items from that array it doesn't know you've done so.

You can easily work around this by using a standard for statement. If you loop backwards from the end of the list through to the beginning you can remove items without affecting the loop. If you loop forwards you need to adjust the loop counter each time you remove an item from the array.

Upvotes: 1

Related Questions