for loop skipping one item in array in javascript

Question

I am trying to make a function that removes strings from an array if they include a character in a certain other list

Here is the code:

var possible = ["salutations", "goodbye", "thanks", "welcome"];
var incorrect = ["o"];

console.log(possible);

function narrowdown(possible, incorrect)
{
    var templist = possible;
    for (i in possible)
    {   
        console.log(i + " " + possible[i]);
        var array1 = possible[i].split("");
        var common = array1.filter(value => incorrect.includes(value));
        console.log(common)
        if (common.length)
        {
            templist.splice(i, 1);
        }
    }
    possible = templist;
}

narrowdown(possible, incorrect);

console.log(possible);

Here I am trying to remove all the words that include the letter o. I created a temporary array in the function because it has happened to me before that a for loop skips items altogether. The code first logs the index of the item in the list and then the item itself.

Then it turns the word into an array and checks for overlap between it and the "incorrect" array. It does that correctly and logs the overlapping characters. The issue seems to be that it skips over the "goodbye" item for some reason. It doesn't even process it.

Here is the output I am getting:

[ 'salutations', 'goodbye', 'thanks', 'welcome' ]
0 salutations
[ 'o' ]
1 thanks
[]
2 welcome
[ 'o' ]
[ 'goodbye', 'thanks' ]

Really the `.filter()` method is exactly the tool for this job. — Pointy, Aug 06 '20 at 12:21
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) — Heretic Monkey, Aug 06 '20 at 12:34

score 2 · Accepted Answer · answered Aug 06 '20 at 12:15

First of all, for (i in possible) is a bad way of looping through an array since it retrieves the keys before the loop begins and it never updates that list of keys. Also, if someone assigns an attribute to the array, like possible.foo = 17, then your loop will also go through that. The issue you're having is that when you splice the array, everything else is shifted one to the left, changing their indices to be one less, so your new index actually skips over the next element. The fix is to use a conventional for loop and decrement i after splicing:

for (let i = 0; i < possible.length; i ++) {
    // more code...
    if (common.length) {
        templist.splice(i, 1);
        i --;
    }
}

Thank you! This is what solved it for me. I used `for i in possible` because I was a bit lazy and I thought it didn't make a difference, plus I am used to Python. — Boopiester, Aug 06 '20 at 12:23

score 0 · Answer 2 · answered Aug 06 '20 at 12:15

The problem is that the array is re-indexed when you splice. One solution is to iterate in reverse:

var possible = ["salutations", "goodbye", "thanks", "welcome"];
var incorrect = ["o"];

console.log(possible);

function narrowdown(possible, incorrect)
{
    var templist = possible;
    var i = possible.length;
    while (i--)
    {   
        console.log(i + " " + possible[i]);
        var array1 = possible[i].split("");
        var common = array1.filter(value => incorrect.includes(value));
        console.log(common)
        if (common.length)
        {
            templist.splice(i, 1);
        }
    }
    possible = templist;
}

narrowdown(possible, incorrect);

console.log(possible);

M0nst3R · Answer 3 · 2020-08-06T12:30:23.333

If you're looking for a less "imperative" version of this function, you can make use of Array.filter(), Array.some(), and String.includes() to make a one-liner function that does the trick (for formatting purposes, I split it in the multiple lines in the snippet below).

const possible = ['salutations', 'goodbye', 'thanks', 'welcome'];

function narrowDown(possible, incorrect) {
    return possible.filter((value) => (
        !incorrect.some((exclude) => (value.includes(exclude)))
    ));
}

console.log('["o"]', JSON.stringify(narrowDown(possible, ['o'])));
console.log('["a"]', JSON.stringify(narrowDown(possible, ['a'])));
console.log('["a", "o"]', JSON.stringify(narrowDown(possible, ['a', 'o'])));

Stephane Vanraes · Answer 4 · 2020-08-06T12:27:17.060

The issue comes from the line tempList = possible which is an assignment by reference, meaning that when you do the splice operation you do it on both arrays at the same time.

Generally it is considered bad form to manipulate data like that anyway, you should have narrowDown return a value that you re-assign to possible instead of filtering them in place. If you do that you can also leverage some of the newer array methods:

var possible = ["salutations", "goodbye", "thanks", "welcome"];
var incorrect = ["o"];

function narrowdown(possible, incorrect)
{
    return possible.filter(item => !incorrect.some(test => item.includes(test)))
}

possible = narrowdown(possible, incorrect);

console.log(possible);

using the .some will also exit early as soon as there is a match, instead of looping over all incorrect values and the entire string, boosting performance slightly

This is incorrect, if you have `var incorrect = ['o', 'a'];`, then instead of nothing returning, you get the entire array except `'salutations'`. The negation should be behind the `.some()` call, not the `.includes()` call. — M0nst3R, Aug 06 '20 at 12:25
true, I copied the ! wrong from my experiment, edited to the correct version — Stephane Vanraes, Aug 06 '20 at 12:27

for loop skipping one item in array in javascript

4 Answers4