From 5ae3eccaf192202d7e3df779193c85eb19c8b9ec Mon Sep 17 00:00:00 2001 From: John-David Dalton Date: Wed, 7 Aug 2013 08:43:41 -0700 Subject: [PATCH] Move `_.remove` to the "Arrays" category and add unit tests. Former-commit-id: b0542496b45582b8ca59de19e950dc2368deee0a --- build.js | 10 +++---- lodash.js | 11 ++++---- test/test-build.js | 2 +- test/test.js | 67 ++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 76 insertions(+), 14 deletions(-) diff --git a/build.js b/build.js index 4c88a03f3..cc3da56d7 100644 --- a/build.js +++ b/build.js @@ -194,7 +194,7 @@ 'reduce': ['baseCreateCallback', 'baseEach', 'isArray'], 'reduceRight': ['baseCreateCallback', 'forEachRight'], 'reject': ['createCallback', 'filter'], - 'remove': ['baseEach', 'createCallback', 'isArray'], + 'remove': ['createCallback'], 'rest': ['createCallback', 'slice'], 'result': ['isFunction'], 'runInContext': ['defaults', 'pick'], @@ -338,6 +338,7 @@ 'lastIndexOf', 'pull', 'range', + 'remove', 'rest', 'sortedIndex', 'union', @@ -375,7 +376,6 @@ 'reduce', 'reduceRight', 'reject', - 'remove', 'shuffle', 'size', 'some', @@ -3001,11 +3001,11 @@ _.pull(funcDepMap[funcName], 'baseEach').push('forEach'); }); - _.each(['contains', 'every', 'filter', 'find', 'forEach', 'map', 'max', 'min', 'reduce', 'remove', 'some'], function(funcName) { + _.each(['contains', 'every', 'filter', 'find', 'forEach', 'map', 'max', 'min', 'reduce', 'some'], function(funcName) { _.pull(funcDepMap[funcName], 'baseEach').push('forOwn'); }); - _.each(['every', 'find', 'filter', 'forEach', 'forIn', 'forOwn', 'map', 'reduce', 'remove', 'shimKeys'], function(funcName) { + _.each(['every', 'find', 'filter', 'forEach', 'forIn', 'forOwn', 'map', 'reduce', 'shimKeys'], function(funcName) { if (!(isUnderscore && isLodash(funcName))) { _.pull(funcDepMap[funcName], 'isArray'); } @@ -3260,7 +3260,7 @@ ].join('\n')); // replace `isArray(collection)` checks in "Collections" functions with simpler type checks - _.each(['every', 'filter', 'find', 'max', 'min', 'reduce', 'remove', 'some'], function(funcName) { + _.each(['every', 'filter', 'find', 'max', 'min', 'reduce', 'some'], function(funcName) { source = source.replace(matchFunction(source, funcName), function(match) { if (funcName == 'reduce') { match = match.replace(/^( *)var noaccum\b/m, '$1if (!collection) return accumulator;\n$&'); diff --git a/lodash.js b/lodash.js index c376140fb..bb0e9c186 100644 --- a/lodash.js +++ b/lodash.js @@ -4647,11 +4647,10 @@ return result; } - /** - * Removes all elements from the given array that the callback returns truthy - * for and returns an array of removed elements. The callback is bound to - * `thisArg` and invoked with three arguments; (value, index, array). + * Removes all elements from an array that the callback returns truthy for + * and returns an array of removed elements. The callback is bound to `thisArg` + * and invoked with three arguments; (value, index, array). * * If a property name is provided for `callback` the created "_.pluck" style * callback will return the property value of the given element. @@ -4663,7 +4662,7 @@ * @static * @memberOf _ * @category Arrays - * @param {Array|Object|String} array The array to modify. + * @param {Array} array The array to modify. * @param {Function|Object|String} [callback=identity] The function called * per iteration. If a property name or object is provided it will be used * to create a "_.pluck" or "_.where" style callback, respectively. @@ -4682,7 +4681,7 @@ */ function remove(array, callback, thisArg) { var index = -1, - length = array.length, + length = array ? array.length : 0, result = []; callback = lodash.createCallback(callback, thisArg, 3); diff --git a/test/test-build.js b/test/test-build.js index a9ea5f6ff..3f9b0d5af 100644 --- a/test/test-build.js +++ b/test/test-build.js @@ -107,6 +107,7 @@ 'lastIndexOf', 'pull', 'range', + 'remove', 'rest', 'sortedIndex', 'union', @@ -144,7 +145,6 @@ 'reduce', 'reduceRight', 'reject', - 'remove', 'shuffle', 'size', 'some', diff --git a/test/test.js b/test/test.js index 83ac6b51d..a3fbf67f7 100644 --- a/test/test.js +++ b/test/test.js @@ -2734,6 +2734,20 @@ /*--------------------------------------------------------------------------*/ + QUnit.module('lodash.pull'); + + (function() { + test('should modify and return the array', function() { + var array = [1, 2, 3], + actual = _.pull(array, 1, 3); + + deepEqual(array, [2]); + ok(actual === array); + }) + }()); + + /*--------------------------------------------------------------------------*/ + QUnit.module('lodash.random'); (function() { @@ -2897,6 +2911,44 @@ /*--------------------------------------------------------------------------*/ + QUnit.module('lodash.remove'); + + (function() { + test('should modify the array and return removed elements', function() { + var array = [1, 2, 3]; + + var actual = _.remove(array, function(value) { + return value < 3; + }); + + deepEqual(array, [3]); + deepEqual(actual, [1, 2]); + }); + + test('should pass the correct `callback` arguments', function() { + var args, + array = [1, 2, 3]; + + _.remove(array, function() { + args || (args = slice.call(arguments)); + }); + + deepEqual(args, [1, 0, array]); + }); + + test('should support the `thisArg` argument', function() { + var array = [1, 2, 3]; + + var actual = _.remove(array, function(value, index) { + return this[index] < 3; + }, array); + + deepEqual(actual, [1, 2]); + }); + }()); + + /*--------------------------------------------------------------------------*/ + QUnit.module('lodash.result'); (function() { @@ -4011,6 +4063,9 @@ deepEqual(_.uniq(args), [1, null, [3], 5], message('uniq')); deepEqual(_.without(args, null), [1, [3], 5], message('without')); deepEqual(_.zip(args, args), [[1, 1], [null, null], [[3], [3]], [null, null], [5, 5]], message('zip')); + + deepEqual(_.pull(args, null), { '0': 1, '1': [3], '2': 5 }, message('pull')); + deepEqual(_.remove(args, function(value) { return typeof value == 'number'; }), [1, 5], message('remove')); }); test('should allow falsey primary arguments', function() { @@ -4125,7 +4180,6 @@ test('should handle `null` `thisArg` arguments', function() { var thisArg, - array = ['a'], callback = function() { thisArg = this; }, expected = (function() { return this; }).call(null); @@ -4134,10 +4188,17 @@ 'every', 'filter', 'find', + 'findIndex', + 'findKey', 'findLast', + 'findLastIndex', + 'findLastKey', 'forEach', + 'forEachRight', 'forIn', + 'forInRight', 'forOwn', + 'forOwnRight', 'groupBy', 'map', 'max', @@ -4147,6 +4208,7 @@ 'reduce', 'reduceRight', 'reject', + 'remove', 'some', 'sortBy', 'sortedIndex', @@ -4155,7 +4217,8 @@ ]; _.forEach(funcs, function(methodName) { - var func = _[methodName], + var array = ['a'], + func = _[methodName], message = '`_.' + methodName + '` handles `null` `thisArg` arguments'; thisArg = undefined;