Why isn't my javascript iterating through all of the arguments?

Question

I'm working on The Odin Project and am on the Fundamentals Part 4 "removeFromArray" assignment. I have to return an array having removed all of the elements that are in the list of arguments. It works with two arguments, but I can't get it to work with four. My code is below:

const removeFromArray = function(firstArray, ...toRemove) {
    let modifiedArray = firstArray;

    for (i = 0; i < firstArray.length; i++) {
        if (modifiedArray.includes(toRemove[i])) {
            modifiedArray.splice(modifiedArray.indexOf(toRemove[i]), 1)
        }    
    }
    return modifiedArray;

}
removeFromArray([1, 2, 3, 4], 7, 2) // works, returns [1, 3, 4]
removeFromArray([1, 2, 3, 4], 3, 2) // also works, returns [1, 4]
removeFromArray([1, 2, 3, 4], 1, 2, 3, 4) //does not work, returns [3, 4]

Any help is much appreciated.

do you need to return the original array reference or can you return a new array? — Nina Scholz, Feb 23 '20 at 21:04
You're using `firstArray.length` in your for loop but surely that should be `toRemove.length` instead? You can also use `return firstArray.filter(el => !toRemove.includes(el));` — , Feb 23 '20 at 21:08
I think I can return a new array. And you're right, Chris, I've since changed that in my code. — Carl D'Oleo-Lundgren, Feb 23 '20 at 21:33

score 2 · Answer 1 · answered Feb 23 '20 at 21:05

2

Splicing from the array shifts all the remaining elements down by one, so you end up skipping over the next element. I'd recommend using Array.prototype.filter instead.

answered Feb 23 '20 at 21:05

snek

1,980
1
15
29

score 1 · Answer 2 · answered Feb 23 '20 at 21:05

1

 1, 2, 3, 4

After you remove the first item, everything suffles down, so what was the second item becomes the first item.

You don't have that problem if you start at the end and work in reverse (sort the list of items to remove).

answered Feb 23 '20 at 21:05

Quentin

914,110
126
1,211
1,335

But given OP's code, does it really matter that the array shrinks during the loop? Once the small error in the loop condition is fixed, the code works fine. – Feb 23 '20 at 21:12

score 1 · Answer 3 · answered Feb 23 '20 at 21:12

I've changed the following line:

let modifiedArray = firstArray;

to

let modifiedArray = [...firstArray];

The problem you were facing is you were iterating over an array assuming it is a copy of the firstArray. But it was just pointing to firstArray. Now in the loop you startet removing items from the array and after you removed 2 in the iteration you were already on 2 so there was nothing left to iterate over...

const removeFromArray = function(firstArray, ...toRemove) {
    let modifiedArray = [...firstArray];
    for (var i = 0; i < firstArray.length; i++) {
        if (modifiedArray.includes(toRemove[i])) {
            modifiedArray.splice(modifiedArray.indexOf(toRemove[i]), 1)
        }    
    }
    return modifiedArray;

}
var x = removeFromArray([1, 2, 3, 4], 7, 2);
var y = removeFromArray([1, 2, 3, 4], 3, 2);
var z = removeFromArray([1, 2, 3, 4], 1, 2, 3, 4);

console.log('x', x);
console.log('y', y);
console.log('z', z);

Someone else shared this same solution and this is exactly what I needed. I had no idea that by setting modifiedArray = firstArray that i was only pointing to the first array and modifying it with my code. The other solutions change my approach, but this lets me take the approach I wanted. Thank you! — Carl D'Oleo-Lundgren, Feb 23 '20 at 21:31

kevin · Accepted Answer · 2020-02-23T21:54:37.597

I think the real issue here, is that you may not be aware that arrays, objects, and functions in JavaScript are passed by reference.

    let modifiedArray = firstArray;

Meaning your firstArray and modifiedArray are both pointing to the same array in memory. The code above assigns the EXACT same array, technically the address in memory of the firstArray to the modifiedArray. Therefore, as you remove items from the modifiedArray, you are also removing them from the firstArray, and therefore changing the length of the firstArray.

You need to copy the array by value, not by reference.

Solution:

Therefore changing

let modifiedArray = firstArray;

to

let modifiedArray = [...firstArray];

or

let modifiedArray = firstArray.slice();

The first solution leverages destructuring of the first array, to create a copy of the array by value, not pointing to the same array in memory.

The second may be more familiar to you as a beginner, since this simply returns a copy of the array, without removing any elements.

See this thread if you have more questions about copying arrays by value: Copy array by value

const removeFromArray = function(firstArray, ...toRemove) {
    let modifiedArray = [...firstArray];

    for (i = 0; i < firstArray.length; i++) {
        if (modifiedArray.includes(toRemove[i])) {
            modifiedArray.splice(modifiedArray.indexOf(toRemove[i]), 1)
        }    
    }
    return modifiedArray;
}

console.log(removeFromArray([1, 2, 3, 4], 7, 2)); // works, returns [1, 3, 4]
console.log(removeFromArray([1, 2, 3, 4], 3, 2)); // also works, returns [1, 4]
console.log(removeFromArray([1, 2, 3, 4], 1, 2, 3, 4)); //does not work, returns [3, 4]

This is true and a problem of OP's code if the original array is supposed to remain unchanged, but doesn't answer OP's question. OP uses the wrong length in the first place, and if that is fixed, the changing length doesn't matter. — , Feb 23 '20 at 21:14
This is marvelous! This piece of knowledge is exactly what I was missing, and you're great for pointing it out, because it was an unknown-unknown that I wouldn't have known to ask about in the first place. Thank you. — Carl D'Oleo-Lundgren, Feb 23 '20 at 21:28
@ChrisG Not sure how you mean he is using the wrong length? That simple change does indeed work in the code sample I've added. — kevin, Feb 23 '20 at 21:29
@CarlD'Oleo-Lundgren Yay! Glad I could be of help :) I only know, because we all ran into this confusion in our journey as well. Best of luck on your javascript journey, and welcome to the club! — kevin, Feb 23 '20 at 21:36
@CarlD'Oleo-Lundgren PS. just in case you weren't aware, Objects and Functions are also passed by reference by default as well. — kevin, Feb 23 '20 at 21:42

score 0 · Answer 5 · answered Feb 23 '20 at 21:08

You take the same index for the array and for the items to remove, which may not be the same.

You could iterate the item to remove and get the index and splice this item from firstArray.

This approach muates the given array and keeps the same object reference.

const removeFromArray = function(firstArray, ...toRemove) {
    for (let i = 0; i < toRemove.length; i++) {
        let index = firstArray.indexOf(toRemove[i]);
        if (index === -1) continue;
        firstArray.splice(index, 1);
    }
    return firstArray;

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

score 0 · Answer 6 · answered Feb 23 '20 at 21:12

0

const removeFromArray = (firstArray, ...toRemove) => 
  firstArray.filter(e => !toRemove.includes(e));
      
console.log(removeFromArray([1, 2, 3, 4], 7, 2));

answered Feb 23 '20 at 21:12

Đinh Carabus

3,403
4
22
44

Why isn't my javascript iterating through all of the arguments?

6 Answers6