Javascript TypeError: Cannot read property 'indexOf' of undefined

Question

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.

Answer 1

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);

Answer 2

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 + "^")
})