Skip to content

Commit c2cf7fe

Browse files
committed
feat(orderBy): add support for custom comparators
With this change the 3rd argument is interpreted as a comparator function, used to compare the values returned by the predicates. Leaving it empty falls back to the default comparator and a value of `false` is a special case that also falls back to the default comparator but reverses the order. Thus the new implementation is backwards compatible. Helps with angular#12572 (maybe this is as close as we want to get). Fixes angular#13238 Fixes angular#14455 Closes angular#5123 Closes angular#8112 Closes angular#10368
1 parent eb9f7d4 commit c2cf7fe

File tree

2 files changed

+97
-21
lines changed

2 files changed

+97
-21
lines changed

src/ng/filter/orderBy.js

+39-21
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@
196196
*/
197197
orderByFilter.$inject = ['$parse'];
198198
function orderByFilter($parse) {
199-
return function(array, sortPredicate, reverseOrder) {
199+
return function(array, sortPredicate, compareFn) {
200200

201201
if (array == null) return array;
202202
if (!isArrayLike(array)) {
@@ -206,11 +206,13 @@ function orderByFilter($parse) {
206206
if (!isArray(sortPredicate)) { sortPredicate = [sortPredicate]; }
207207
if (sortPredicate.length === 0) { sortPredicate = ['+']; }
208208

209-
var predicates = processPredicates(sortPredicate, reverseOrder);
210-
// Add a predicate at the end that evaluates to the element index. This makes the
211-
// sort stable as it works as a tie-breaker when all the input predicates cannot
212-
// distinguish between two elements.
213-
predicates.push({ get: function() { return {}; }, descending: reverseOrder ? -1 : 1});
209+
var predicates = processPredicates(sortPredicate);
210+
211+
// Define the `compare()` function. Use a default comparator if none is specified.
212+
// A value of `true` means: Use the default comparator, but reverse the order.
213+
var compare = isFunction(compareFn) ? compareFn :
214+
compareFn ? reverseDefaultCompare :
215+
defaultCompare;
214216

215217
// The next three lines are a version of a Swartzian Transform idiom from Perl
216218
// (sometimes called the Decorate-Sort-Undecorate idiom)
@@ -222,26 +224,29 @@ function orderByFilter($parse) {
222224
return array;
223225

224226
function getComparisonObject(value, index) {
227+
// NOTE: We are adding an extra `tieBreaker` value based on the element's index.
228+
// This will be used to keep the sort stable when all the input predicates cannot
229+
// distinguish between two elements.
225230
return {
226231
value: value,
232+
tieBreaker: {value: index, type: 'number'},
227233
predicateValues: predicates.map(function(predicate) {
228234
return getPredicateValue(predicate.get(value), index);
229235
})
230236
};
231237
}
232238

233239
function doComparison(v1, v2) {
234-
var result = 0;
235240
for (var index=0, length = predicates.length; index < length; ++index) {
236-
result = compare(v1.predicateValues[index], v2.predicateValues[index]) * predicates[index].descending;
237-
if (result) break;
241+
var result = compare(v1.predicateValues[index], v2.predicateValues[index]) * predicates[index].descending;
242+
if (result) return result;
238243
}
239-
return result;
244+
245+
return compare(v1.tieBreaker, v2.tieBreaker);
240246
}
241247
};
242248

243-
function processPredicates(sortPredicate, reverseOrder) {
244-
reverseOrder = reverseOrder ? -1 : 1;
249+
function processPredicates(sortPredicate) {
245250
return sortPredicate.map(function(predicate) {
246251
var descending = 1, get = identity;
247252

@@ -260,7 +265,7 @@ function orderByFilter($parse) {
260265
}
261266
}
262267
}
263-
return { get: get, descending: descending * reverseOrder };
268+
return { get: get, descending: descending };
264269
});
265270
}
266271

@@ -277,7 +282,7 @@ function orderByFilter($parse) {
277282

278283
function objectValue(value, index) {
279284
// If `valueOf` is a valid function use that
280-
if (typeof value.valueOf === 'function') {
285+
if (isFunction(value.valueOf)) {
281286
value = value.valueOf();
282287
if (isPrimitive(value)) return value;
283288
}
@@ -295,23 +300,36 @@ function orderByFilter($parse) {
295300
if (value === null) {
296301
type = 'string';
297302
value = 'null';
298-
} else if (type === 'string') {
299-
value = value.toLowerCase();
300303
} else if (type === 'object') {
301304
value = objectValue(value, index);
302305
}
303306
return { value: value, type: type };
304307
}
305308

306-
function compare(v1, v2) {
309+
function defaultCompare(v1, v2) {
307310
var result = 0;
308-
if (v1.type === v2.type) {
309-
if (v1.value !== v2.value) {
310-
result = v1.value < v2.value ? -1 : 1;
311+
312+
var type1 = v1.type;
313+
var type2 = v2.type;
314+
var value1 = v1.value;
315+
var value2 = v2.value;
316+
317+
if (type1 === type2) {
318+
if (type1 === 'string') {
319+
value1 = value1.toLowerCase();
320+
value2 = value2.toLowerCase();
321+
}
322+
if (value1 !== value2) {
323+
result = value1 < value2 ? -1 : 1;
311324
}
312325
} else {
313-
result = v1.type < v2.type ? -1 : 1;
326+
result = type1 < type2 ? -1 : 1;
314327
}
328+
315329
return result;
316330
}
331+
332+
function reverseDefaultCompare(v1, v2) {
333+
return -1 * defaultCompare(v1, v2);
334+
}
317335
}

test/ng/filter/orderBySpec.js

+58
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,18 @@ describe('Filter: orderBy', function() {
1313
toThrowMinErr('orderBy', 'notarray', 'Expected array but received: {}');
1414
});
1515

16+
1617
it('should not throw an exception if a null or undefined value is provided', function() {
1718
expect(orderBy(null)).toEqual(null);
1819
expect(orderBy(undefined)).toEqual(undefined);
1920
});
2021

22+
2123
it('should not throw an exception if an array-like object is provided', function() {
2224
expect(orderBy('cba')).toEqual(['a', 'b', 'c']);
2325
});
2426

27+
2528
it('should return sorted array if predicate is not provided', function() {
2629
expect(orderBy([2, 1, 3])).toEqual([1, 2, 3]);
2730

@@ -44,6 +47,27 @@ describe('Filter: orderBy', function() {
4447
});
4548

4649

50+
it('should compare strings case-insensitively in the default comparator', function() {
51+
var items = [{id: 'item1'}, {id: 'Item2'}, {id: 'item3'}];
52+
53+
expect(orderBy(items, 'id')).toEqual([
54+
items[0],
55+
items[1],
56+
items[2]
57+
]);
58+
});
59+
60+
61+
it('should not convert strings to lowercase for passing to a custom comparator', function() {
62+
var items = [{id: 'item1'}, {id: 'Item2'}];
63+
var comparator = jasmine.createSpy('comparator');
64+
65+
orderBy(items, 'id', comparator);
66+
67+
expect(comparator.calls.argsFor(0)).toContain({value: 'Item2', type: 'string'});
68+
});
69+
70+
4771
it('should sort inherited from array', function() {
4872
function BaseCollection() {}
4973
BaseCollection.prototype = Array.prototype;
@@ -93,6 +117,7 @@ describe('Filter: orderBy', function() {
93117
{ a:new Date('01/01/2014'), b:3 }]);
94118
});
95119

120+
96121
it('should compare timestamps when sorting dates', function() {
97122
expect(orderBy([
98123
new Date('01/01/2015'),
@@ -140,6 +165,7 @@ describe('Filter: orderBy', function() {
140165
expect(orderBy(array)).toEqualData(array);
141166
});
142167

168+
143169
it('should reverse array of objects with no predicate and reverse is `true`', function() {
144170
var array = [
145171
{ id: 2 },
@@ -157,6 +183,38 @@ describe('Filter: orderBy', function() {
157183
});
158184

159185

186+
it('should support a custom comparator', function() {
187+
var items = [
188+
{owner: 'ownerA', type: 'typeA'},
189+
{owner: 'ownerB', type: 'typeB'},
190+
{owner: 'ownerC', type: 'typeB'},
191+
{owner: 'ownerD', type: 'typeB'}
192+
];
193+
var predicates = ['type', '-owner'];
194+
var comparator = function(o1, o2) {
195+
var v1 = o1.value;
196+
var v2 = o2.value;
197+
var hasEra1 = v1.toLowerCase().indexOf('nerd') !== -1;
198+
var hasEra2 = v2.toLowerCase().indexOf('nerd') !== -1;
199+
200+
// Shamelessly promote "nerds"
201+
if (hasEra1 || hasEra2) {
202+
return (hasEra1 && hasEra2) ? 0 : (hasEra1) ? -1 : 1;
203+
}
204+
205+
// No "nerd"; lexicographic order
206+
return (v1 === v2) ? 0 : (v1 < v2) ? -1 : 1;
207+
}
208+
209+
expect(orderBy(items, predicates, comparator)).toEqual([
210+
items[0],
211+
items[2],
212+
items[1],
213+
items[3]
214+
]);
215+
});
216+
217+
160218
it('should reverse array of objects with predicate of "-"', function() {
161219
var array = [
162220
{ id: 2 },

0 commit comments

Comments
 (0)