Seek and Destroy : getting a warning "don't make functions within a loop" -- but all tests pass

Seek and Destroy : getting a warning "don't make functions within a loop" -- but all tests pass
0.0 0
#1

How do I refactor this to remove that fCC editor warning “don’t make functions within a loop”

I tried an external function, but don’t know how to pass a 2nd argument (variable arg) to that filter function.

It’s passing all the fCC tests and browser console is not giving that warning.

function destroyer(arr) {
  // Remove all the values in array [0]
  // function may have multiple arguments.... ([0],1,2,3,4,5)
  
  // arguments[0] will always contain the array
  // arguments[1..n] will be additional parameters passed to the function  
  theArray = arguments[0];
  output = [];  // output of function is an array
  var arg;      // we need this temp variable for the function inside the for..loop

  for (i=1; i<arguments.length; i++){ // we loop through arguments array, but skipping [0] index 
    arg = arguments[i];
    theArray = theArray.filter(function(val){ return arg !== val;});
  }
  return theArray;
}

// destroyer([1, 2, 3, 1, 2, 3], 2, 3);
// destroyer([1, 2, 3, 5, 1, 2, 3], 2, 3);
// destroyer([3, 5, 1, 2, 2], 2, 3, 5);
destroyer(["tree", "hamburger", 53], "tree", 53);
#2

Just do what it says and don’t make functions within a loop. Instead, move the function outside of the for loop, give it a name and call it within filter. I took out your comments, so you could easily see the code I added.

function destroyer(arr) {
  function argFilter(val){ return arg !== val;}
  theArray = arguments[0];
  output = [];
  var arg;     
  for (i=1; i<arguments.length; i++){
    arg = arguments[i];
    theArray = theArray.filter(argFilter);
  }
  return theArray;
}
#3

Why not just filter on arguments (or some subset of arguments?) directly without a loop at all?

I consider it a bit of an anti-pattern to mix for loops and map/filter/reduce.

#4

Yes, that is an option and is the way I approached the challenge. However, based on how the OP had started the solution, I gave an answer that kept the code basically unchanged.

function destroyer(arr, ...args) {
    return arr.filter( val =>  !args.includes(val));
}
#5

Thanks @rmdawson71. That did the trick.

I was originally defining the filter function outside the destroyer() function. And that’s why it wouldn’t work. Thanks again

#6

Okay, that’s some voodoo magic going in there…

what do you call that concept? …args and the => so I can google it. Thanks.

#7

It is new in ES6. The … is called the spread operator and => is for arrow functions.

#8

Why do we need to use this output though?

#10

@wuzrak - I am not sure what you are asking.

#11

The road to callback hell is paved with well-intentioned anonymous functions.

#12

There is “output = [];” in the code, I see that it isn’t necessary for the function to work, in fact, I don’t understand what purpose it holds.

#13

When I was answering the OP’s question, I simply copied/pasted the OP’s code and moved the creation of the function used in the filter out of the for loop. His code worked as it was and I only wanted to show the OP a different way of using the same code. I did not even notice the output = []; part until you just mentioned. You are correct it has no purpose in the code and could be removed. It is probably some code the OP had intended on using in some way, but forgot to remove it when the solution worked without actually using the line of code…