-2

I'm trying to do the opposite of How to short circuit Array.forEach like calling break? 

arr = [{id:"231"}, {id:"343"}];
arr.forEach(function (item) {
    if (item.id === "231") {
        arr.splice(arr.indexOf(item), 1);
        return;
    }
});

When running a code based on this, the if runs just once, if the item order is reversed it runs twice, why?

Community
  • 1
  • 1
Didi Kohen
  • 560
  • 4
  • 19
  • 2
    Your code has a syntax error and won't run. Anyway, don't try modifying the array in midstream. This will not end well. Instead, create a new array without the items you don't want, probably by using `filter`. –  Jan 12 '17 at 17:30
  • 1
    And, `.forEach()` offers a second argument that is the `index`. No need to use `.indexOf()` to find the element again. – jfriend00 Jan 12 '17 at 17:33
  • prefer `filter()` over `splice()`: `var filtered = arr.filter(item => item.id !== "231")` – Thomas Jan 12 '17 at 17:53

3 Answers3

1

The usual solution for mutating the current array is to use a for loop and do the iteration from backwards to front:

var arr = [{id:"231"}, {id:"343"}];
for (var i = arr.length - 1; i >= 0; i--) {
    if (arr[i].id === "231") {
        arr.splice(i, 1);
    }
}

console.log(JSON.stringify(arr));

This way, when you splice() out the current item, you are only changing the indexes of array elements that you have already visited and the iteration is not affected.

It's also possible to run the for loop from start to finish and just correct the index value after splicing out an element.

var arr = [{id:"231"}, {id:"343"}];
for (var i = arr.length - 1; i >= 0; i--) {
    if (arr[i].id === "231") {
        arr.splice(i, 1);
    }
}

console.log(JSON.stringify(arr));

Keep in mind that a plain for loop always gives you more control over the iteration than a .forEach() loop since you can break, continue or modify the iteration index at any time, things which you cannot do with .forEach().

If you're OK with the end result being a new array, then .filter() would work quite easily for you as this is what it is built for.

var arr = [{id:"231"}, {id:"343"}];
arr = arr.filter(function(item) {
    return item.id !== "231";
});

console.log(JSON.stringify(arr));

Or, in ES6:

var arr = [{id:"231"}, {id:"343"}];
arr = arr.filter(item => item.id !== "231");

console.log(JSON.stringify(arr));
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • While this works as well, this was a code segment I did not want to change too much to avoid merge issues, the splice was a better option for me. – Didi Kohen Jan 12 '17 at 17:27
  • @DavidKohen - Seriously, either one of these first two options is too much code change to avoid doing unnecessary copying of the array? Or, if you're going to copy the array anyway, then use `.filter()`. – jfriend00 Jan 12 '17 at 17:29
  • No, the usual solution is not to mutate the array like this. –  Jan 12 '17 at 17:32
  • @torazaburo - That depends entirely upon your application and the situation. Sometimes a copy is appropriate and sometime mutating is appropriate. There are very legit reasons for each - it depends entirely upon the design and the situation. Ideally one wouldn't be doing a brute force search by id, but would be using a Map anyway, but these are the cards we were dealt here. – jfriend00 Jan 12 '17 at 17:35
  • @torazaburo - The length is adjusted automatically by the array and compared before every iteration of the `for` loop so it won't go off the end of the array. If one was caching the length in a separate variable (which I am purposely not doing here), then that would have to be updated too. – jfriend00 Jan 12 '17 at 18:21
  • I made all my code examples into runnable snippets. See for yourself how they work. – jfriend00 Jan 12 '17 at 18:27
  • @jfriend00 Yes, both of these changes will need more code changes than copying the array, the code that I posted is a simplification of a loop with hundreds of lines of code (most of it not mine). – Didi Kohen Jan 13 '17 at 07:59
  • @jfriend00 - I meant to write slice, not splice in my first comment. – Didi Kohen Jan 13 '17 at 08:11
1

splice will messup the size of array and your loop will give wrong result. You should use filter to do this.

arr = arr.filter((item)=>{
     return item.id !== "231";
})
Anurag Awasthi
  • 6,115
  • 2
  • 18
  • 32
0

Splicing an array used in a foreach loop may cause skipping of items, to avoid this I ran the forEach on the result of slice on the array.

Didi Kohen
  • 560
  • 4
  • 19