Filter array elements using variable number of filters

Question

This is a question generated from a coding challenge, and I solved it, but I'm just not sure it's a great solution, nor am I sure why the first way I was trying didn't work.

Challenge: Implement a function called destroyer(). "You will be provided with an initial array (the first argument in the destroyer function), followed by one or more arguments. Remove all elements from the initial array that are of the same value as these arguments."

Some test cases: - destroyer([1, 2, 3, 1, 2, 3], 2, 3) should return [1, 1] - destroyer(["tree", "hamburger", 53], "tree", 53) should return ["hamburger"]

So here is the function that passed the test.

  function destroyer(arr) {
    // make a temp array to store iterations of the loop used to filter
    var newArr = []; 

    // get the items to filter from the arguments passed.
    // EDIT: changed this for loop to use slice
    var filterItems = Array.from(arguments).slice(1);

    // use the array of items to filter, to filter the 'arr'
    // NOTE:I get a warning about calling a function in a loop. but it works in my lab environment anyhow.
    for (i=0; i < filterItems.length; i++){
      newArr = arr.filter(function(el){ // cannot assign directly to arr here for some reason???
       return el !== filterItems[i] }
     );
    arr = newArr;
    }



    return newArr;
  }

Okay, so calling the function with this, the array passed in argument 1 gets filtered, removing any numbers passed later.

destroyer([1, 2, 3, 1, 2, 3], 2, 3);

What didn't work, is using arr = arr.filter... to iterate over the array passed into the function. Instead, I first had to output the filter into the temp newArray and then reassign it in the for loop. That kind of confuses me. if I can change arr with arr = newArr. Why does that work and not the other way where I output the arr.filter right back into arr?

Answer 1

What didn't work, is using arr = arr.filter... to iterate over the array passed into the function

It works. But you would then need to return arr instead of newArr. Also, it is less readable as you re-assign the function's return value to the input parameter, which is confusing.

Further suggested improvements:

Naming:

• Rename newArr to e.g. result as this makes its role more clear.

Style:

• Array.from(arguments).slice(1) creates an unnecessary internal copy. Use e.g. Array.prototype.slice.call(arguments, 1). Some people prefer [].slice.call(arguments, 1)
• Don't make an unnecessary local copy of the arguments if performance is key.
• Since you already make a local copy, replace the for-loop with the functional style Array.includes for consistency's sake, since you already use Array.filter.
• Having a function call within a loop body is slow as the function call overhead is not marginal in JavaScript.
• Declare the loop iterator i as a local variable for (var i = ...

Applying those suggestions to your code gives us:

function destroyer(array) {
  var filter = Array.prototype.slice.call(arguments, 1);
  return array.filter(function(el) {
    return !filter.includes(el);
  });
}

console.log(destroyer([1, 2, 3, 1, 2, 3], 2, 3)); // [1, 1]
console.log(destroyer(["tree", "hamburger", 53], "tree", 53)) // ["hamburger"]

However, this function is inefficient for larger inputs as it iterates the filter array for each array element. It's time complexity is in O(mn) where m and n are the lengths of array and filter. You can improve on that by using a Set and come up with a linear time solution. Using rest parameters allows us to get rid of the argument slicing:

function destroyer(array, ...filter) {
  filter = new Set(filter);
  return array.filter(element => !filter.has(element));
}

console.log(destroyer([1, 2, 3, 1, 2, 3], 2, 3)); // [1, 1]
console.log(destroyer(["tree", "hamburger", 53], "tree", 53)) // ["hamburger"]

Answer 2

It works fine for me:

  function destroyer(arr) {
    // get the items to filter from the arguments passed.
    var filterItems = Array.from(arguments).slice(1);

    // use the array of items to filter, to filter the 'arr'
    for (i=0; i < filterItems.length; i++){
      arr = arr.filter(function(el) { 
       return el !== filterItems[i] }
     );
    }

    return arr;
  }
  
  console.log( destroyer([1, 2, 3, 1, 2, 3], 2, 3) );

But I would go for the simpler:

function destroyer(arr) {
    // get the items to filter from the arguments passed.
    var filterItems = Array.from(arguments).slice(1);
    return arr.filter(function(el) {
                         return filterItems.indexOf(el) < 0; 
                     } );
  }
  
  console.log( destroyer([1, 2, 3, 1, 2, 3], 2, 3) );