Looping through array and removing items, without breaking for loop

dzm Source

I have the following for loop, and when I use splice() to remove an item, I then get that 'seconds' is undefined. I could check if it's undefined, but I feel there's probably a more elegant way to do this. The desire is to simply delete an item and keep on going.

for (i = 0, len = Auction.auctions.length; i < len; i++) {
    auction = Auction.auctions[i];
    Auction.auctions[i]['seconds'] --;
    if (auction.seconds < 0) { 
        Auction.auctions.splice(i, 1);
    }           
}
javascriptloops

Answers

answered 6 years ago Marc #1

Recalculate the length each time through the loop instead of just at the outset, e.g.:

for (i = 0; i < Auction.auctions.length; i++) {
      auction = Auction.auctions[i];
      Auction.auctions[i]['seconds'] --;
      if (auction.seconds < 0) { 
          Auction.auctions.splice(i, 1);
          i--; //decrement
      }
}

That way you won't exceed the bounds.

EDIT: added a decrement in the if statement.

answered 6 years ago user1106925 #2

The array is being re-indexed when you do a .splice(), which means you'll skip over an index when one is removed, and your cached .length is obsolete.

To fix it, you'd either need to decrement i after a .splice(), or simply iterate in reverse...

var i = Auction.auctions.length
while (i--) {
    ...
    if (...) { 
        Auction.auctions.splice(i, 1);
    } 
}

This way the re-indexing doesn't affect the next item in the iteration, since the indexing affects only the items from the current point to the end of the Array, and the next item in the iteration is lower than the current point.

answered 5 years ago 0xc0de #3

Although your question is about deleting elements from the array being iterated upon and not about removing elements (in addition to some other processing) efficiently, I think one should reconsider it if in similar situation.

The algorithmic complexity of this approach is O(n^2) as splice function and the for loop both iterate over the array (splice function shifts all elements of array in the worst case). Instead you can just push the required elements to the new array and then just assign that array to the desired variable (which was just iterated upon).

var newArray = [];
for (var i = 0, len = Auction.auctions.length; i < len; i++) {
    auction = Auction.auctions[i];
    auction.seconds--;
    if (!auction.seconds < 0) { 
        newArray.push(auction);
    }
}
Auction.auctions = newArray;

Or use the array functions and arrow function to fit it all on one line:

Auction.auctions = Auction.auctions.filter(auction => --auction.seconds >=0);

answered 5 years ago Aesthete #4

Auction.auction = Auction.auctions.filter(function(el) {
  return --el["seconds"] > 0;
});

answered 4 years ago frattaro #5

This is a pretty common issue. The solution is to loop backwards:

for (var i = Auction.auctions.length - 1; i >= 0; i--) {
    Auction.auctions[i].seconds--;
    if (Auction.auctions[i].seconds < 0) { 
        Auction.auctions.splice(i, 1);
    }
}

It doesn't matter if you're popping them off of the end because the indices will be preserved as you go backwards.

answered 3 years ago Zon #6

Try to relay an array into newArray when looping:

var auctions = Auction.auctions;
var auctionIndex;
var auction;
var newAuctions = [];

for (
  auctionIndex = 0; 
  auctionIndex < Auction.auctions.length;
  auctionIndex++) {

  auction = auctions[auctionIndex];

  if (auction.seconds >= 0) { 
    newAuctions.push(
      auction);
  }    
}

Auction.auctions = newAuctions;

answered 3 years ago daniel.szaniszlo #7

Here is another example for the proper use of splice. This example is about to remove 'attribute' from 'array'.

for (var i = array.length; i--;) {
    if (array[i] === 'attribute') {
        array.splice(i, 1);
    }
}

answered 2 years ago Pablo #8

Another simple solution to digest an array elements once:

while(Auction.auctions.length){
    // From first to last...
    var auction = Auction.auctions.shift();
    // From last to first...
    var auction = Auction.auctions.pop();

    // Do stuff with auction
}

answered 2 years ago Dmitry Ragozin #9

for (i = 0, len = Auction.auctions.length; i < len; i++) {
    auction = Auction.auctions[i];
    Auction.auctions[i]['seconds'] --;
    if (auction.seconds < 0) {
        Auction.auctions.splice(i, 1);
        i--;
        len--;
    }
}

answered 1 year ago user8533067 #10

You can just look through and use shift()

answered 10 months ago Rubinsh #11

If you are e using ES6+ - why not just use Array.filter method?

Auction.auctions = Auction.auctions.filter((auction) => {
  auction['seconds'] --;
  return (auction.seconds > 0)
})  

Note that modifying the array element during filter iteration only works for objects and will not work for array of primitive values.

comments powered by Disqus