159

I am trying to remove an element in an array in a forEach loop, but am having trouble with the standard solutions I've seen.

This is what I'm currently trying:

review.forEach(function(p){
   if(p === '\u2022 \u2022 \u2022'){
      console.log('YippeeeE!!!!!!!!!!!!!!!!')
      review.splice(p, 1);
   }
});

I know it's getting into the if because I'm seeing YippeeeeeE!!!!!!!!!!!!! in the console.

MY PROBLEM: I know that my for loop and if logic are sound, but my attempt to remove the current element from the array is failing.

UPDATE:

Tried out Xotic750's answer, and the element is still not being removed:

Here is the function in my code:

review.forEach(function (item, index, object) {
    if (item === '\u2022 \u2022 \u2022') {
       console.log('YippeeeE!!!!!!!!!!!!!!!!')
       object.splice(index, 1);
    }
    console.log('[' + item + ']');
});

Here is the output where the array is still not removed:

[Scott McNeil]
[reviewed 4 months ago]
[ Mitsubishi is AMAZING!!!]
YippeeeE!!!!!!!!!!!!!!!!
[• • •]

So obviously it is going into the if statement as directed, but it's also obvious that the [• • •] is still there.

scottt
  • 7,008
  • 27
  • 37
novicePrgrmr
  • 18,647
  • 31
  • 81
  • 103
  • 11
    Is there a reason you are using `forEach`? If you want to remove items, the most appropriate function is [`filter`](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Array/filter). – Jon Jul 17 '14 at 20:31
  • 3
    Not if you need to keep the reference to the original array. – Xotic750 Jul 17 '14 at 20:35
  • Yes, we'd like to keep the reference to the original array. – novicePrgrmr Jul 17 '14 at 20:40
  • It is not clear from your question, what is the actual problem that you are having? Can you give an example, perhaps a jsFiddle? It seems that you should perhaps be using the `index` attribute rather than `item` for your `splice` – Xotic750 Jul 17 '14 at 20:42
  • @Xotic750 Sorry, added clarification. – novicePrgrmr Jul 17 '14 at 20:44
  • You are logging the `item` inside the `forEach`, add `console.log(review);` after this, like in my example. – Xotic750 Jul 17 '14 at 21:25
  • Possible duplicate of [How do I remove an element in a list, using forEach?](https://stackoverflow.com/questions/6950236/how-do-i-remove-an-element-in-a-list-using-foreach) – Armfoot Oct 15 '19 at 22:36

8 Answers8

347

It looks like you are trying to do this?

Iterate and mutate an array using Array.prototype.splice

var pre = document.getElementById('out');

function log(result) {
  pre.appendChild(document.createTextNode(result + '\n'));
}

var review = ['a', 'b', 'c', 'b', 'a'];

review.forEach(function(item, index, object) {
  if (item === 'a') {
    object.splice(index, 1);
  }
});

log(review);
<pre id="out"></pre>

Which works fine for simple case where you do not have 2 of the same values as adjacent array items, other wise you have this problem.

var pre = document.getElementById('out');

function log(result) {
  pre.appendChild(document.createTextNode(result + '\n'));
}

var review = ['a', 'a', 'b', 'c', 'b', 'a', 'a'];

review.forEach(function(item, index, object) {
  if (item === 'a') {
    object.splice(index, 1);
  }
});

log(review);
<pre id="out"></pre>

So what can we do about this problem when iterating and mutating an array? Well the usual solution is to work in reverse. Using ES3 while but you could use for sugar if preferred

var pre = document.getElementById('out');

function log(result) {
  pre.appendChild(document.createTextNode(result + '\n'));
}

var review = ['a' ,'a', 'b', 'c', 'b', 'a', 'a'],
  index = review.length - 1;

while (index >= 0) {
  if (review[index] === 'a') {
    review.splice(index, 1);
  }

  index -= 1;
}

log(review);
<pre id="out"></pre>

Ok, but you wanted to use ES5 iteration methods. Well and option would be to use Array.prototype.filter but this does not mutate the original array but creates a new one, so while you can get the correct answer it is not what you appear to have specified.

We could also use ES5 Array.prototype.reduceRight, not for its reducing property by rather its iteration property, i.e. iterate in reverse.

var pre = document.getElementById('out');

function log(result) {
  pre.appendChild(document.createTextNode(result + '\n'));
}

var review = ['a', 'a', 'b', 'c', 'b', 'a', 'a'];

review.reduceRight(function(acc, item, index, object) {
  if (item === 'a') {
    object.splice(index, 1);
  }
}, []);

log(review);
<pre id="out"></pre>

Or we could use ES5 Array.protoype.indexOf like so.

var pre = document.getElementById('out');

function log(result) {
  pre.appendChild(document.createTextNode(result + '\n'));
}

var review = ['a', 'a', 'b', 'c', 'b', 'a', 'a'],
  index = review.indexOf('a');

while (index !== -1) {
  review.splice(index, 1);
  index = review.indexOf('a');
}

log(review);
<pre id="out"></pre>

But you specifically want to use ES5 Array.prototype.forEach, so what can we do? Well we need to use Array.prototype.slice to make a shallow copy of the array and Array.prototype.reverse so we can work in reverse to mutate the original array.

var pre = document.getElementById('out');

function log(result) {
  pre.appendChild(document.createTextNode(result + '\n'));
}

var review = ['a', 'a', 'b', 'c', 'b', 'a', 'a'];

review.slice().reverse().forEach(function(item, index, object) {
  if (item === 'a') {
    review.splice(object.length - 1 - index, 1);
  }
});

log(review);
<pre id="out"></pre>

Finally ES6 offers us some further alternatives, where we do not need to make shallow copies and reverse them. Notably we can use Generators and Iterators. However support is fairly low at present.

var pre = document.getElementById('out');

function log(result) {
  pre.appendChild(document.createTextNode(result + '\n'));
}

function* reverseKeys(arr) {
  var key = arr.length - 1;

  while (key >= 0) {
    yield key;
    key -= 1;
  }
}

var review = ['a', 'a', 'b', 'c', 'b', 'a', 'a'];

for (var index of reverseKeys(review)) {
  if (review[index] === 'a') {
    review.splice(index, 1);
  }
}

log(review);
<pre id="out"></pre>

Something to note in all of the above is that, if you were stripping NaN from the array then comparing with equals is not going to work because in Javascript NaN === NaN is false. But we are going to ignore that in the solutions as it it yet another unspecified edge case.

So there we have it, a more complete answer with solutions that still have edge cases. The very first code example is still correct but as stated, it is not without issues.

Joel G Mathew
  • 7,561
  • 15
  • 54
  • 86
Xotic750
  • 22,914
  • 8
  • 57
  • 79
  • Thanks for answering. I tried using your solution, but it still isn't removing the element from the array. I'll put up the details in the question. – novicePrgrmr Jul 17 '14 at 21:13
  • Put `console.log(review);` after the `forEach`, like in my example. – Xotic750 Jul 17 '14 at 21:24
  • 4
    Careful, this breaks if two consecutive elements to delete: var review = ['a', 'a', 'c', 'b', 'a']; will yield ['a', 'c', 'b'] – quentinadam Apr 02 '15 at 19:35
  • @goldmine I wouldn't use this method to do this job either, but the issue was not about the suitability of this method for the job at hand but more of how to use `Array#splice` and `Array#forEach`. It's great that you have alerted people to a potential problem with doing it this way, but I don't think it deserved a down-vote as it addresses the actual problem that the OP was having. – Xotic750 Apr 02 '15 at 20:38
  • hi, when you have to delete consecutive elements, the best way is forEach a copy of array and delete in the original! https://jsfiddle.net/Xotic750/hddsB/ var review = ['x', 'b', 'c', 'a', 'm']; var obj = []; angular.copy(review, obj); obj.forEach(function (item, index, object) { if (item === 'x' || item == 'a') { review.splice(review.indexOf(item), 1); } }); alert(review); – alvarodoune Sep 15 '15 at 14:40
  • 4
    NOTE - This answer is WRONG! The foreach iterate through the array by index. Once you delete elements while iterating the index of following items changes. In this example, once you delete the first 'a', index number 1 now becomes 'c'. Therefore the first 'b' is not even evaluated. Since you didn't try to delete it, it just happened to be ok, but that is not the way. You should iterate through a reverse copy of the array, and then delete items in the original array. – danbars Sep 20 '15 at 12:17
  • @user1655734 The answer was not wrong, but it did not point out any pitfalls or edge cases. I have expanded the answer now to give greater detail. – Xotic750 Sep 20 '15 at 14:34
  • 5
    @Xotic750 - the original answer (now being the first code snippet) is wrong simply because the forEach will not loop through all elements in the array, as I explained in the previous comment. I know that the question was how to remove elements in a forEach loop, but the simple answer is that you don't do that. Since many people are reading those answers, and many times blindly copying answers (especially accepted answers), it is important to note flaws in the code. I think that the reverse while loop is the simplest, most efficient and most readable solution and should be the accepted answer – danbars Sep 21 '15 at 20:04
  • As I stated in my comments, that would not be code that I would ever (at least unlikely) use. However, if I know that my array never has 2 adjacent elements with the same value then there is really nothing wrong with it. But I have given several much better alternatives that can be used and that will never suffer the problem that the comments have pointed out. – Xotic750 Sep 21 '15 at 20:18
  • An eye-opener! :-) – David R Jul 05 '18 at 16:00
  • Brilliant answer! – Orlandster Oct 05 '18 at 06:14
  • My guess would be that your problem is one of equality comparison, as the ideas demonstrated above are equally valid for an array of Objects as they are for an array of Primitives, etc. etc. – Xotic750 Jan 01 '19 at 01:52
56

Use Array.prototype.filter instead of forEach:

var pre = document.getElementById('out');

function log(result) {
  pre.appendChild(document.createTextNode(result + '\n'));
}

var review = ['a', 'b', 'c', 'b', 'a', 'e'];
review = review.filter(item => item !== 'a');
log(review);
Badacadabra
  • 8,043
  • 7
  • 28
  • 49
alemjerus
  • 8,023
  • 3
  • 32
  • 40
43

Although Xotic750's answer provides several good points and possible solutions, sometimes simple is better.

You know the array being iterated on is being mutated in the iteration itself (i.e. removing an item => index changes), thus the simplest logic is to go backwards in an old fashioned for (à la C language):

let arr = ['a', 'a', 'b', 'c', 'b', 'a', 'a'];

for (let i = arr.length - 1; i >= 0; i--) {
  if (arr[i] === 'a') {
    arr.splice(i, 1);
  }
}

document.body.append(arr.join());

If you really think about it, a forEach is just syntactic sugar for a for loop... So if it's not helping you, just please stop breaking your head against it.

CPHPython
  • 12,379
  • 5
  • 59
  • 71
2

I understood that you want to remove from the array using a condition and have another array that has items removed from the array. Is right?

How about this?

var review = ['a', 'b', 'c', 'ab', 'bc'];
var filtered = [];
for(var i=0; i < review.length;) {
  if(review[i].charAt(0) == 'a') {
    filtered.push(review.splice(i,1)[0]);
  }else{
    i++;
  }
}

console.log("review", review);
console.log("filtered", filtered);

Hope this help...

By the way, I compared 'for-loop' to 'forEach'.

If remove in case a string contains 'f', a result is different.

var review = ["of", "concat", "copyWithin", "entries", "every", "fill", "filter", "find", "findIndex", "flatMap", "flatten", "forEach", "includes", "indexOf", "join", "keys", "lastIndexOf", "map", "pop", "push", "reduce", "reduceRight", "reverse", "shift", "slice", "some", "sort", "splice", "toLocaleString", "toSource", "toString", "unshift", "values"];
var filtered = [];
for(var i=0; i < review.length;) {
  if( review[i].includes('f')) {
    filtered.push(review.splice(i,1)[0]);
  }else {
    i++;
  }
}
console.log("review", review);
console.log("filtered", filtered);
/**
 * review [  "concat",  "copyWithin",  "entries",  "every",  "includes",  "join",  "keys",  "map",  "pop",  "push",  "reduce",  "reduceRight",  "reverse",  "slice",  "some",  "sort",  "splice",  "toLocaleString",  "toSource",  "toString",  "values"] 
 */

console.log("========================================================");
review = ["of", "concat", "copyWithin", "entries", "every", "fill", "filter", "find", "findIndex", "flatMap", "flatten", "forEach", "includes", "indexOf", "join", "keys", "lastIndexOf", "map", "pop", "push", "reduce", "reduceRight", "reverse", "shift", "slice", "some", "sort", "splice", "toLocaleString", "toSource", "toString", "unshift", "values"];
filtered = [];

review.forEach(function(item,i, object) {
  if( item.includes('f')) {
    filtered.push(object.splice(i,1)[0]);
  }
});

console.log("-----------------------------------------");
console.log("review", review);
console.log("filtered", filtered);

/**
 * review [  "concat",  "copyWithin",  "entries",  "every",  "filter",  "findIndex",  "flatten",  "includes",  "join",  "keys",  "map",  "pop",  "push",  "reduce",  "reduceRight",  "reverse",  "slice",  "some",  "sort",  "splice",  "toLocaleString",  "toSource",  "toString",  "values"]
 */

And remove by each iteration, also a result is different.

var review = ["of", "concat", "copyWithin", "entries", "every", "fill", "filter", "find", "findIndex", "flatMap", "flatten", "forEach", "includes", "indexOf", "join", "keys", "lastIndexOf", "map", "pop", "push", "reduce", "reduceRight", "reverse", "shift", "slice", "some", "sort", "splice", "toLocaleString", "toSource", "toString", "unshift", "values"];
var filtered = [];
for(var i=0; i < review.length;) {
  filtered.push(review.splice(i,1)[0]);
}
console.log("review", review);
console.log("filtered", filtered);
console.log("========================================================");
review = ["of", "concat", "copyWithin", "entries", "every", "fill", "filter", "find", "findIndex", "flatMap", "flatten", "forEach", "includes", "indexOf", "join", "keys", "lastIndexOf", "map", "pop", "push", "reduce", "reduceRight", "reverse", "shift", "slice", "some", "sort", "splice", "toLocaleString", "toSource", "toString", "unshift", "values"];
filtered = [];

review.forEach(function(item,i, object) {
  filtered.push(object.splice(i,1)[0]);
});

console.log("-----------------------------------------");
console.log("review", review);
console.log("filtered", filtered);
Park Jun-Hong
  • 103
  • 1
  • 6
1

You could also use indexOf instead to do this

var i = review.indexOf('\u2022 \u2022 \u2022');
if (i !== -1) review.splice(i,1);
jj689
  • 456
  • 2
  • 4
0

Here is how you should do it:

review.forEach(function(p,index,object){
   if(review[index] === '\u2022 \u2022 \u2022'){
      console.log('YippeeeE!!!!!!!!!!!!!!!!')
      review.splice(index, 1);
   }
});
blueren
  • 2,730
  • 4
  • 30
  • 47
Alaeddin Hussein
  • 738
  • 1
  • 7
  • 14
  • 1
    I don't think is the case. I changed my code assuming p was an index, and now it's not even getting into the ``if`` statement. – novicePrgrmr Jul 17 '14 at 20:39
  • 3
    @WhoCares You should see the spec http://www.ecma-international.org/ecma-262/5.1/#sec-15.4.4.18 The callBack function arguments are `item, index, object` – Xotic750 Jul 17 '14 at 20:40
0

The following will give you all the elements which is not equal to your special characters!

review = jQuery.grep( review, function ( value ) {
    return ( value !== '\u2022 \u2022 \u2022' );
} );
Jenna Leaf
  • 2,255
  • 21
  • 29
0

I had problems with the following code. My solution follows after. First, the problem: Let's assume you want to trim strings, and discard the string with a 'c' in it:

var review = ['    a      ', '   b   ', '   c   ', '   d   ', '   e   '];

review.forEach(function(item, index) {
   review[index] = item.trim();
   console.log("After trimming, the item is ", review[index]);
  if (review[index] === 'c') {
    review.splice(index, 1);
  }
});
console.log("Review is now: ");
console.log(review);

If you run the above piece of code, you will see that ' d ' is never trimmed. It remains in the review array, with spaces before and after it.

enter image description here

That's because you are messing with the review array that the foreach is built on. It's a lot better to make a copy of the array, and push elements into it that are OK to keep, and skip the ones that are not. Then, output the new FinalArray, like this:

var review = ['    a      ', '   b   ', '   c   ', '   d   ', '   e   '];
var finalArray = [];

review.forEach(function(item, index) {
    // Add it to the new array first.  If it turns out to be bad, remove it before going onto 
    // the next iteration.
   finalArray.push(item.trim());
   console.log("After trimming, the item is ", item.trim());
  if (item.trim() == 'c') { // skip c
    finalArray.pop();     // This way, removing it doesn't affect the original array.
  }
});
console.log("FinalArray is now: ");
console.log(finalArray);

As you can see, this worked perfectly:

enter image description here

Steve Stilson
  • 1,015
  • 7
  • 11