1

I've been trying to write a function which takes in an array as the first argument, then one or more other arguments which are numbers. The purpose of the function is to check whether these numbers are present in the array and remove them if so.

I have tried the following but the results haven't been what I had expected. The desired outcome is that 3 and 2 be removed from the array leaving me with [1,4]. Instead, only 2 is removed with the end result being [1,3,4]. I've been struggling with this for a while and would appreciate any feedback you might be able to provide. I'm knew to this and this is the first problem which has left me stumped so far!

function test(myArray, ...checkNums) {
  for (let num in checkNums) {
    for (let num2 in myArray) {
      if (myArray[num] == checkNums[num2]) {
        myArray.splice(num, 1);
      }
    }
  }
  return myArray;
}

const arr = test([1, 2, 3, 4], 3, 2);
console.log({arr})
Ruan Mendes
  • 90,375
  • 31
  • 153
  • 217
  • You've mixed up *num* and *num2*, use `myArray[num2] == checkNums[num]`. But using *for..in* over arrays is discouraged because it visits all enumerable properties, not just indexes. If someone has added an enumerable non–numeric property you may get unexpected results. – RobG Jan 13 '23 at 15:54

4 Answers4

0

The easiest way is to just filter the array to only keep values not in checkNums. Using a Set gives better performance (depending on the implementation, lookup is either O(1) or O(log n) or anything sublinear for a Set, compared to O(n) for an Array).

function test(myArray, ...checkNums) {
  const checkNumsSet = new Set(checkNums);
  return myArray.filter((num) => !checkNumsSet.has(num));
}

const arr = test([1, 2, 3, 4], 3, 2);
console.log({arr})
Samathingamajig
  • 11,839
  • 3
  • 12
  • 34
  • I don't see why you'd need to convert it to a set, potentially a performance improvement but seems like premature optimization. – Ruan Mendes Jan 13 '23 at 15:47
0

Your code is removing items so your index variable is stale after you remove an element. The simplest fix is to just iterate backwards.

Also, you should avoid using for in to iterate over an array

Lastly, your array was just modifying what was passed in but you never kept a reference to it, I'm returning the modified array.

function test(myArray, ...checkNums) {
  for (let checkNumsIndex = checkNums.length - 1; checkNumsIndex >=0; checkNumsIndex--) {
    for (let myArrayIndex = myArray.length - 1; myArrayIndex >=0; myArrayIndex--) {
      if (myArray[myArrayIndex] == checkNums[checkNumsIndex]) {
        myArray.splice(myArrayIndex, 1);
      }
    }  
  }

  return myArray;
}

const arr = test([1, 2, 3, 4], 3, 2);
console.log({arr});

A more straight forward is using filter and includes. This doesn't have the problem that your example has where you're testing values outside of the bounds of the array.

function removeElements(myArray, ...checkNums) {
  return myArray.filter((num) => !checkNums.includes(num));
}

const arr = removeElements([1, 2, 3, 4], 3, 2);
console.log({arr});
Ruan Mendes
  • 90,375
  • 31
  • 153
  • 217
0

With myArray and checkNums as arrays, you can use a filter based on .includes:

const myArray = [1,2,3,4];
const checkNums = [3,4];

const filterNums = (nums, checkNums) => {
  return nums.filter(num => !checkNums.includes(num));
}

console.log(filterNums(myArray, checkNums));
Rhys Mills
  • 187
  • 7
0

You can use for...of see for..in vs for...of in order to iterate through your arguments, check if the number exist in your array and if yes, splice at index number

function test(myArray, ...checkNums) {
  //get the elements from ...checkNums
  for (let num of checkNums) {
    //check if your number exist in array
    if (myArray.includes(num)) {
      const indexOfNum = myArray.indexOf(num);
      //if it exists splice at found index of your umber
      myArray.splice(indexOfNum, 1)
    }
  }
  return myArray;
}

const result = test([1, 2, 3, 4], 3, 2);
console.log(result)
Mara Black
  • 1,666
  • 18
  • 23