Remove element from array if given in argument

Question

I'm trying to get a hang of JavaScript (again) and so far it's not going great.

The challenge:

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.

Why doesn't the code below remove the required elements for the second test case?

function destroyer(arr) {
  console.log(arguments);

  for (var array_i = 0; array_i < arr.length; array_i++){
    for (var arg_i = 1; arg_i < arguments.length; arg_i++){
      console.log("indexes", array_i, arg_i);
      if (arr[array_i] === arguments[arg_i]){
        console.log(arr[array_i], arguments[arg_i], "destroyed");
        arr.splice(array_i, 1);
        console.log(arr, arguments);
        array_i = 0;
        arg_i = 1;
      }
    }
  }
  return arr;
}

It successfully works here:

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

But not here:

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

You reset array_i and arg_i when you delete something, but you reset them to the initial values before the for loop increment. Thus, you'll sometimes skip an element. Try resetting array_i to -1 and arg_i to 0. — Pointy
– Pointy, Commented Dec 28, 2015 at 18:09
'console.log(arguments);' where is the variable arguments coming from? — Ben Glasser
– Ben Glasser, Commented Dec 28, 2015 at 18:09

Paul Roub · Accepted Answer · 2015-12-28 18:11:32Z

3

You're trying to compensate for the elements deleted along the way by resetting your indices. It's much simpler (and actually works) if you delete from the end of the array. No resets needed.

function destroyer(arr) {

  for (var array_i = arr.length - 1; array_i >= 0; array_i--){
    for (var arg_i = 1; arg_i < arguments.length; arg_i++){
      if (arr[array_i] === arguments[arg_i]){
        arr.splice(array_i, 1);
        break;
      }
    }
  }
  return arr;
}

console.log( destroyer([2, 3, 4, 2, 3], 2, 3)  );   // [4]
console.log( destroyer([2, 3, 2, 3], 2, 3)  );      // []

answered Dec 28, 2015 at 18:11

Paul Roub

36.5k27 gold badges88 silver badges95 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Mast Over a year ago

Marvellous, I didn't think of that. Thanks!

jcubic · Accepted Answer · 2015-12-28 18:14:27Z

1

The problem is that you modify the array while you're looping.

You can use filter function to do the same:

function destroyer(arr) {
  var args = [].slice.call(arguments);
  return arr.filter(function(item) {
    return args.indexOf(item) == -1;
  });
}
document.getElementById('output').innerHTML = 
  JSON.stringify(destroyer([2,3,2,3], 2, 3)) + "\n" +
  
  JSON.stringify(destroyer([1,2,3,4,2,3], 2, 3))

<pre id="output"></pre>

edited Dec 28, 2015 at 18:14

answered Dec 28, 2015 at 18:11

jcubic

67.1k58 gold badges252 silver badges466 bronze badges

3 Comments

Mast Over a year ago

I knew my approach was wrong, this is probably the best way of solving it.

Xotic750 Over a year ago

Whether it matters or not, but this method creates a new array of items rather than modifying the original. I mention this because Remove all elements from the initial array

Mast Over a year ago

@Xotic750 Good catch. In this case it doesn't matter much, but it's not indeed making a new array. It would be interesting to see which answer is the most efficient one, but that's for another time and another question.

Xotic750 · Accepted Answer · 2015-12-28 18:39:30Z

An alternative to reverse iteration using ES5 methods, and modifying the original array.

function destroyer(arr) {
  var rest = Array.prototype.slice.call(arguments, 1);
  rest.forEach(function(unwanted) {
    var index = arr.indexOf(unwanted);
    while (index > -1) {
      arr.splice(index, 1);
      index = arr.indexOf(unwanted);
    }
  });
  return arr;
}

var initial1 = [2, 3, 2, 3];
destroyer(initial1, 2, 3);
document.getElementById('out1').textContent = JSON.stringify(initial1);
console.log(initial1);

var initial2 = [3, 5, 1, 2, 2];
destroyer(initial2, 2, 3, 5);
document.getElementById('out2').textContent = JSON.stringify(initial2);
console.log(initial2);

<script src="https://cdnjs.cloudflare.com/ajax/libs/es5-shim/4.4.1/es5-shim.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/json3/3.3.2/json3.min.js"></script>
<pre id="out1"></pre>
<pre id="out2"></pre>

Ashutosh Singh · Accepted Answer · 2015-12-28 18:40:49Z

The problem here is you are relying on length to loop through the element of array and simultaneously you are rearranging the elements of the array. So what happens is after removing the 2 and 3, 2 again comes at 0th place and your counter is ahead now and will never cover the position 0th. So there will be other case too in which you will not get the desired result.

So instead of this try looping through each argument only once, Select an argument and then search for it in the array.

for(var i = 1; i < arguments.length; i++)
{
    var index = arr.indexOf(arguments[i]);
    while(index > -1)
    {
        arr.splice(index, 1);
        index = arr.indexOf(arguments[i]);
    }
}

And you will get what you want.

Collectives™ on Stack Overflow

Remove element from array if given in argument

4 Answers 4

1 Comment

3 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

3 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related