# Seek and Destroy flyin

Tell us what’s happening:
Just wondering if anyone can help me understand why this doesn’t push the values to the array ‘c’.

Thank you,

``````
function destroyer(arr) {
let a = Array.from(arguments);
let b = a[0];
a.splice(0,1);
let c = [];
a.forEach((x) => b.indexOf(x) < 0 ? c.push(x):'');
console.log(c);
return c;
}

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

User Agent is: `Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0`.

Let’s examine what is happening inside the forEach with the following test case.

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

Before the forEach, a is [2, 3]

In the first iteration of the forEach, x = 2 and b is [ 1, 2, 3, 1, 2, 3 ], so b.indexOf(2) returns 1, so the push does not get called.

In the second iteration of the forEach, x = 3 and b is [ 1, 2, 3, 1, 2, 3 ], so b.indexOf(3) returns 2, so the push does not get called.

This is why c ends up still an empty array.

FYI - It is best practice not to use ternary operator as a direct subsitution for an if statement, because an if statement make the code more readable, plus a ternary’s main purpose is to be used when assigning a value and not just making a conditional statement.

``````if (b.indexOf(x) < 0)  c.push(x);
``````

is more readable and the following even makes it clearer, because it blocks the code.

``````if (b.indexOf(x) < 0) {
c.push(x);
}
``````

Do not confuse concise for better in this case. Readable code which accomplishes the same thing is always better.

@RandellDawson I had the array comparison backward. Also, thank you for the advice! I rewrote the function below. Does this seem like an appropriate solution? I don’t just want to just pass the challenges but have a clearer understanding in writing good, efficient, and effective code as well.

Thank you!

``````function destroyer(arr) {
let a = Array.from(arguments);
let b = a[0];
a.splice(0,1);
let c = [];
b.forEach(function(x) {
if (a.indexOf(x) < 0) {
c.push(x);
}
});
console.log(c);
return c;
}

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

I suggest given your variables more meaningful names. Variable names should describe what they contain. It will make your code more readable. Also, think how you might use the rest syntax (`...`) in your function’s declaration to simplify the following 3 lines of code:

``````let a = Array.from(arguments);
let b = a[0];
a.splice(0,1);
``````

@RandellDawson That took me a bit to understand but that works really well. This is how I modified the declaration and it allows me to completely delete the 3 lines. Thank you for your very instructive comments!!

``````function destroyer(b, ...a) {
``````

I still would recommend changing your variables a and b to names which reflect what they contain. May a would be args and b would be arr?

@RandellDawson So something more like this for a final solution?

``````function destroyer(arr, ...args) {
let c = [];
arr.forEach(function(x) {
if (args.indexOf(x) < 0) {
c.push(x);
}
});
console.log(c);
return c;
}

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

What about variable c? What does it contain at the end?

@RandellDawson I guess the best description is a cleaned array

``````function destroyer(arr, ...args) {
let clean = [];
arr.forEach(function(x) {
if (args.indexOf(x) < 0) {
clean.push(x);
}
});
return clean;
}
``````
1 Like

I will guide you to a more concise solution. What if you renamed clean to filtered, because it really is a filtered version of arr. Now think how you could use the actual Array.prototype.filter method on arr to accomplish what all of the following lines do:

``````  let clean = [];
arr.forEach(function(x) {
if (args.indexOf(x) < 0) {
clean.push(x);
}
});
``````

@RandellDawson This is what I came up with using the filter() function. It seems like this could still be more concise.

``````function destroyer(arr, ...args) {
let filtered = arr.filter(function(element) {
return args.indexOf(element) === -1;
});
return filtered;
}

``````

It can be:

replace === -1 with < 0 OR research the includes method.

Also, do you even need to declare filtered? Why not just return the filtered array without creating the extra variable. If you want it more concise, then you arrow function syntax.

@RandellDawson I like this solution. I’ll look at includes method also.

``````function destroyer(arr, ...args) {
return arr.filter((element) => args.indexOf(element) < 0);
}
``````

Use arrow function syntax for the destroyer function also.

@RandellDawson Is it most appropriate to use let versus const or var?

``````let destroyer = (arr, ...args) => arr.filter((element) => args.indexOf(element) < 0);
``````

For functions, arrays, and objects, you should typically go with const to prevent any reassignment, though there could be some special situations where let is needed if you need to reassign an array or object. The only reason you would use var anymore is if you are writing code for browsers which do not recognize the const and let keywords.

Did you not figure out how to use includes yet?

It’s not working for me yet. I haven’t figured out how to correctly implement it.

@RandellDawson Here it is using includes .

``````const destroyer = (arr, ...args) => arr.filter((element) => !args.includes(element));
``````

That is about as concise and still readable as you could probably make it using this specific algorithm unless you renamed element to elem.

Thank you again for help with this!