13

In this code I want to remove an element from the cart_products array.

var cart_products = ["17^1", "19^1", "18^1"];
var product = 17;

$.each(cart_products,function(key, item) {
    if(item.indexOf(product+"^") !== -1){
        cart_products.splice(key, 1);
    }
});

But I get this error in Google Chrome console:

Uncaught TypeError: Cannot read property 'indexOf' of undefined

Is there something wrong with the code?

Thanks for your help.

4
  • What's inside cart_products ? Commented Jan 27, 2017 at 14:13
  • what is the value of cookie ? Commented Jan 27, 2017 at 14:13
  • 1
    F.Y.I. you don't need to use jQuery to iterate through each element in an array. The Array prototype already has the forEach() method. Commented Jan 27, 2017 at 14:17
  • 1
    @MysterX indexOf() is also a string method. No suggestion that OP is looking to use an array inside the loop Commented Jan 27, 2017 at 14:18

2 Answers 2

17

The problem is that you're modifying the array while jQuery's $.each is looping over it, so by the time it gets to the end, the entry that used to be at index 2 is no longer there. (I admit I'm a bit surprised $.each behaves that way, but I haven't used $.each in at least five years, so...)

If the goal is to remove matches from the array, the better choice is filter:

var cart_products = ["17^1", "19^1", "18^1"];
var product = 17;

cart_products = cart_products.filter(function(item) {
    return item.indexOf(product+"^") === -1;
});
console.log(cart_products);

...or alternately if it's important to modify the array in-place rather than creating a new one use a boring for loop as Andreas points out looping backward through the array so it doesn't matter when you remove things:

var cart_products = ["17^1", "19^1", "18^1"];
var product = 17;

var target = product + "^";
for (var index = cart_products.length - 1; index >= 0; --index) {
  if (cart_products[index].indexOf(target) !== -1) {
    cart_products.splice(index, 1);
  }
}
console.log(cart_products);

Sign up to request clarification or add additional context in comments.

3 Comments

Um...wow, folks. I think the answer's been sufficiently upvoted. ;-)
@Andreas: :-) There is that. And it's better than my findIndex, which would only find and remove the first.
A perfect answer. Thanks you.
1

First of all, you don't need to use a jQuery each for this. Second, it's not a great idea to alter an array that you are operating on. If you're trying to remove elements from an array, use filter. Filter has the following signature:

someArray.filter(function(item, index, array) {
  // return a value that is truthy to keep an item or falsey to remove it
})

Filter returns a new array with only the values that match what you want. That means you don't mess with your original array, which is a good idea anyways. In your case it would look like this:

var filteredProducst = cart_products.filter(function(item) {
  return item.indexOf(product + "^")
})

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.