Skip to content

Commit

Permalink
Merge pull request #1525 from davidchambers/consistent-each
Browse files Browse the repository at this point in the history
remove references to native array methods
  • Loading branch information
jashkenas committed Mar 28, 2014
2 parents 35a53d4 + 527cbe9 commit 7a252d5
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 60 deletions.
30 changes: 11 additions & 19 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -359,10 +359,7 @@
Underscore provides 80-odd functions that support both the usual
functional suspects: <b>map</b>, <b>filter</b>, <b>invoke</b> &mdash;
as well as more specialized helpers: function binding, javascript
templating, deep equality testing, and so on. It delegates to built-in
functions, if present, so modern browsers will use the
native implementations of <b>forEach</b>, <b>map</b>, <b>reduce</b>,
<b>filter</b>, <b>every</b>, <b>some</b> and <b>indexOf</b>.
templating, deep equality testing, and so on.
</p>

<p>
Expand Down Expand Up @@ -443,8 +440,7 @@ <h2 id="collections">Collection Functions (Arrays or Objects)</h2>
function. The <b>iterator</b> is bound to the <b>context</b> object, if one is
passed. Each invocation of <b>iterator</b> is called with three arguments:
<tt>(element, index, list)</tt>. If <b>list</b> is a JavaScript object, <b>iterator</b>'s
arguments will be <tt>(value, key, list)</tt>. Delegates to the native
<b>forEach</b> function if it exists. Returns the <b>list</b> for chaining.
arguments will be <tt>(value, key, list)</tt>. Returns the <b>list</b> for chaining.
</p>
<pre>
_.each([1, 2, 3], alert);
Expand All @@ -468,9 +464,9 @@ <h2 id="collections">Collection Functions (Arrays or Objects)</h2>
<span class="alias">Alias: <b>collect</b></span>
<br />
Produces a new array of values by mapping each value in <b>list</b>
through a transformation function (<b>iterator</b>). If the native <b>map</b> method
exists, it will be used instead. If <b>list</b> is a JavaScript object,
<b>iterator</b>'s arguments will be <tt>(value, key, list)</tt>.
through a transformation function (<b>iterator</b>). If <b>list</b>
is a JavaScript object, <b>iterator</b>'s arguments will be
<tt>(value, key, list)</tt>.
</p>
<pre>
_.map([1, 2, 3], function(num){ return num * 3; });
Expand Down Expand Up @@ -529,8 +525,7 @@ <h2 id="collections">Collection Functions (Arrays or Objects)</h2>
<span class="alias">Alias: <b>select</b></span>
<br />
Looks through each value in the <b>list</b>, returning an array of all
the values that pass a truth test (<b>predicate</b>). Delegates to the
native <b>filter</b> method, if it exists.
the values that pass a truth test (<b>predicate</b>).
</p>
<pre>
var evens = _.filter([1, 2, 3, 4, 5, 6], function(num){ return num % 2 == 0; });
Expand Down Expand Up @@ -582,8 +577,8 @@ <h2 id="collections">Collection Functions (Arrays or Objects)</h2>
<b class="header">every</b><code>_.every(list, [predicate], [context])</code>
<span class="alias">Alias: <b>all</b></span>
<br />
Returns <i>true</i> if all of the values in the <b>list</b> pass the <b>predicate</b>
truth test. Delegates to the native method <b>every</b>, if present.
Returns <i>true</i> if all of the values in the <b>list</b> pass the
<b>predicate</b> truth test.
</p>
<pre>
_.every([true, 1, null, 'yes'], _.identity);
Expand All @@ -596,8 +591,7 @@ <h2 id="collections">Collection Functions (Arrays or Objects)</h2>
<br />
Returns <i>true</i> if any of the values in the <b>list</b> pass the
<b>predicate</b> truth test. Short-circuits and stops traversing the list
if a true element is found. Delegates to the native method <b>some</b>,
if present.
if a true element is found.
</p>
<pre>
_.some([null, 0, 'yes', false]);
Expand Down Expand Up @@ -964,8 +958,7 @@ <h2 id="arrays">Array Functions</h2>
<b class="header">indexOf</b><code>_.indexOf(array, value, [isSorted])</code>
<br />
Returns the index at which <b>value</b> can be found in the <b>array</b>,
or <i>-1</i> if value is not present in the <b>array</b>. Uses the native
<b>indexOf</b> function unless it's missing. If you're working with a
or <i>-1</i> if value is not present in the <b>array</b>. If you're working with a
large array, and you know that the array is already sorted, pass <tt>true</tt>
for <b>isSorted</b> to use a faster binary search ... or, pass a number as
the third argument in order to look for the first matching value in the
Expand All @@ -980,8 +973,7 @@ <h2 id="arrays">Array Functions</h2>
<b class="header">lastIndexOf</b><code>_.lastIndexOf(array, value, [fromIndex])</code>
<br />
Returns the index of the last occurrence of <b>value</b> in the <b>array</b>,
or <i>-1</i> if value is not present. Uses the native <b>lastIndexOf</b>
function if possible. Pass <b>fromIndex</b> to start your search at a
or <i>-1</i> if value is not present. Pass <b>fromIndex</b> to start your search at a
given index.
</p>
<pre>
Expand Down
3 changes: 3 additions & 0 deletions test/arrays.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,9 @@
numbers = [1, 2, 3, 1, 2, 3, 1, 2, 3];
index = _.indexOf(numbers, 2, 5);
equal(index, 7, 'supports the fromIndex argument');

index = _.indexOf([,,,], undefined);
equal(index, 0, 'treats sparse arrays as if they were dense');
});

test('lastIndexOf', function() {
Expand Down
6 changes: 6 additions & 0 deletions test/collections.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@
var a = [1, 2, 3];
strictEqual(_.each(a, function(){}), a);
strictEqual(_.each(null, function(){}), null);

var b = [1, 2, 3];
b.length = 100;
answers = 0;
_.each(b, function(){ ++answers; });
equal(answers, 100, 'enumerates [0, length)');
});

test('map', function() {
Expand Down
46 changes: 5 additions & 41 deletions underscore.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,6 @@
// All **ECMAScript 5** native function implementations that we hope to use
// are declared here.
var
nativeForEach = ArrayProto.forEach,
nativeMap = ArrayProto.map,
nativeReduce = ArrayProto.reduce,
nativeReduceRight = ArrayProto.reduceRight,
nativeFilter = ArrayProto.filter,
nativeEvery = ArrayProto.every,
nativeSome = ArrayProto.some,
nativeIndexOf = ArrayProto.indexOf,
nativeLastIndexOf = ArrayProto.lastIndexOf,
nativeIsArray = Array.isArray,
nativeKeys = Object.keys,
nativeBind = FuncProto.bind;
Expand Down Expand Up @@ -71,13 +62,11 @@
// --------------------

// The cornerstone, an `each` implementation, aka `forEach`.
// Handles objects with the built-in `forEach`, arrays, and raw objects.
// Delegates to **ECMAScript 5**'s native `forEach` if available.
// Handles raw objects in addition to array-likes. Treats all
// sparse array-likes as if they were dense.
_.each = _.forEach = function(obj, iterator, context) {
if (obj == null) return obj;
if (nativeForEach && obj.forEach === nativeForEach) {
obj.forEach(iterator, context);
} else if (obj.length === +obj.length) {
if (obj.length === +obj.length) {
for (var i = 0, length = obj.length; i < length; i++) {
if (iterator.call(context, obj[i], i, obj) === breaker) return;
}
Expand All @@ -91,11 +80,9 @@
};

// Return the results of applying the iterator to each element.
// Delegates to **ECMAScript 5**'s native `map` if available.
_.map = _.collect = function(obj, iterator, context) {
var results = [];
if (obj == null) return results;
if (nativeMap && obj.map === nativeMap) return obj.map(iterator, context);
_.each(obj, function(value, index, list) {
results.push(iterator.call(context, value, index, list));
});
Expand All @@ -105,14 +92,10 @@
var reduceError = 'Reduce of empty array with no initial value';

// **Reduce** builds up a single result from a list of values, aka `inject`,
// or `foldl`. Delegates to **ECMAScript 5**'s native `reduce` if available.
// or `foldl`.
_.reduce = _.foldl = _.inject = function(obj, iterator, memo, context) {
var initial = arguments.length > 2;
if (obj == null) obj = [];
if (nativeReduce && obj.reduce === nativeReduce) {
if (context) iterator = _.bind(iterator, context);
return initial ? obj.reduce(iterator, memo) : obj.reduce(iterator);
}
_.each(obj, function(value, index, list) {
if (!initial) {
memo = value;
Expand All @@ -126,14 +109,9 @@
};

// The right-associative version of reduce, also known as `foldr`.
// Delegates to **ECMAScript 5**'s native `reduceRight` if available.
_.reduceRight = _.foldr = function(obj, iterator, memo, context) {
var initial = arguments.length > 2;
if (obj == null) obj = [];
if (nativeReduceRight && obj.reduceRight === nativeReduceRight) {
if (context) iterator = _.bind(iterator, context);
return initial ? obj.reduceRight(iterator, memo) : obj.reduceRight(iterator);
}
var length = obj.length;
if (length !== +length) {
var keys = _.keys(obj);
Expand Down Expand Up @@ -165,12 +143,10 @@
};

// Return all the elements that pass a truth test.
// Delegates to **ECMAScript 5**'s native `filter` if available.
// Aliased as `select`.
_.filter = _.select = function(obj, predicate, context) {
var results = [];
if (obj == null) return results;
if (nativeFilter && obj.filter === nativeFilter) return obj.filter(predicate, context);
_.each(obj, function(value, index, list) {

This comment has been minimized.

Copy link
@jdalton

jdalton Mar 28, 2014

Contributor

internal _.each use could be swapped with a baseEach like helper that avoids things like context binding (e.g. iterator.call(...) since internal use isn't using the context.

This comment has been minimized.

Copy link
@jdalton

jdalton Mar 28, 2014

Contributor

Also the big 5 (_.each, _.filter, _.map, _.some, _.every) could be optimized to use a for-loop for array-like values and fallback to a helper for objects.

if (predicate.call(context, value, index, list)) results.push(value);
});
Expand All @@ -183,27 +159,23 @@
};

// Determine whether all of the elements match a truth test.
// Delegates to **ECMAScript 5**'s native `every` if available.
// Aliased as `all`.
_.every = _.all = function(obj, predicate, context) {
predicate || (predicate = _.identity);
var result = true;
if (obj == null) return result;
if (nativeEvery && obj.every === nativeEvery) return obj.every(predicate, context);
_.each(obj, function(value, index, list) {
if (!(result = result && predicate.call(context, value, index, list))) return breaker;
});
return !!result;
};

// Determine if at least one element in the object matches a truth test.
// Delegates to **ECMAScript 5**'s native `some` if available.
// Aliased as `any`.
_.some = _.any = function(obj, predicate, context) {
predicate || (predicate = _.identity);
var result = false;
if (obj == null) return result;
if (nativeSome && obj.some === nativeSome) return obj.some(predicate, context);
_.each(obj, function(value, index, list) {
if (result || (result = predicate.call(context, value, index, list))) return breaker;
});
Expand All @@ -214,7 +186,6 @@
// Aliased as `include`.
_.contains = _.include = function(obj, target) {
if (obj == null) return false;
if (nativeIndexOf && obj.indexOf === nativeIndexOf) return obj.indexOf(target) != -1;

This comment has been minimized.

Copy link
@jdalton

jdalton Mar 28, 2014

Contributor

Now that the native branch is removed in _.contains it goes to the slower _.some branch. In this case using _.indexOf is still preferable to the _.some route so some check is needed.

return _.some(obj, function(value) {
return value === target;
});
Expand Down Expand Up @@ -568,7 +539,6 @@
// If the browser doesn't supply us with indexOf (I'm looking at you, **MSIE**),
// we need this function. Return the position of the first occurrence of an
// item in an array, or -1 if the item is not included in the array.
// Delegates to **ECMAScript 5**'s native `indexOf` if available.
// If the array is large and already in sort order, pass `true`
// for **isSorted** to use binary search.
_.indexOf = function(array, item, isSorted) {
Expand All @@ -582,19 +552,13 @@
return array[i] === item ? i : -1;
}
}
if (nativeIndexOf && array.indexOf === nativeIndexOf) return array.indexOf(item, isSorted);
for (; i < length; i++) if (array[i] === item) return i;
return -1;
};

// Delegates to **ECMAScript 5**'s native `lastIndexOf` if available.
_.lastIndexOf = function(array, item, from) {
if (array == null) return -1;
var hasIndex = from != null;
if (nativeLastIndexOf && array.lastIndexOf === nativeLastIndexOf) {
return hasIndex ? array.lastIndexOf(item, from) : array.lastIndexOf(item);
}
var i = (hasIndex ? from : array.length);
var i = from == null ? array.length : from;
while (i--) if (array[i] === item) return i;
return -1;
};
Expand Down

0 comments on commit 7a252d5

Please sign in to comment.