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);
}
}
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.
length === 0 would end up in an infinite loop, I have tried this solution and (of course it does work), since it evaluates first the value of i and then decrements. However, -- (and ++) are so weird in their behaviour that a modern language like swift stopped supporting them. I think they are bad practise (at least in a such a context).
i++ mean evaluate the value, then increment it. ++i means increment the value, then evaluate it. JS will never do anything but that. This is really easy to understand, and it's guaranteed to work exactly the same way every time, even if you use a different JS engine.
if(!array[i]) continue; at the beginning of the loop, so it ignores indexes that now are out of range.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.
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.
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;
Since ES2015 we can use Array.prototype.filter to fit it all in one line:
Auction.auctions = Auction.auctions.filter(auction => --auction.seconds >= 0);
Auction.auctions = Auction.auctions.filter(function(el) {
return --el["seconds"] > 0;
});
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.
const?Here is a simple linear time solution to this simple linear time problem.
When I run this snippet, with n = 1 million, each call to filterInPlace() takes .013 to .016 seconds. A quadratic solution (e.g. the accepted answer) would take a million times that, or so.
// Remove from array every item such that !condition(item).
function filterInPlace(array, condition) {
var iOut = 0;
for (var i = 0; i < array.length; i++)
if (condition(array[i]))
array[iOut++] = array[i];
array.length = iOut;
}
// Try it out. A quadratic solution would take a very long time.
var n = 1*1000*1000;
console.log("constructing array...");
var Auction = {auctions: []};
for (var i = 0; i < n; ++i) {
Auction.auctions.push({seconds:1});
Auction.auctions.push({seconds:2});
Auction.auctions.push({seconds:0});
}
console.log("array length should be "+(3*n)+": ", Auction.auctions.length);
filterInPlace(Auction.auctions, auction => --auction.seconds >= 0);
console.log("array length should be "+(2*n)+": ", Auction.auctions.length);
filterInPlace(Auction.auctions, auction => --auction.seconds >= 0);
console.log("array length should be "+n+": ", Auction.auctions.length);
filterInPlace(Auction.auctions, auction => --auction.seconds >= 0);
console.log("array length should be 0: ", Auction.auctions.length);Note that this modifies the original array in place rather than creating a new array; doing it in place like this can be advantageous, e.g. in the case that the array is the program's single memory bottleneck; in that case, you don't want to create another array of the same size, even temporarily.
The normal for loop is more familiar for me, I just need to decrement the index each time I remove an item from the array
//5 trues , 5 falses
var arr1 = [false, false, true, true, false, true, false, true, true, false];
//remove falses from array
for (var i = 0; i < arr1.length; i++){
if (arr1[i] === false){
arr1.splice(i, 1);
i--;// decrement index if item is removed
}
}
console.log(arr1);// should be 5 trues
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
}
why waste cpu cycles on .splice? that operation has to go through the whole loop again and again to remove an element in an array.
why not just use traditional 2 flags in one loop?
const elements = [1, 5, 5, 3, 5, 2, 4];
const remove = 5
i = 0
for(let j = 0; j < elements.length; j++){
if (elements[j] !== remove) {
elements[i] = elements[j]
i++
}
}
elements.length = iTwo examples that work:
Example ONE
// Remove from Listing the Items Checked in Checkbox for Delete
let temp_products_images = store.state.c_products.products_images
if (temp_products_images != null) {
for (var l = temp_products_images.length; l--;) {
// 'mark' is the checkbox field
if (temp_products_images[l].mark == true) {
store.state.c_products.products_images.splice(l,1); // THIS WORKS
// this.$delete(store.state.c_products.products_images,l); // THIS ALSO WORKS
}
}
}
Example TWO
// Remove from Listing the Items Checked in Checkbox for Delete
let temp_products_images = store.state.c_products.products_images
if (temp_products_images != null) {
let l = temp_products_images.length
while (l--)
{
// 'mark' is the checkbox field
if (temp_products_images[l].mark == true) {
store.state.c_products.products_images.splice(l,1); // THIS WORKS
// this.$delete(store.state.c_products.products_images,l); // THIS ALSO WORKS
}
}
}
a shortness one...
by using precedence judiciously in increment/decrement operations). see there
for (let i = 0; i < Auction.auctions.length; i++)
{
if (--Auction.auctions[i].seconds <= 0)
Auction.auctions.splice(i--, 1);
}
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;
Give this a try
RemoveItems.forEach((i, j) => {
OriginalItems.splice((i - j), 1);
});
oldJson=[{firstName:'s1',lastName:'v1'},
{firstName:'s2',lastName:'v2'},
{firstName:'s3',lastName:'v3'}]
newJson = oldJson.map(({...ele}) => {
delete ele.firstName;
return ele;
})
it deletes and and create new array and as we are using spread operator on each objects so the original array objects are also remains unharmed
const arr = [...]
const indexes = [1,2,8,9,34,67]
const result = arr.filter((_, index) => !indexes.includes(index));
console.log(result);
You can just look through and use shift()
Auction.auctions[i]['seconds']--instead ofauction.seconds--?