Javascript Question posted in
A very good W3school tutorial can be found here.

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

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]

Tags:

3

Answers


  1. 0

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

    Login or Signup to reply.
  2. 0

    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));
    Login or Signup to reply.
  3. 0

    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.

    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search