Skip to content

Commit

Permalink
Ensure _.sortBy performs a stable sort. [closes #59]
Browse files Browse the repository at this point in the history
Former-commit-id: 09c5ff85ef0f1d054579ec4260a7f76d9c0da281
  • Loading branch information
jdalton committed Aug 17, 2012
1 parent 83d08e3 commit fab2d69
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 6 deletions.
3 changes: 2 additions & 1 deletion build/pre-compile.js
Expand Up @@ -151,6 +151,7 @@
'head',
'identity',
'include',
'index',
'indexOf',
'initial',
'inject',
Expand Down Expand Up @@ -304,7 +305,7 @@

// minify internal properties used by 'compareAscending', `_.clone`, `_.merge`, and `_.sortBy`
(function() {
var properties = ['criteria', 'source', 'value'],
var properties = ['criteria', 'index', 'source', 'value'],
snippets = source.match(/( +)(?:function clone|function compareAscending|var merge|var sortBy)\b[\s\S]+?\n\1}/g);

if (!snippets) {
Expand Down
17 changes: 12 additions & 5 deletions lodash.js
Expand Up @@ -678,15 +678,18 @@
}

/**
* Used by `sortBy` to compare transformed values of `collection`, sorting
* them in ascending order.
* Used by `sortBy` to compare transformed `collection` values, sorting them
* stabily in ascending order.
*
* @private
* @param {Object} a The object to compare to `b`.
* @param {Object} b The object to compare to `a`.
* @returns {Number} Returns `-1` if `a` < `b`, `0` if `a` == `b`, or `1` if `a` > `b`.
* @returns {Number} Returns the sort order indicator of `1` or `-1`.
*/
function compareAscending(a, b) {
var ai = a.index,
bi = b.index;

a = a.criteria;
b = b.criteria;

Expand All @@ -696,7 +699,9 @@
if (b === undefined) {
return -1;
}
return a < b ? -1 : a > b ? 1 : 0;
// ensure a stable sort in V8 and other engines
// http://code.google.com/p/v8/issues/detail?id=90
return a < b ? -1 : a > b ? 1 : ai < bi ? -1 : 1;
}

/**
Expand Down Expand Up @@ -746,7 +751,7 @@
function isPlainObject(value) {
// avoid non-objects and false positives for `arguments` objects in IE < 9
var result = false;
if (!(value && typeof value == 'object') || (noArgumentsClass && isArguments(value))) {
if (!(value && typeof value == 'object') || (noArgsClass && isArguments(value))) {
return result;
}
// IE < 9 presents DOM nodes as `Object` objects except they have `toString`
Expand Down Expand Up @@ -2272,11 +2277,13 @@
'array':
'result[index] = {\n' +
' criteria: callback(value, index, collection),\n' +
' index: index,\n' +
' value: value\n' +
'}',
'object':
'result' + (isKeysFast ? '[ownIndex] = ' : '.push') + '({\n' +
' criteria: callback(value, index, collection),\n' +
' index: index,\n' +
' value: value\n' +
'})'
},
Expand Down
22 changes: 22 additions & 0 deletions test/test.js
Expand Up @@ -1112,6 +1112,28 @@
QUnit.module('lodash.sortBy');

(function() {
test('should perform a stable sort', function() {
function Pair(x, y) {
this.x = x;
this.y = y;
}

var collection = [
new Pair(1, 1), new Pair(1, 2),
new Pair(1, 3), new Pair(1, 4),
new Pair(1, 5), new Pair(1, 6),
new Pair(2, 1), new Pair(2, 2),
new Pair(2, 3), new Pair(2, 4),
new Pair(2, 5), new Pair(2, 6)
];

var actual = _.sortBy(collection, function(pair) {
return pair.x;
});

deepEqual(actual, collection);
});

test('supports the `thisArg` argument', function() {
var actual = _.sortBy([1, 2, 3], function(num) {
return this.sin(num);
Expand Down

0 comments on commit fab2d69

Please sign in to comment.