diff --git a/platform/features/table/src/controllers/MCTTableController.js b/platform/features/table/src/controllers/MCTTableController.js index 1470a9e47c..fd949f7056 100644 --- a/platform/features/table/src/controllers/MCTTableController.js +++ b/platform/features/table/src/controllers/MCTTableController.js @@ -286,21 +286,6 @@ define( } }; - /** - * Given a value, if it is a number, or a string representation of a - * number, then return a number representation. Otherwise, return - * the original value. It's a little more robust than using just - * Number() or parseFloat, or isNaN in isolation, all of which are - * fairly inconsistent in their results. - * @param value The value to return as a number. - * @returns {*} The value cast to a Number, or the original value if - * a Number representation is not possible. - */ - MCTTableController.prototype.toNumber = function (value){ - var val = !isNaN(Number(value)) && !isNaN(parseFloat(value)) ? Number(value) : value; - return val; - }; - /** * @private */ @@ -310,17 +295,14 @@ define( sortKey = this.$scope.sortColumn; function binarySearch(searchArray, searchElement, min, max){ - var sampleAt = Math.floor((max - min) / 2) + min, - valA, - valB; + var sampleAt = Math.floor((max - min) / 2) + min; + if (max < min) { return min; // Element is not in array, min gives direction } - valA = self.toNumber(searchElement[sortKey].text); - valB = self.toNumber(searchArray[sampleAt][sortKey].text); - - switch(self.sortComparator(valA, valB)) { + switch(self.sortComparator(searchElement[sortKey].text, + searchArray[sampleAt][sortKey].text)) { case -1: return binarySearch(searchArray, searchElement, min, sampleAt - 1); @@ -358,8 +340,34 @@ define( */ MCTTableController.prototype.sortComparator = function (a, b) { var result = 0, - sortDirectionMultiplier; + sortDirectionMultiplier, + numberA, + numberB; + /** + * Given a value, if it is a number, or a string representation of a + * number, then return a number representation. Otherwise, return + * the original value. It's a little more robust than using just + * Number() or parseFloat, or isNaN in isolation, all of which are + * fairly inconsistent in their results. + * @param value The value to return as a number. + * @returns {*} The value cast to a Number, or the original value if + * a Number representation is not possible. + */ + function toNumber (value){ + var val = !isNaN(Number(value)) && !isNaN(parseFloat(value)) ? Number(value) : value; + return val; + } + numberA = toNumber(a); + numberB = toNumber(b); + + //If they're both numbers, then compare them as numbers + if (typeof numberA === "number" && typeof numberB === "number") { + a = numberA; + b = numberB; + } + + //If they're both strings, then ignore case if (typeof a === "string" && typeof b === "string") { a = a.toLowerCase(); b = b.toLowerCase(); @@ -396,14 +404,7 @@ define( } return rowsToSort.sort(function (a, b) { - //If the values to compare can be compared as - // numbers, do so. String comparison of number - // values can cause inconsistencies - - var valA = self.toNumber(a[sortKey].text), - valB = self.toNumber(b[sortKey].text); - - return self.sortComparator(valA, valB); + return self.sortComparator(a[sortKey].text, b[sortKey].text); }); }; diff --git a/platform/features/table/test/controllers/MCTTableControllerSpec.js b/platform/features/table/test/controllers/MCTTableControllerSpec.js index 436b53bb16..175d8fed0a 100644 --- a/platform/features/table/test/controllers/MCTTableControllerSpec.js +++ b/platform/features/table/test/controllers/MCTTableControllerSpec.js @@ -180,20 +180,6 @@ define( expect(sortedRows[2].col2.text).toEqual('abc'); }); - it('converts number strings to numbers', function () { - var val1 = "", - val2 = "1", - val3 = "2016-04-05 18:41:30.713Z", - val4 = "1.1", - val5 = "8.945520958175627e-13"; - - expect(controller.toNumber(val1)).toEqual(""); - expect(controller.toNumber(val2)).toEqual(1); - expect(controller.toNumber(val3)).toEqual("2016-04-05 18:41:30.713Z"); - expect(controller.toNumber(val4)).toEqual(1.1); - expect(controller.toNumber(val5)).toEqual(8.945520958175627e-13); - }); - it('correctly sorts rows of differing types', function () { mockScope.sortColumn = 'col2'; mockScope.sortDirection = 'desc'; @@ -224,7 +210,24 @@ define( expect(sortedRows[sortedRows.length-1].col2.text).toEqual(''); }); - describe('Adding new rows', function() { + describe('The sort comparator', function () { + it('Correctly sorts different data types', function () { + var val1 = "", + val2 = "1", + val3 = "2016-04-05 18:41:30.713Z", + val4 = "1.1", + val5 = "8.945520958175627e-13"; + mockScope.sortDirection = "asc"; + + expect(controller.sortComparator(val1, val2)).toEqual(-1); + expect(controller.sortComparator(val3, val1)).toEqual(1); + expect(controller.sortComparator(val3, val2)).toEqual(1); + expect(controller.sortComparator(val4, val2)).toEqual(1); + expect(controller.sortComparator(val2, val5)).toEqual(1); + }); + }); + + describe('Adding new rows', function () { var row4, row5, row6;