1

Problem

I am trying to remove all of the odd numbers from an array. For example, if I pass an array like so...

var arr = [1,2,3,6,22,98,45,23,22,12]

...the function removes all of the odd numbers except for 23. Why doesn't it remove 23 as well? If different numbers are used or if the order of the numbers is changed, it is always the last odd number that is not removed. I don't understand why though, since the for loop should continue until it gets to the end of the array (i < passedArray.length).

I am sure it is something simple, but I can't figure it out! Any help would be much appreciated ;)

Code

// PROBLEM: Loop through arr removing all values that aren't even.

// The original array
var arr = [1, 2, 3, 6, 22, 98, 45, 23, 22, 12];

// Function to remove all odd numbers from the array that is passed to it.
// Returns the new array.
var getEvenNumbers = function(passedArray) {
  for (var i = 0; i < passedArray.length; i++) {

    // If the remainder of the current number in the array is equal to one, the number is odd so remove it from the array.
    if ((passedArray[i] % 2) === 1) {
      arr.splice(i, 1);
    }
  }

  // Return the array with only even numbers left.
  return passedArray;
};

// Call the function and store the results.
var evenNumbers = getEvenNumbers(arr);

// Alert the new array that only has even numbers.
alert(evenNumbers);

Improve this question
0

2 Answers 2

Reset to default
3

The bug is that once you've spliced a number out of the array, you still increment i. This makes the code skip the number that follows the one that you've just deleted.

Since the number that precedes the 23 is odd (45), you never look at the 23.

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

4 Comments

OP should start from the end and decrement the loop to prevent splicing from altering future comparisons.
@JoelEtherton: Or only increment i if the if condition doesn't hold.
Possible, but which would you do if you were writing it? Personally I'd decrement. It's simpler since you have to traverse the length of the array anyway. Extra if conditions for increment just makes it muddier IMO.
@JoelEtherton: Actually, I'd probably do neither. Modifying a data structure while iterating over it is often a recipe for bugs.
0

You are chaging your array with splice, so your length changes as well. You could change your function to this:

var getEvenNumbers = function(passedArray) {
  var evenArr=[];
  for (var i = 0; i < passedArray.length; i++) {

    if ((passedArray[i] % 2) != 1) { // if its even, add it
      evenArr.push(passedArray[i]);
    }
  }

  // Return the array with only even numbers left.
  return evenArr;
};
Improve this answer

2 Comments

That is a great idea for how to re-build an array with only even numbers; however, the problem I am working on has asked that I remove odd numbers from the array (it is a coding exercise). According to MDN, the for loop condition is "evaluated before each loop iteration" which would mean that even though the length of the array is changing, so is the value returned by passedArray.length; it seems to me that the changing size of the array wouldn't be an issue because the amount of items to loop through would update each time through the loop...am I missing something?
@trevordmiller well, as posted in other comments, you could just change your iteration order: for (var i = passedArray.length-1; i >=0; i--) {

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.