Skip to content

Commit

Permalink
Make _.each skip holes in sparse arrays.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
benjamn committed Jun 22, 2012
1 parent 64b8f86 commit 6afc156
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions underscore.js
Expand Up @@ -77,7 +77,9 @@
obj.forEach(iterator, context);
} else if (obj.length === +obj.length) {
for (var i = 0, l = obj.length; i < l; i++) {
if (iterator.call(context, obj[i], i, obj) === breaker) return;
if (i in obj && // Skip holes in sparse arrays.
iterator.call(context, obj[i], i, obj) === breaker)
return;
}
} else {
for (var key in obj) {
Expand Down Expand Up @@ -805,11 +807,15 @@
// An "empty" object has no enumerable own-properties.
_.isEmpty = function(obj) {
if (obj == null) return true;
if (_.isArray(obj) || _.isString(obj)) return obj.length === 0;
for (var key in obj) if (_.has(obj, key)) return false;
return true;
if (_.isString(obj))
return obj.length === 0;
if (_.isArray(obj) && obj.length === 0)
return true;
return !any(obj, alwaysTrue);
};

function alwaysTrue() { return true }

// Is a given value a DOM element?
_.isElement = function(obj) {
return !!(obj && obj.nodeType == 1);
Expand Down

1 comment on commit 6afc156

@benjamn
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Please sign in to comment.