Skip to content

Commit

Permalink
Make _.sample more efficient by exiting early, make _.shuffle call th…
Browse files Browse the repository at this point in the history
…rough to _.sample

* Improves efficiency of _.sample for when n is significantly smaller than obj.length
* Improves tests to make sure _.sample and _.shuffle actually change the order
* Replace shuffle with call to _.sample to avoid duplicated logic
  • Loading branch information
marekventur committed Apr 28, 2015
1 parent 4f2474c commit 42b763e
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 17 deletions.
1 change: 1 addition & 0 deletions test/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"strictEqual": false,
"notStrictEqual": false,
"notEqual": false,
"notDeepEqual": false,
"throws": false,
"asyncTest": false,
"start": false,
Expand Down
13 changes: 9 additions & 4 deletions test/collections.js
Original file line number Diff line number Diff line change
Expand Up @@ -737,19 +737,21 @@
});

test('shuffle', function() {
var numbers = _.range(10);
deepEqual(_.shuffle([1]), [1], 'behaves correctly on size 1 arrays');
var numbers = _.range(20);
var shuffled = _.shuffle(numbers);
notDeepEqual(numbers, shuffled, 'does change the order'); // Chance of false negative: 1 in ~2.4*10^18
notStrictEqual(numbers, shuffled, 'original object is unmodified');
ok(_.every(_.range(10), function() { //appears consistent?
return _.every(numbers, _.partial(_.contains, numbers));
}), 'contains the same members before and after shuffle');
deepEqual(numbers, _.sortBy(shuffled), 'contains the same members before and after shuffle');

shuffled = _.shuffle({a: 1, b: 2, c: 3, d: 4});
equal(shuffled.length, 4);
deepEqual(shuffled.sort(), [1, 2, 3, 4], 'works on objects');
});

test('sample', function() {
strictEqual(_.sample([1]), 1, 'behaves correctly when no second parameter is given');
deepEqual(_.sample([1, 2, 3], -2), [], 'behaves correctly on negative n');
var numbers = _.range(10);
var allSampled = _.sample(numbers, 10).sort();
deepEqual(allSampled, numbers, 'contains the same members before and after sample');
Expand All @@ -761,6 +763,9 @@
notStrictEqual(_.sample([1, 2, 3], 0), [], 'sampling an array with 0 picks returns an empty array');
deepEqual(_.sample([1, 2], -1), [], 'sampling a negative number of picks returns an empty array');
ok(_.contains([1, 2, 3], _.sample({a: 1, b: 2, c: 3})), 'sample one value from an object');
var partialSample = _.sample(_.range(1000), 10);
var partialSampleSorted = partialSample.sort();
notDeepEqual(partialSampleSorted, _.range(10), 'samples from the whole array, not just the beginning');
});

test('toArray', function() {
Expand Down
29 changes: 16 additions & 13 deletions underscore.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,29 +349,32 @@
return result;
};

// Shuffle a collection, using the modern version of the
// [Fisher-Yates shuffle](http://en.wikipedia.org/wiki/Fisher–Yates_shuffle).
// Shuffle a collection.
_.shuffle = function(obj) {
var set = isArrayLike(obj) ? obj : _.values(obj);
var length = set.length;
var shuffled = Array(length);
for (var index = 0, rand; index < length; index++) {
rand = _.random(0, index);
if (rand !== index) shuffled[index] = shuffled[rand];
shuffled[rand] = set[index];
}
return shuffled;
return _.sample(obj, Infinity);
};

// Sample **n** random values from a collection.
// Sample **n** random values from a collection using the modern version of the
// [Fisher-Yates shuffle](http://en.wikipedia.org/wiki/Fisher–Yates_shuffle).
// If **n** is not specified, returns a single random element.
// The internal `guard` argument allows it to work with `map`.
_.sample = function(obj, n, guard) {
if (n == null || guard) {
if (!isArrayLike(obj)) obj = _.values(obj);
return obj[_.random(obj.length - 1)];
}
return _.shuffle(obj).slice(0, Math.max(0, n));

var sample = isArrayLike(obj) ? _.clone(obj) : _.values(obj);
var length = getLength(sample);
n = Math.max(Math.min(n, length), 0);
var rand, temp;
for (var index = length - 1; index > length - n - 1; index--) {
rand = _.random(0, index);
temp = sample[index];
sample[index] = sample[rand];
sample[rand] = temp;
}
return sample.slice(length - n);
};

// Sort the object's values by a criterion produced by an iteratee.
Expand Down

0 comments on commit 42b763e

Please sign in to comment.