Trouble removing elements from multidimensional array using for loop (javascript)

Question

I'm banging my noob head against the wall on this one...

I have the following code:

var guns2 = 
[
["Model 17", "Glock"],
["Model 19", "Glock"],
["PPQ", "Walther"],
["P2000", "HK"],
["Model 92", "Beretta"],
["Model 34", "Glock"]
]

var gunsMake = function () {
    for (i=0; i<guns2.length; i++){
        var make = guns2[i][1];
            if (make ==="Glock"){
                }
            else {
                guns2.splice(i,1);
                }
        };
    };
gunsMake();
console.log(guns2);

The result I get in the console is as follows:

[["Model 17", "Glock"], ["Model 19", "Glock"], ["P2000", "HK"], ["Model 34", "Glock"]]

What I'd like to see is:

[["Model 17", "Glock"], ["Model 19", "Glock"], ["Model 34", "Glock"]]

"["P2000", "HK"]" shouldn't be there...I have a feeling it has something to do with the "guns2.length" argument in the for loop..it seems like it's skipping the subsequent array every time it splices, but I can't quite wrap my brain around a fix.

Please, someone steer me right :)

Don't modify an array while you're iterating over it. Insert desired results into a different array. — user1726343
– user1726343, Commented Apr 10, 2013 at 2:38
Ah, that makes a ton of sense, especially for what I need to achieve in my end goal. Would "push" be an effective way to accomplish that? — Remo Williams
– Remo Williams, Commented Apr 10, 2013 at 2:49
Yes, push is one way to add elements to an array. You need to create a different array first, then push to it inside the loop. — user1726343
– user1726343, Commented Apr 10, 2013 at 2:51

score 6 · Accepted Answer · 2013-04-10 02:59:18Z

6

It usually isn't a good idea to modify an array while you're iterating over it, since it becomes hard to keep track of indices and exit conditions. Either insert desired results into a separate array, or use the native filter method to return a filtered array.

var gunsMake = function (guns, desiredMake) {
    return guns.filter(function(v,i,a){
        return v[1] == desiredMake;
    });
};
guns2 = gunsMake(guns2, "Glock");
console.log(guns2);

More on the Array filter method on MDN: Array filter method

edited Apr 10, 2013 at 2:59

answered Apr 10, 2013 at 2:39

user1726343

Sign up to request clarification or add additional context in comments.

2 Comments

Remo Williams Over a year ago

Asad, thanks again for your help on this. I'll give filter a shot--looks like a pretty powerful method. Much appreciated.

user1726343 Over a year ago

Yup, no problem. You should take a look at the other Array methods (documented on MDN) as well. Once you're familiar with them, they pretty much obviate the need to use a for loop in most cases.

Trevor Dixon · Accepted Answer · 2013-04-10 02:48:48Z

3

It's not always awful to modify the array in-place. If that's what you decide to do, decrement i when you remove an element.

http://jsfiddle.net/kVzLn/

var guns2 = 
[
["Model 17", "Glock"],
["Model 19", "Glock"],
["PPQ", "Walther"],
["P2000", "HK"],
["Model 92", "Beretta"],
["Model 34", "Glock"]
]

var gunsMake = function () {
    for (i=0; i<guns2.length; i++){
        var make = guns2[i][1];
            if (make ==="Glock"){
                }
            else {
                guns2.splice(i--,1); // Decrement i here
                }
        };
    };
gunsMake();
console.log(guns2);

edited Apr 10, 2013 at 2:48

answered Apr 10, 2013 at 2:42

Trevor Dixon

24.6k13 gold badges77 silver badges114 bronze badges

1 Comment

Remo Williams Over a year ago

Awesome. For this particular app, I'm probably going to go with Asad's suggestion of pushing to a new array, but you're advice worked like a charm. Thanks!

Luke Z · Accepted Answer · 2013-04-10 02:40:36Z

1

You're deleting (splicing) nodes out of your array while you're looping through it with a fixed index. So, you're not checking all of the elements.

If you check element #1 and decide to delete it, then when you check #2 you're actually checking #3.

answered Apr 10, 2013 at 2:40

Luke Z

2,4491 gold badge19 silver badges13 bronze badges

1 Comment

Remo Williams Over a year ago

Yeh, that's what I figured (thanks for the confirmation). I just wasn't sure how to do it properly.

Collectives™ on Stack Overflow

Trouble removing elements from multidimensional array using for loop (javascript)

3 Answers 3

2 Comments

1 Comment

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

1 Comment

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related