Make _.each skip holes in sparse arrays #651

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

benjamn commented Jun 22, 2012

Most immediately, this change is necessary to match the behavior of the native forEach method. To see what I mean, try doing the following:

[0,,2].forEach(function(elem, i) {
  console.log(elem, "at index", i);
});

The changes to _.isEmpty seemed necessary to fulfill the promises of this comment: "An 'empty' object has no enumerable own-properties."

In general, wherever we bother to skip non-own object properties using _.has, we should be careful to skip array holes as well.

@benjamn benjamn Make _.each skip holes in sparse arrays.
Most immediately, this change is required to match the behavior of the
native forEach method.

The changes to _.isEmpty are required to fulfill the promises of this
comment: "An 'empty' object has no enumerable own-properties."

In general, wherever we bother to skip non-own object properties using
_.has, we should be careful to skip array holes as well.
6afc156
Owner

benjamn commented on 6afc156 Jun 22, 2012

There were two reasons to use i in obj instead of _.has(obj, i):

  1. The in keyword is much faster: http://jsperf.com/has-vs-in
  2. The results are the same as long as there are never any numerical keys in Array.prototype, which would be completely insane!

benjamn commented Jun 22, 2012

To be sure, I fully realize that sparse arrays are rare in day-to-day JS code, and iteration over sparse arrays is even rarer.

Nevertheless, here's a pretty reasonable snippet of code that behaves differently depending on whether Array.prototype.forEach is being used behind the scenes:

_.all([,3,,5], function(n) { return n > 2 });

All the elements that are actually defined as elements of the array (3 and 5) are > 2, but [,3,,5][i] > 2 is not true when i = 0 or i = 2, so the _.all expression above returns false if we're not skipping holes.

_.each and forEach will return different values.

Contributor

jdalton commented Jul 13, 2012

Just a heads-up. I got my first sparse array bug report for Underscore compatibility.

Owner

jashkenas commented Aug 31, 2012

@benjamn -- Yep, we're getting rid of "support" for sparse arrays in Underscore, and returning it to its regular iteration. See various previous tickets. Most of the best use cases for iterating sparse arrays are much better served by using an object (perhaps with numeric keys) instead.

jashkenas closed this Aug 31, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment