4

I am using splice to remove an element from an array, but it's not working. as far as I know, the code looks okay , but maybe I am missing/overseeing something. please take a look. here 'state' is an array containing objects.

 let removeFromState = state;
            for(var i=0;i<removeFromState.length;i++){
                if(removeFromState[i].index===action.index){
                    removeFromState.splice[i,1];
                }
            }
            return removeFromState;

I cant believe, i was being so silly. and i had been looking at it for quite a while but didnt see it right in front of me. but I am glad i posted it here, because of the remarks about missing entries because I was increasing 'i' even though I was removing entries.

faraz
  • 2,603
  • 12
  • 39
  • 61
  • Use `()` instead of `[]` – Eddie Feb 28 '18 at 02:47
  • 1
    Does this answer your question? [Looping through array and removing items, without breaking for loop](https://stackoverflow.com/questions/9882284/looping-through-array-and-removing-items-without-breaking-for-loop) – gre_gor Mar 01 '23 at 02:38

6 Answers6

13

There are two issues:

  1. A typo, you're using [i,1] (square brackets) where you should be using (i,1) (parentheses). Square brackets are property accessors. (The reason this isn't a syntax error is that JavaScript has a comma operator, and [i,1] evaluates to [1]. The reason it's not a runtime error is that functionname[1] looks up the property "1" on the function — and then disregards any value it finds.)

  2. Your loop will skip entries after the entries it removes, because you both remove the entry and increase i. So when you remove entry #5, entry #6 becomes #5 — but then you move on to i == 6. So you never look at the new entry #5.

To fix #2, either only increase i if you don't remove the entry, or loop backward.

So either:

var i = 0;
while (i < removeFromState.length) {
    if(removeFromState[i].index === action.index) {
        removeFromState.splice(i, 1);
    } else {
        ++i;
    }
}

or

for (var i = removeFromState.length - 1; i >= 0; --i) {
    if(removeFromState[i].index === action.index) {
        removeFromState.splice(i, 1);
    }
}

Or alternately, create a new array containing only the entries you want to keep:

var newArray = removeFromState.filter(function(entry) { return entry.index !== action.index; });
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
2

splice is a method, you call it with parentheses, not square brackets. Square brackets are for indexing (JavaScript is just excessively relaxed and silently returns undefined when you "index" the method). Try:

removeFromState.splice(i, 1);
ShadowRanger
  • 143,180
  • 12
  • 188
  • 271
2

I was stuck on the problem for couple of hours. I did figure out that when you splice an array it does returns the element that was spliced from the original array.

let  originalArray = ["Apples","Oranges","Kiwi"];
let newArray = originalArray.splice(0,1);
console.log(newArray);

I was expecting the modified originalArray in the spliced variable but instead it returns the element that was spliced. The answer was to

console.log(originalArray);

Whatever splice returns, we might not need that, we need to check our originalArray since it's the one that got sliced. The originalArray consists my expected answer which was ["Oranges","Kiwi"]

Sagar Khatri
  • 575
  • 3
  • 16
  • How it that relevant to the question? OP is not saving the result of splice anywhere. And [this behavior is clearly documented](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice). – gre_gor Mar 01 '23 at 11:51
1

Mistake was: you used square brackets instead of parenthesis

function remove(state, action) {
  let removeFromState = state;

  for (var i = 0; i < removeFromState.length; i++) {
    if (removeFromState[i].index === action.index) {
      removeFromState.splice(i, 1);
      i--;    // After deleting, counter reduced
    }
  }
  return removeFromState;
}

console.log(remove([{index: 3}, {index: 3}, {index: 3}], {index: 3}));
Abhijit Kar ツ
  • 1,732
  • 1
  • 11
  • 24
0

The problem is how you are calling splice, it is a function and needs to be called like splice(i, 1)

You could simplify the function as below

let removeFromState = state;
const findEntry = entry => entry.index === action.index;
removeFromState.splice(removeFromState.findIndex(findEntry), 1);
return removeFromState;
Jiby Jose
  • 3,745
  • 4
  • 24
  • 32
-1

removeFromState is instantiated as a let constant and therefore not mutable. It should work if let removeFromState is changed to var removeFromState (.