From 0f6de542bf328654c2480e457ece0473fe1cda2c Mon Sep 17 00:00:00 2001 From: John-David Dalton Date: Sun, 4 Aug 2013 12:46:27 -0700 Subject: [PATCH] Fix `_.forEachRight`, `_.forInRight`, and `_.findLastIndex` and add related unit tests. Former-commit-id: 5131ae4559cd71d8016745f85158bb6f96426d01 --- lodash.js | 19 ++++++-------- test/test.js | 74 ++++++++++++++++++++++++++++++++-------------------- 2 files changed, 53 insertions(+), 40 deletions(-) diff --git a/lodash.js b/lodash.js index 177ff22d3..39305d9f8 100644 --- a/lodash.js +++ b/lodash.js @@ -2102,17 +2102,16 @@ * // => logs 'name' and 'bark' assuming `_.forIn ` logs 'bark' and 'name' */ function forInRight(object, callback, thisArg) { - var index = -1, - pairs = []; + var pairs = []; forIn(object, function(value, key) { - pairs.push(value, key); + pairs.push(key, value); }); var length = pairs.length; callback = baseCreateCallback(callback, thisArg, 3); - while (++index < length) { - if (callback(pairs[index], pairs[++index], object) === false) { + while (length--) { + if (callback(pairs[length--], pairs[length], object) === false) { break; } } @@ -3317,7 +3316,7 @@ callback = baseCreateCallback(callback, thisArg, 3); forEach(collection, function(value, index, collection) { index = props ? props[--length] : --length; - callback(iterable[index], index, collection); + return callback(iterable[index], index, collection); }); return collection; } @@ -4196,13 +4195,11 @@ * // => 2 */ function findLastIndex(array, callback, thisArg) { - var index = -1, - length = array ? array.length : 0; - + var length = array ? array.length : 0; callback = lodash.createCallback(callback, thisArg); while (length--) { - if (callback(array[index], index, array)) { - return index; + if (callback(array[length], length, array)) { + return length; } } return -1; diff --git a/test/test.js b/test/test.js index 51ed4577e..af997401c 100644 --- a/test/test.js +++ b/test/test.js @@ -957,8 +957,11 @@ _.forEach({ 'find': [objects[1], undefined, objects[2], objects[1]], + 'findLast': [objects[1], undefined, objects[2], objects[2]], 'findIndex': [1, -1, 2, 1], - 'findKey': ['1', undefined, '2', '1'] + 'findLastIndex': [1, -1, 2, 2], + 'findKey': ['1', undefined, '2', '1'], + 'findLastKey': ['1', undefined, '2', '2'] }, function(expected, methodName) { QUnit.module('lodash.' + methodName); @@ -1138,30 +1141,32 @@ /*--------------------------------------------------------------------------*/ - QUnit.module('lodash.forEach'); + QUnit.module('forEach methods'); - (function() { - test('should return the collection', function() { + _.forEach(['forEach', 'forEachRight'], function(methodName) { + var func = _[methodName]; + + test('`_.' + methodName + '` should return the collection', function() { var collection = [1, 2, 3]; - equal(_.forEach(collection, Boolean), collection); + equal(func(collection, Boolean), collection); }); - test('should return the existing wrapper when chaining', function() { + test('`_.' + methodName + '` should return the existing wrapper when chaining', function() { var wrapper = _([1, 2, 3]); - equal(wrapper.forEach(Boolean), wrapper); + equal(wrapper[methodName](Boolean), wrapper); }); - test('should support the `thisArg` argument', function() { + test('`_.' + methodName + '` should support the `thisArg` argument', function() { var actual; function callback(value, index) { actual = this[index]; } - _.forEach([1], callback, [2]); + func([1], callback, [2]); equal(actual, 2); - _.forEach({ 'a': 1 }, callback, { 'a': 2 }); + func({ 'a': 1 }, callback, { 'a': 2 }); equal(actual, 2); }); @@ -1170,36 +1175,43 @@ 'object': Object('abc') }, function(collection, key) { - test('should work with a string ' + key + ' for `collection` (test in IE < 9)', function() { + test('`_.' + methodName + '` should work with a string ' + key + ' for `collection` (test in IE < 9)', function() { var args, values = []; - _.forEach(collection, function(value) { + func(collection, function(value) { args || (args = slice.call(arguments)); values.push(value); }); - deepEqual(args, ['a', 0, collection]); - deepEqual(values, ['a', 'b', 'c']); + if (methodName == 'forEach') { + deepEqual(args, ['a', 0, collection]); + deepEqual(values, ['a', 'b', 'c']); + } else { + deepEqual(args, ['c', 2, collection]); + deepEqual(values, ['c', 'b', 'a']); + } }); }); - }()); + }); /*--------------------------------------------------------------------------*/ - QUnit.module('lodash.forIn'); + QUnit.module('forIn methods'); - (function() { - test('iterates over inherited properties', function() { + _.forEach(['forIn', 'forInRight'], function(methodName) { + var func = _[methodName]; + + test('`_.' + methodName + '` iterates over inherited properties', function() { function Foo() { this.a = 1; } Foo.prototype.b = 2; var keys = []; - _.forIn(new Foo, function(value, key) { keys.push(key); }); + func(new Foo, function(value, key) { keys.push(key); }); deepEqual(keys.sort(), ['a', 'b']); }); - test('fixes the JScript [[DontEnum]] bug with inherited properties (test in IE < 9)', function() { + test('`_.' + methodName + '` fixes the JScript [[DontEnum]] bug with inherited properties (test in IE < 9)', function() { function Foo() {} Foo.prototype = shadowedObject; @@ -1208,30 +1220,32 @@ Bar.prototype.constructor = Bar; var keys = []; - _.forIn(shadowedObject, function(value, key) { keys.push(key); }); + func(shadowedObject, function(value, key) { keys.push(key); }); deepEqual(keys.sort(), shadowedProps); }); - }()); + }); /*--------------------------------------------------------------------------*/ - QUnit.module('lodash.forOwn'); + QUnit.module('forOwn methods'); + + _.forEach(['forOwn', 'forOwnRight'], function(methodName) { + var func = _[methodName]; - (function() { test('iterates over the `length` property', function() { var keys = [], object = { '0': 'zero', '1': 'one', 'length': 2 }; - _.forOwn(object, function(value, key) { keys.push(key); }); + func(object, function(value, key) { keys.push(key); }); deepEqual(keys.sort(), ['0', '1', 'length']); }); - }()); + }); /*--------------------------------------------------------------------------*/ QUnit.module('collection iteration bugs'); - _.forEach(['forEach', 'forIn', 'forOwn'], function(methodName) { + _.forEach(['forEach', 'forEachRight', 'forIn', 'forInRight', 'forOwn', 'forOwnRight'], function(methodName) { var func = _[methodName]; test('`_.' + methodName + '` fixes the JScript [[DontEnum]] bug (test in IE < 9)', function() { @@ -1391,7 +1405,7 @@ QUnit.module('exit early'); - _.forEach(['forEach', 'forIn', 'forOwn'], function(methodName) { + _.forEach(['forEach', 'forEachRight', 'forIn', 'forInRight', 'forOwn', 'forOwnRight'], function(methodName) { var func = _[methodName]; test('`_.' + methodName + '` can exit early when iterating arrays', function() { @@ -1399,7 +1413,7 @@ values = []; func(array, function(value) { values.push(value); return false; }); - deepEqual(values, [1]); + deepEqual(values, [/Right/.test(methodName) ? 3 : 1]); }); test('`_.' + methodName + '` can exit early when iterating objects', function() { @@ -3980,6 +3994,7 @@ deepEqual(_.compact(args), [1, [3], 5], message('compact')); deepEqual(_.findIndex(args, _.identity), 0, message('findIndex')); + deepEqual(_.findLastIndex(args, _.identity), 4, message('findLastIndex')); deepEqual(_.first(args), 1, message('first')); deepEqual(_.flatten(args), [1, null, 3, null, 5], message('flatten')); deepEqual(_.indexOf(args, 5), 4, message('indexOf')); @@ -4115,6 +4130,7 @@ 'every', 'filter', 'find', + 'findLast', 'forEach', 'forIn', 'forOwn',