Why does the last element of my array return undefined, rather than splicing it off?

Question

I'm working on a small exercise: The problem is that I want this:

=> ['kept','kept']

But instead, I keep getting this:

function keep(array, keeper) {

//This will return an array of undefined's and 'kept's
// =>[ 'kept', undefined, 'kept', undefined, undefined ]

  matchingNumbers = array.map(function matching(element){
     if (element === keeper) {
      return element;
     }
  });


//Eliminate all undefined's from the matchingNumbers array

  matchingLength = matchingNumbers.length;
  for (var i = 1; i < matchingLength; i++) {
    if(matchingNumbers[i] === undefined) {
      (matchingNumbers.splice(i, 1));
    }
  }
  return matchingNumbers;
}

keep(['kept', 'thirty', 'kept', 2, 1], 'kept')

I'm trying to splice off all of the undefined's in matchingNumbers with the for-loop, so why is there a last undefined remaining?

Your `i` gets incremented after each iteration, but your array is losing elements at the same time, so an incremented `i` won’t always mean “the next element”. — Sebastian Simon, Feb 24 '17 at 23:35
firstly, arrays start at index 0 ... secondly, [Array#filter](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Array/filter) — Jaromanda X, Feb 24 '17 at 23:37
if you look at the Polyfill in the link above, you'll see how you should've written the code :p — Jaromanda X, Feb 24 '17 at 23:39

Barmar · Accepted Answer · 2017-02-24T23:39:56.343

4

When a function doesn't execute a return statement, it returns undefined by default. array.map() puts the return values of the function into the resulting array, and this includes those undefined values.

You should use array.filter instead of array.map:

matchingNumbers = array.filter(function matching(element){
     return element === keeper;
});

edited Feb 24 '17 at 23:39

answered Feb 24 '17 at 23:37

Barmar

741,623
53
500
612

1

I'd like to add that as a result of using the `array.filter` function you don't even have to do your second loop to remove `undefined`s from the list. Just use array.filter and that should be enough. If used correctly it won't add any `undefined` to the list. – Jerry Saravia Feb 24 '17 at 23:47
@JerrySaravia Yes, that was the intent, this replaces all of his code. – Barmar Feb 24 '17 at 23:52
Yes, I understand that, just wanted to make it explicit for the Wali.It wasn't too clear in your answer since it doesn't include the full function definition and doesn't state it outright. – Jerry Saravia Feb 25 '17 at 00:04

score 2 · Answer 2 · answered Feb 24 '17 at 23:35

2

My guess is because you're starting i at 1 instead of 0. Also, as @Wali mentioned, you're changing the array length while iterating over it. To solve that, go through the array in reverse order:

for (var i = array.length; i > 0; i--) {
  ...
}

As a side note, you can achieve what you want using Array.filter rather than dealing with mapping and splicing.

answered Feb 24 '17 at 23:35

Matthew Herbst

29,477
23
85
128

it's not the change of array length, it's the fact that when you for example, process element 3, and decide to splice at 3, the next element tested is 4 ... but that used to be 5 before the splice ... so the original element 4 is now 3, but that's not tested because i == 4 ... one could `i--` in the if after the splice, or as you suggest, loop backwards. The only time it's an issue is when there are two `undefined` next to each other – Jaromanda X Feb 24 '17 at 23:43

Why does the last element of my array return undefined, rather than splicing it off?

2 Answers2