using .foreach to remove one array from another

Question

I was attempting to use .forEach to remove the contents of one array(ignore) from another array(input).

As an example in the below I would expect the input array to contain "b" and "h" however it contains "g" and "h" when I run this.

curious why I am not getting my expected results and if this is a good method.

var input = ["a", "b", "g", "h"],                                               
    ignore = ["b", "h"];                                                        

var newInput = function(element, index, array){                                 
    if (input.indexOf(ignore[index]) > -1){                                     
       input.splice((ignore[index]), 1)                                        
    }                                                                           
}                                                                               

  ignore.forEach(newInput);

With ignore set as ["b", "h"] I am expecting input to contain ["a", "g"] — imnothardcore
– imnothardcore, Commented Sep 15, 2015 at 4:49
@imnothardcore : did you dry run your code? I did and same you given code will give you the ["b", "h"] — Muhammad Usman
– Muhammad Usman, Commented Sep 15, 2015 at 4:58
ignore[index] is precisely equal to element, right? You can't pass the element to splice, you have to pass the position. — user663031
– user663031, Commented Sep 15, 2015 at 6:33

Mauricio Poppe · Accepted Answer · 2015-09-15 05:09:15Z

1

You should use Array.prototype.filter instead

var input = ['a', 'b', 'g', 'h']
var ignore = ['b', 'h']
var newInput = input.filter(function (el) {
  return ignore.indexOf(el) === -1
})

Note that input remains unchanged

answered Sep 15, 2015 at 5:09

Mauricio Poppe

4,8861 gold badge25 silver badges30 bronze badges

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

Comments

bentinata · Accepted Answer · 2015-09-15 06:38:10Z

1

Replace your conditionals with input.indexOf(element) > -1.

Since you are forEach-ing the ignore array, ignore[index] is the same as element.

And change splice to splice(input.indexOf(element), 1).

Array.splice() takes index as first arguments, and you are using the element instead.

edited Sep 15, 2015 at 6:38

user663031

answered Sep 15, 2015 at 4:54

bentinata

3341 gold badge5 silver badges14 bronze badges

Comments

user663031 · Accepted Answer · 2015-09-15 06:44:15Z

1

Array#filter is probably the best approach, but if you want to remove the ignored elements in-place, as I infer from your use of splice, you can do:

function ignore_elements(input, ignore) {
  var i = 0;
  while (i < input.length)
    if (ignore.indexOf(input[i]) !== -1) input.splice(i, 1);
    else i++;
}

By looping over the elements in the input, instead of ignore, you can more easily eliminate all occurrences of the elements to be ignored, not just the first one.

answered Sep 15, 2015 at 6:44

user663031

2 Comments

imnothardcore Over a year ago

I have seen some discussion of people suggesting a while and for loop over foreach and filter. what advantages does it offer? Also I like your idea of looping the input to get them all. Not needed in my application of this but probably better practice. Thanks

user663031 Over a year ago

There's no hard and fast rule. However if you're filtering or mapping then filter and map are typically more succinct and readable.

Jaromanda X · Accepted Answer · 2015-09-15 06:51:06Z

1

var input = ["a", "b", "g", "h"],
    ignore = ["b", "h"];                        

var newInput = function(element, index, array){
    if (input.indexOf(ignore[index]) > -1) {
       input.splice(input.indexOf(ignore[index]), 1)
//                  ^^^^^^^^^^^^^  added this
    }
} 

ignore.forEach(newInput);

but that code could be cleaner, e.g. ignore[index] is element

var input = ["a", "b", "g", "h"],
    ignore = ["b", "h"];

var newInput = function(element) {
    var index = input.indexOf(element);
    if (index > -1){
       input.splice(index, 1)
    }
}

ignore.forEach(newInput);

If you really want to impress the girls

var input = ["a", "b", "g", "h"],
    ignore = ["b", "h"];

var newInput = function(element, index) {
    ~(index = input.indexOf(element)) && input.splice(index, 1);
}

ignore.forEach(newInput);

I'll let you figure out that one liner :p

mutliple deletions

var newInput = function(element) {
    var index;
    while (~(index = input.indexOf(element)))
        input.splice(index, 1);
}

ignore.forEach(newInput);

edited Sep 15, 2015 at 6:51

answered Sep 15, 2015 at 5:00

Jaromanda X

57.1k6 gold badges82 silver badges96 bronze badges

1 Comment

user663031 Over a year ago

It's very confusing to provide the index parameter to the callback, when it is not needed and not used, and then redefine it to mean something entirely else. I supposed it's intended to avoid having to spend one line declaring a variable, but come on. Also, although the OP did not say so explicitly, it's reasonable to assume that he wants to remove all occurrences of an ignored element, not just the first one.

Collectives™ on Stack Overflow

using .foreach to remove one array from another

4 Answers 4

Comments

Comments

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related