Dose splice works when the second parameter is an array.length?

Question

I want to implement a function that takes an array and some other arguments then removes the other arguments from that array. So i used the rest parameter for the second argument then used Array.from to convert it an array, then used .length to get its length to use it as the second parameter for splice and it doesn't works. I know there are better ways to solve this problem, but I want to why it's not working .

const removeFromArray = function(array, ...number) {
    let numArray = Array.from(number)

    for(let i = 0; i < array.length  ; i++ ){
       if(number == array[i]){
        array.splice( i , numArray.length);
       }
    }
    return array;
};

removeFromArray([2, 1, 2, 3, 4], 1, 2);

expecting the output to be: [3, 4]

but it's: [2, 1, 2, 3, 4]

Answer 1

const removeFromArray = function(array, ...numbers) { let numArray = Array.from(numbers);

for (let i = 0; i < numArray.length; i++) {
    let index = array.indexOf(numArray[i]);
    while (index !== -1) {
        array.splice(index, 1);
        index = array.indexOf(numArray[i]);
    }
}
return array;

};

console.log(removeFromArray([2, 1, 2, 3, 4], 1, 2));

Answer 2

number == array[i] will never be true. Since number is an Array and you are comparing it with a single number (even if array[i] were an Array, the result would be false, since arrays aren't compared like that, in javascript).

If you want to remove values from an Array an easy solution would be to use the filter function (link to documenation).

function removeFromArray(array, ...numbers) {
    return array.filter(v => numbers.indexOf(v) == -1);
};
    
console.log('result:', removeFromArray([2, 1, 2, 3, 4], 1, 2));

Short sidenote: your function uses splice, which changes the underlying array, the solution I presented only creates a new array. So If you need the passed array the be altered, this solution is not for you.

Answer 3

As pointed out by winner_joiner, the easier solution in this case is to use the built-in .filter method, so this is just provided as a learning exercise.

---

This is a demonstration of why naming variables carefully is so useful, as the bug becomes more obvious if we rename them like this:

const removeFromArray = function(targetArray, ...numsToRemove) {
    for(let targetArrayIdx = 0; targetArrayIdx < targetArray.length  ; targetArrayIdx++ ){
       if(numsToRemove == targetArray[targetArrayIdx]){
        targetArray.splice(targetArrayIdx, numsToRemove.length);
       }
    }
    return targetArray;
};
console.log('result:', removeFromArray([2, 1, 2, 3, 4], 1, 2));

(Note that I've removed the Array.from call, because JavaScript rest parameters are already an array. So numsToRemove replaces both number and numArray in the original code.)

Two things hopefully now stand out:

• if(numsToRemove == targetArray[targetArrayIdx]) is comparing a whole list of numbers to a single item in the target array
• if we corrected that to find a matching item, targetArray.splice(targetArrayIdx, numsToRemove.length); is going to delete as many items as are in the numsToRemove list every time

What we want instead is:

• Test if the single item is included in the list of numsToRemove, using numsToRemove.includes(value).
• Remove the single item we've found, by always passing 1 to targetArray.splice:

const removeFromArray = function(targetArray, ...numsToRemove) {
    for(let targetArrayIdx = 0; targetArrayIdx < targetArray.length  ; targetArrayIdx++ ){
       if(numsToRemove.includes(targetArray[targetArrayIdx])){
        targetArray.splice(targetArrayIdx, 1);
       }
    }
    return targetArray;
};
console.log('result:', removeFromArray([2, 1, 2, 3, 4], 1, 2));

This leaves a more subtle bug: as we delete items, the remaining items in the array move; so after we delete the 2 at index 0, the 1 moves to index 0 - but we carry on with the for loop, and don't check index 0 again.

We can avoid this by iterating the loop backwards, instead of forwards:

const removeFromArray = function(targetArray, ...numsToRemove) {
    for(let targetArrayIdx = targetArray.length-1; targetArrayIdx >= 0; targetArrayIdx-- ){
       if(numsToRemove.includes(targetArray[targetArrayIdx])){
        targetArray.splice(targetArrayIdx, 1);
       }
    }
    return targetArray;
};
console.log('result:', removeFromArray([2, 1, 2, 3, 4], 1, 2));

Now when we remove an item, the only items that move are ones we've already checked, so we safely check each item once.

Answer 4

The fastest way is to use an object to keep your numbers to remove:

const removeFromArray = function(array, ...number) {

    const set = {};
    for(let i = 0; i < number.length; i++){
      set[number[i]] = true;
    }

    for(let i = 0; i < array.length  ; i++ ){
        set[array[i]] && array.splice(i--, 1);
    }
    return array;
};

console.log(removeFromArray([2, 1, 2, 3, 4], 1, 2));

The more items in the both arrays, the more the object lookup dominating over Array::includes() which is O(n^k):

` Chrome/121
-----------------------------------------------------------------------------
>                n=10      |     n=100     |     n=1000      |    n=10000    
Alexander   3.40x x10m 686 | 1.00x x1m 135 | 1.00x   x1m 805 | 1.00x x10k  84
IMSoP       1.00x x10m 202 | 1.90x x1m 256 | 3.32x x100k 267 | 3.27x x10k 275
-----------------------------------------------------------------------------
https://github.com/silentmantra/benchmark `

const $chunk = () => Array.from({length:10}, () => Math.random()*10|0);
const $input = [];

// @benchmark Alexander
const removeFromArray = function(array, ...number) {

    const set = {};
    for(let i = 0; i < number.length; i++){
      set[number[i]] = true;
    }

    for(let i = 0; i < array.length  ; i++ ){
        set[array[i]] && array.splice(i--, 1);
    }
    return array;
};
removeFromArray($input, 1, 2, 3, 4, 5);

// @benchmark IMSoP
{
const removeFromArray = function(targetArray, ...numsToRemove) {
    for(let targetArrayIdx = targetArray.length-1; targetArrayIdx >= 0; targetArrayIdx-- ){
       if(numsToRemove.includes(targetArray[targetArrayIdx])){
        targetArray.splice(targetArrayIdx, 1);
       }
    }
    return targetArray;
};

removeFromArray($input, 1, 2, 3, 4, 5);
}

/*@end*/eval(atob('e2xldCBlPWRvY3VtZW50LmJvZHkucXVlcnlTZWxlY3Rvcigic2NyaXB0Iik7aWYoIWUubWF0Y2hlcygiW2JlbmNobWFya10iKSl7bGV0IHQ9ZG9jdW1lbnQuY3JlYXRlRWxlbWVudCgic2NyaXB0Iik7dC5zcmM9Imh0dHBzOi8vY2RuLmpzZGVsaXZyLm5ldC9naC9zaWxlbnRtYW50cmEvYmVuY2htYXJrL2xvYWRlci5qcyIsdC5kZWZlcj0hMCxkb2N1bWVudC5oZWFkLmFwcGVuZENoaWxkKHQpfX0='));