Evik James
Evik James

Reputation: 10503

Most efficient way to remove an item from array?

I have two arrays. If a user adds a product, we put it in the ProductArray. If they remove that product, we add it to the ProductArrayRemove array as well as remove it from the product array. (We need to know products that have been added as well as products that have been removed. This requires redundancy.)

ProductArray = JSON.parse(ProductArray);
ProductArrayRemove = JSON.parse(ProductArrayRemove);

When I add and item to the array, I simply do this:

 ProductArray.push(ItemID);
 ProductArrayRemove.push(ItemID);

But when I remove it, I have to do this:

var len = ProductArray.length;
for (i = 0; i < len; i++) {
    ProductID = ProductArray[i];
    if (ProductID == ItemID) {
        ProductArray.splice(i,1);
        break;
    }
}

It seems that there should be a better way of accomplishing this task.

Is there a more efficient means of getting rid of a single item (that will always be an integer) from an array?

Upvotes: 5

Views: 5294

Answers (10)

Chirka
Chirka

Reputation: 31

Array.prototype.filter() should do the trick. ref

Array.filter() goes through each array element (this case product) and returns a new modified array based on your needs (this case, returns an array without the deleted product)

It would look like something like this:

  ProductArray = ProductArray.filter(function(product) {
        return product !== ItemID
    })

the product is each array element in your ProductArray.

You even make it smaller with arrow function which is almost the same function() but more compact and limited.

  ProductArray = ProductArray.filter(product => product !== ItemID)

Now the ProductArray doesn't have the deleted product and it's a one-liner solution

Upvotes: 0

Diode
Diode

Reputation: 25165

First thing, it is often mistaken that calling function is more efficient than writing 10 lines of code. Most of us don't think what is actually the function is doing and how many lines are there in the function. For e.g: indexOf cannot be implemented without using a loop or additional variables.

Here in your case you add an element using push which actually appends an element at the end of the array. So it is simple, add one more memory location to the array. But when you remove an element you remove it from any index in the array. So the steps involved are

  1. Find the index of the element
  2. Remove the element from that index
  3. Adjust the index of all elements after the deleted index

Obviously looping is needed and using loop will not make it less efficient. My suggestion is to avoid using splice ( as you have already started a loop ) and remove the element using your loop only - do something as shown below

var len = ProductArray.length;
for (i = 0; i < len; i++) {
    ProductID = ProductArray[i];
    if (ProductID == ItemID && i != len-1) {
        ProductArray[i] = ProductArray[i+1];
        ProductArray[i+1] = ItemID;
    }else if(i == len-1){
        ProductArray.length = i;
    }
}

You can make it a function of array by adding it to protoype

Array.prototype.removeItem = function (itemID){
    // put the code here replacing "ProductArray" with "this"
}

Upvotes: 1

kennebec
kennebec

Reputation: 104840

use the Array method indexOf- it is worth the extra code you need to explain it to old and undernourished browsers to have it to use in code like yours.

var i= ProductArray.indexOf(ProductID);
if(i> -1) ProductArray.splice(i, 1);

//This shim is about average:

if(!Array.prototype.indexOf) Array.prototype.indexOf= function(what, i){
    if(!i || typeof i!= 'number') i= 0;
    var L= this.length;
    while(i< L){
        if(this[i]=== what) return i;
        ++i;
    }
    return -1;
}

Upvotes: 1

Alex Vidal
Alex Vidal

Reputation: 4108

You could make your two lists objects instead of arrays of integers, and use delete to remove the items:

// removing an item
function RemoveProduct(ItemID) {
    delete ProductsAdded[ItemID];
    ProductsRemoved[ItemID] = ItemID;
}

// adding an item
function AddProduct(ItemID) {
    delete ProductsRemoved[ItemID];
    ProductsAdded[ItemID] = ItemID;
}

Now, this would require some extra mangling on the server-side, since it should send off a json encoded hash (or map, or assoc array depending on language) instead of a list, but this is definitely a much faster way to remove.

Upvotes: 3

Erik  Reppen
Erik Reppen

Reputation: 4635

Sort is probably surprisingly well optimized in most browsers. Pop is also typically very fast since the concept of stacks begins in assembly. Being able to define sort functions is really powerful. Mostly though I get the impression you'd like it a touch more easy to use which I think this is.

Array.prototype.numFindPop = function(id){
    this.sort( 
        function(a,b){
            if(a === id){ return -1; }//keeps moving id to the right when found
        }
    );

    return this.pop();

}

So in your case:

//remove
ProductArrayRemove.push(
    ProductArray.numFindPop(35)
);

Upvotes: 0

sp00m
sp00m

Reputation: 48837

I would have done this:

var itemId = 6;
var products = [1,3,5,6,7];
var removedProducts = new Array();
removedProducts.push(
   products.splice(products.indexOf(itemId), 1)
);

Upvotes: 0

jfriend00
jfriend00

Reputation: 708026

One different idea that would be super fast to remove an item is to just maintain a single list of items and have a property on each item for whether it is removed or not.

Then to remove an item, all you do it set it's property obj.removed = true.

The add it back again, you just change the value of that property.

To iterate just the added items, you just skip the ones with the .removed == true property. To iterate just the removed items, you do just the reverse. Here would be a couple of iterators:

ProductArray.iterateAdded = function(fn) {
    for (var i = 0; i < this.length; i++) {
        if (!this[i].removed) {
            if (fn.call(this, i, this[i]) === false) {
                return;
            }
        }
    }
}

ProductArray.iterateRemoved = function(fn) {
    for (var i = 0; i < this.length; i++) {
        if (this[i].removed) {
            if (fn.call(this, i, this[i]) === false) {
                return;
            }
        }
    }
}

ProductArray.getSubLength = function(removed) {
    var cnt = 0;
    for (var i = 0; i < this.length; i++) {
        if (removed == this[i].removed) {
            ++cnt;
        }
    }
    return(cnt);
}

And, you would use them like this:

ProductArray.iterateAdded(function(i, val) {
   // this is the array
   // i is the index we are iterating
   // val is the array element we are iterating this[i]
   // put code here
});

Upvotes: 1

ninjagecko
ninjagecko

Reputation: 91149

Why don't you use objects, where the keys are the product ID#s, and the values contain whatever metadata you want (e.g. when the removal occurred, on what webpage, etc.; it could even be arbitrary).

var added = {432215: {...}, 432676: {...}};
var removed = {432215: {...}};

That would be the most efficient way, as you asked for, if you're willing to modify things around a bit. However your question "What's the most efficient way..." is not that reasonable, since a user is unlikely to have more than maybe 100 items in his cart tops. Even if you had an O(N^2) algorithm you wouldn't see issues until perhaps 1000 items. So just use the method which is easiest to code.

Do note that there is a possible flaw in the design of this system: it is reasonable for the customer to remove an item and add it back later. Depending on what you are keeping track of, you may need to special-case this.

Upvotes: 0

wirate
wirate

Reputation: 695

No. Such an operation has to take linear time to find the element to remove (assuming the items are not ordered in any way) and to move succeeding elements backward. The code can be cleaned though as other answers have suggested.

Upvotes: 0

David Mulder
David Mulder

Reputation: 27030

Might be mistaken, but isn't your code simply doing the same as

ProductArray.splice(ProductArray.indexOf(ItemID),1);

(not tested and written from the top of my head, but that should work as far as I know, though it won't work if you have multiple items with ItemID)

Upvotes: 0

Related Questions