Very confused about splicing behavior within a loop

So while working at a solution to one of the basic javascript algorithm challenges I came across an issue that has me really confused.

The objective is to return the input array but without the elements that equate to a boolean value of false after type coercion (0, “”, NaN, undefined, null, false). Below is my code.

function falseblock(inputArr) {
for (let item of inputArr){
if (item == false) {
//console.log(item); //this line for debugging
inputArr.splice(inputArr.indexOf(item),1);
}
}
return inputArr;
}

console.log(falseblock([12, “apples”, “”, false, “testing”, “”, false, 9]));

Instead of the expected output of the input array without the “” and false elements, only the “” elements are removed:
[12, “apples”, false, “testing”, false, 9]

However, if the input array is changed to one with a false elements appearing first:
[12, false, “apples”, “”, false, “testing”, “”, false, 9]

Then in this case only the false elements are removed:
[12, “apples”, “”, “testing”, “”, 9]

If the splicing line is commented out, and the line i have marked for debugging above it is enabled, it correctly outputs a log for each of the “” and false elements of the input array regardless of order of appearance.

However, if BOTH the splicing line and “debugging” line are enabled. The console logs are only performed for whichever type of element appears first in the array.

So it seems to me that there’s something about having this splicing operation within the for loop (or perhaps in the logic statement?) that is causing the code to lock in the first element that matches to false as the only possible one or something.

Can anyone help me understand what is going on here and why this happens? I’ve never encountered this kind of behavior before.

P.S. I know there is a much simpler way to accomplish the objective, I’m just experimenting with different ways.

I think there’s two problems here

In general it’s a bad idea to mutate what you’re iterating over unless you know precisely in advance how the control flow could behave, which can get pretty messy pretty quickly depending on what’s going on

Otherwise there’s a bit of a common gotcha with comparing to false like that, consider the following:
false == false returns true
null == false returns false

instead you can directly use its falsy property or convert it to a boolean

Edit: After you’ve solved the problem in a few ways like you’re doing consider solving it using filter, that’s a pretty slick way to do it

1 Like

Well I’m no expert but here’s what I THINK is happening. I think for… of is doing something along the lines of this:

for (let i=0; i<Array.length; i++);

The problem with splicing in this way is that if you do array[i] to represent the items, after you splice them, things start getting wonky and you skip over values in the string. So I think with for… of, values are being skipped over after you splice them.

You’re better off pushing the true values into a new arr or using filter or something. I wouldn’t know how to go about using splicing in a loop that wouldn’t start making things weird.

1 Like

Ah, thanks for the tip about the comparisons! Initially I did have the if condition as (Boolean(item) == false) but I actually changed it to just (item == false) in an attempt to simplify it as I thought that type coercion would automatically accomplish the same thing after testing that “” == false returns as true.

With the test arrays I was using both of those conditions give the same results though, so it must just be some problem with performing mutations on the array in the middle of the iterations as you were saying.

I thought that maybe it had something to do with the fact that the indices of all the successive elements change every time one gets spliced out, but I don’t think that quite explains it since it does remove elements correctly, it just locks onto whichever type of element matches the if condition first and then only removes those.

And yeah, the filter method is for sure the best way to do this one. I just always like to try and figure out exactly why something isn’t working whenever I encounter an issue like this so that I get a better understanding of the language and can avoid making similar mistakes in the future.

Yes, that’s the real problem, consider putting:

console.log(`inputArr: ${inputArr} - item: ${item}`)

inside the loop and you’ll see it in action

1 Like

Wow yeah I see what you mean. It definitely isn’t behaving predictably. I tried experimenting around with different input arrays and looks like it’s just jumping around sometimes, like sometimes it would remove more than one type of non-true value but it wouldn’t do all of them. It seems to work for removing all instances of one particular element in an array (for example all 5’s or something) but anything more that that seems to cause problems.

I think the removing of 1 instance is just by chance. Like, as an extreme example, if you were to plug in:

falseblock([false,false,false,false,false,false,false,false,false,false,false,false,false,false,false,false,])

some falses would remain while others wouldn’t. Which leads me to believe it’s just skipping over values like this:

vals = [0,1,2,3,4]
for (let i=0; i<vals.length; i++) {
if (vals[i] > 1) {vals.splice(i,1)};
}

so this function wants to remove any value in vals that is greater than 1. When it gets to 2, it removes 2 because the i = 2 and it’s splicing at index i. Now vals looks like this

[0,1,3,4]

i = 2 and now i is incremented by 1 so i = 3. Vals[3] = 4 now, and we skipped over 3. So 4 will be removed and we’ll be left with vals = [0,1,3]. Hope this makes sense.

1 Like

@habit456 said

But there is a way: start from last index and check i == 0 last

Or don’t splice and instead use push. Or filter, Using splice like this, as you didn’t create a local copy, is bad practice because you are mutating the input array (please create a local copy).