From f7a857744fcda637b6efe396c43b08d8a7aa9897 Mon Sep 17 00:00:00 2001 From: John-David Dalton Date: Mon, 5 Oct 2015 17:01:33 -0700 Subject: [PATCH] No longer coerce values to integers in `add`, `max`, `min`, or `sum` methods. --- lodash.js | 129 ++++++++++++++++++++++++++++----------------------- test/test.js | 39 +++++++++------- 2 files changed, 92 insertions(+), 76 deletions(-) diff --git a/lodash.js b/lodash.js index b79d7f115..8a2088b59 100644 --- a/lodash.js +++ b/lodash.js @@ -433,42 +433,6 @@ return true; } - /** - * A specialized version of `baseExtremum` for arrays which invokes `iteratee` - * with one argument: (value). - * - * @private - * @param {Array} array The array to iterate over. - * @param {Function} iteratee The function invoked per iteration. - * @param {Function} comparator The function used to compare values. - * @param {*} exValue The initial extremum value. - * @returns {*} Returns the extremum value. - */ - function arrayExtremum(array, iteratee, comparator, exValue) { - var index = -1, - length = array.length, - computed = exValue, - result = computed; - - while (++index < length) { - var value = array[index], - current = +iteratee(value); - - if (comparator(current, computed)) { - computed = current; - result = value; - } - } - index = result === exValue ? -1 : index; - while (++index < length) { - value = array[index]; - if (+iteratee(value) === exValue) { - return value; - } - } - return result; - } - /** * A specialized version of `_.filter` for arrays without support for * callback shorthands. @@ -635,6 +599,44 @@ } } + /** + * The base implementation of methods like `_.max` and `_.min` which accepts a + * `comparator` to determine the extremum value. + * + * @private + * @param {Array} array The array to iterate over. + * @param {Function} iteratee The function invoked per iteration. + * @param {Function} comparator The function used to compare values. + * @returns {*} Returns the extremum value. + */ + function baseExtremum(array, iteratee, comparator) { + var computed, + result, + index = -1, + length = array.length; + + while (++index < length) { + var value = array[index], + current = iteratee(value); + + if (current === current && current != null) { + computed = current; + result = value; + break; + } + } + while (++index < length) { + var value = array[index], + current = iteratee(value); + + if (current != null && comparator(current, computed)) { + computed = current; + result = value; + } + } + return result; + } + /** * The base implementation of methods like `_.find` and `_.findKey` without * support for callback shorthands, which iterates over `collection` using @@ -769,12 +771,15 @@ * @returns {number} Returns the sum. */ function baseSum(array, iteratee) { - var index = -1, - length = array.length, - result = 0; + var result, + index = -1, + length = array.length; while (++index < length) { - result += +iteratee(array[index]) || 0; + var current = iteratee(array[index]); + if (current === current && current != null) { + result = result === undefined ? current : (result + current); + } } return result; } @@ -12378,7 +12383,14 @@ * // => 10 */ function add(augend, addend) { - return (+augend || 0) + (+addend || 0); + var result; + if (augend === augend && augend != null) { + result = augend; + } + if (addend === addend && addend != null) { + result = result === undefined ? addend : (result + addend); + } + return result; } /** @@ -12426,8 +12438,8 @@ var floor = createRound('floor'); /** - * Gets the maximum value of `collection`. If `collection` is empty or falsey - * `-Infinity` is returned. + * Gets the maximum value of `array`. If `array` is empty or falsey + * `undefined` is returned. * * @static * @memberOf _ @@ -12440,12 +12452,12 @@ * // => 8 * * _.max([]); - * // => -Infinity + * // => undefined */ function max(array) { return (array && array.length) - ? arrayExtremum(array, identity, gt, -INFINITY) - : -INFINITY; + ? baseExtremum(array, identity, gt) + : undefined; } /** @@ -12475,13 +12487,13 @@ */ function maxBy(array, iteratee) { return (array && array.length) - ? arrayExtremum(array, getIteratee(iteratee), gt, -INFINITY) - : -INFINITY; + ? baseExtremum(array, getIteratee(iteratee), gt) + : undefined; } /** * Gets the minimum value of `array`. If `array` is empty or falsey - * `Infinity` is returned. + * `undefined` is returned. * * @static * @memberOf _ @@ -12494,12 +12506,12 @@ * // => 2 * * _.min([]); - * // => Infinity + * // => undefined */ function min(array) { return (array && array.length) - ? arrayExtremum(array, identity, lt, INFINITY) - : INFINITY; + ? baseExtremum(array, identity, lt) + : undefined; } /** @@ -12529,8 +12541,8 @@ */ function minBy(array, iteratee) { return (array && array.length) - ? arrayExtremum(array, getIteratee(iteratee), lt, INFINITY) - : INFINITY; + ? baseExtremum(array, getIteratee(iteratee), lt) + : undefined; } /** @@ -12567,12 +12579,11 @@ * * _.sum([4, 6]); * // => 10 - * - * _.sum({ 'a': 4, 'b': 6 }); - * // => 10 */ function sum(array) { - return array ? baseSum(array, identity) : 0; + return (array && array.length) + ? baseSum(array, identity) + : undefined; } /** @@ -12603,7 +12614,7 @@ function sumBy(array, iteratee) { return (array && array.length) ? baseSum(array, getIteratee(iteratee)) - : 0; + : undefined; } /*------------------------------------------------------------------------*/ diff --git a/test/test.js b/test/test.js index 73b258787..71ab95327 100644 --- a/test/test.js +++ b/test/test.js @@ -898,12 +898,11 @@ assert.strictEqual(_.add(6, 4), 10); }); - QUnit.test('should coerce params to numbers', function(assert) { - assert.expect(3); + QUnit.test('should not coerce params to numbers', function(assert) { + assert.expect(2); - assert.strictEqual(_.add('6', '4'), 10); - assert.strictEqual(_.add('6', 'y'), 6); - assert.strictEqual(_.add('x', 'y'), 0); + assert.strictEqual(_.add('6', '4'), '64'); + assert.strictEqual(_.add('x', 'y'), 'xy'); }); QUnit.test('should return an unwrapped value when implicitly chaining', function(assert) { @@ -11924,11 +11923,11 @@ assert.strictEqual(_.max([1, 2, 3]), 3); }); - QUnit.test('should return `-Infinity` for empty collections', function(assert) { + QUnit.test('should return `undefined` for empty collections', function(assert) { assert.expect(1); var values = falsey.concat([[]]), - expected = _.map(values, _.constant(-Infinity)); + expected = _.map(values, _.constant(undefined)); var actual = _.map(values, function(value, index) { try { @@ -11939,10 +11938,10 @@ assert.deepEqual(actual, expected); }); - QUnit.test('should return `-Infinity` for non-numeric collection values', function(assert) { + QUnit.test('should work with non-numeric collection values', function(assert) { assert.expect(1); - assert.strictEqual(_.max(['a', 'b']), -Infinity); + assert.strictEqual(_.max(['a', 'b']), 'b'); }); }()); @@ -12873,11 +12872,11 @@ assert.strictEqual(_.min([1, 2, 3]), 1); }); - QUnit.test('should return `Infinity` for empty collections', function(assert) { + QUnit.test('should return `undefined` for empty collections', function(assert) { assert.expect(1); var values = falsey.concat([[]]), - expected = _.map(values, _.constant(Infinity)); + expected = _.map(values, _.constant(undefined)); var actual = _.map(values, function(value, index) { try { @@ -12888,10 +12887,10 @@ assert.deepEqual(actual, expected); }); - QUnit.test('should return `Infinity` for non-numeric collection values', function(assert) { + QUnit.test('should work with non-numeric collection values', function(assert) { assert.expect(1); - assert.strictEqual(_.min(['a', 'b']), Infinity); + assert.strictEqual(_.min(['a', 'b']), 'a'); }); }()); @@ -17185,10 +17184,10 @@ assert.strictEqual(_.sum(array), 12); }); - QUnit.test('should return `0` when passing empty `array` values', function(assert) { + QUnit.test('should return `undefined` when passing empty `array` values', function(assert) { assert.expect(1); - var expected = _.map(empties, _.constant(0)); + var expected = _.map(empties, _.constant(undefined)); var actual = _.map(empties, function(value) { return _.sum(value); @@ -17197,10 +17196,16 @@ assert.deepEqual(actual, expected); }); - QUnit.test('should coerce values to numbers and `NaN` to `0`', function(assert) { + QUnit.test('should not coerce values to numbers', function(assert) { assert.expect(1); - assert.strictEqual(_.sum(['1', NaN, '2']), 3); + assert.strictEqual(_.sum(['1', '2']), '12'); + }); + + QUnit.test('should skip `null`, `undefined`, and `NaN` values', function(assert) { + assert.expect(1); + + assert.strictEqual(_.sum(['1', null, undefined, NaN, '2']), '12'); }); }());