1

I have two arrays like this:

var arr = ["1", "3" , "4"];
var arr2 = ["1", "2", "3", "4", "5", "6", "7", "8", "9"];

I want to remove numbers from second array based on numbers that are available in first array.

I tried this but I am getting wrong values such as 2,4,6,8:

theButton.onclick = function removePassedInNumbers(arr){
    for(var i = 0; i < arr2.length; i++){
    if(arr2.indexOf(arr[i])){
        arr2.splice(i, 1);
    }
  }
    document.getElementById('myNumber').innerHTML = arr2;
}

<input type="button" id="theButton" onclick="removePassedInNumbers(arr)" value="Click here"/>
<p id="myNumber">hey</p>

Here is the fiddle: https://jsfiddle.net/4hLcanc6/

Where did I go wrong?

kittu
  • 5,835
  • 18
  • 72
  • 142

3 Answers3

3

You can just use filter() method :

var arr = ["1", "3" , "4"];
var arr2 = ["1", "2", "3", "4", "5", "6", "7", "8", "9"];
var res = arr2.filter(item => arr.indexOf(item)===-1);
console.log(res); // [ '2', '5', '6', '7', '8', '9' ]

Or with ES5 :

var res = arr2.filter(function(x) {return arr.indexOf(x)===-1});
kevin ternet
  • 3,779
  • 2
  • 14
  • 23
2

You can update your function with something like this

var arr = ["1", "3" , "4"];
var arr2 = ["1", "2", "3", "4", "5", "6", "7", "8", "9"];

arr.forEach(function(n){
  var index = arr2.indexOf(n); 
  if(index !== -1) arr2.splice(index, 1);
})

console.log(arr2);

Note: splice will change your array arr2, if you want a new array without changing the original arr2 you should use filter.

taguenizy
  • 1,922
  • 1
  • 6
  • 23
  • 1 - `arr2.length` should been `arr.length`. 2 - `arr2.indexOf(arr[i])` should be `arr2.indexOf(arr[i]) !== -1`. 3 - you need to store the `index` on `arr2` array so you can `splice` it. @Satyadev – taguenizy Oct 28 '16 at 15:41
  • There's not all the changes there. https://jsfiddle.net/a3w0n4yf/ . Also you need to remove the `arr` from the function or properly pass it on the HTML – taguenizy Oct 28 '16 at 15:54
  • Thank you. I like the native js script provided in the fiddle in your comment. – kittu Oct 28 '16 at 16:02
1

This for loop is incorrect. Even if, as some users has already answered, there are better methods to filter an array, I wanna try to explain why you're doing this loop wrong.

Let's start with the first line

for(var i = 0; i < arr2.length; i++)

here you're storing in variable i your current position in arr2. Ok.

if(arr2.indexOf(arr[i]))

This is a no-no. arr[i] may not exist!! Infact, arr[3] is undefined, and you're looking for it the fourth time you enter the loop. So, let's fix this one:

if(arr.indexOf(arr2[i]))

Now is ok. We're asking to look inside arr to find the value of arr2[i], that will be "1" the first time we jump in the loop, "2" the second time ... and "9" the last time.

Now it will work. To recap:

for(var i = 0; i < arr2.length; i++){    
  if(arr.indexOf(arr2[i])){
    arr2.splice(i, 1);
  }
}

Hope this will help you understand what's going on in your code.