Browse files

Fixed bug #339 by adding a stable sort utility

  • Loading branch information...
1 parent b1b59c3 commit 61d4b1312bce6b294998f3715da812b5a847c13c @eolsson eolsson committed Aug 4, 2011
Showing with 86 additions and 6 deletions.
  1. +26 −2 js/highcharts.src.js
  2. +1 −1 js/parts/Series.js
  3. +24 −0 js/parts/Utilities.js
  4. +35 −3 test/unit/UtilitiesTest.js
View
28 js/highcharts.src.js
@@ -478,7 +478,7 @@ ChartCounters.prototype = {
/**
* Utility method extracted from Tooltip code that places a tooltip in a chart without spilling over
- * and not covering the point it self.
+ * and not covering the point it self.
*/
function placeBox(boxWidth, boxHeight, outerLeft, outerTop, outerWidth, outerHeight, point) {
// keep the box within the chart area
@@ -508,6 +508,30 @@ function placeBox(boxWidth, boxHeight, outerLeft, outerTop, outerWidth, outerHei
}
/**
+ * Utility method that sorts an object array and keeping the order of equal items.
+ * ECMA script standard does not specify the behaviour when items are equal.
+ */
+function stableSort(arr, sortFunction) {
+ var length = arr.length,
+ i;
+
+ // Add index to each item
+ for (i = 0; i < length; i++) {
+ arr[i].ss_i = i; // stable sort index
+ }
+
+ arr.sort(function (a, b) {
+ var sortValue = sortFunction(a, b);
+ return sortValue === 0 ? a.ss_i - b.ss_i : sortValue;
+ });
+
+ // Remove index from items
+ for (i = 0; i < length; i++) {
+ delete arr[i].ss_i; // stable sort index
+ }
+}
+
+/**
* Set the global animation to either a given value, or fall back to the
* given chart's animation option
* @param {Object} animation
@@ -8645,7 +8669,7 @@ Series.prototype = {
i;
// sort the data points
- data.sort(function (a, b) {
+ stableSort(data, function (a, b) {
return (a.x - b.x);
});
View
2 js/parts/Series.js
@@ -475,7 +475,7 @@ Series.prototype = {
i;
// sort the data points
- data.sort(function (a, b) {
+ stableSort(data, function (a, b) {
return (a.x - b.x);
});
View
24 js/parts/Utilities.js
@@ -391,3 +391,27 @@ function placeBox(boxWidth, boxHeight, outerLeft, outerTop, outerWidth, outerHei
return {x: x, y: y};
}
+
+/**
+ * Utility method that sorts an object array and keeping the order of equal items.
+ * ECMA script standard does not specify the behaviour when items are equal.
+ */
+function stableSort(arr, sortFunction) {
+ var length = arr.length,
+ i;
+
+ // Add index to each item
+ for (i = 0; i < length; i++) {
+ arr[i].ss_i = i; // stable sort index
+ }
+
+ arr.sort(function (a, b) {
+ var sortValue = sortFunction(a, b);
+ return sortValue === 0 ? a.ss_i - b.ss_i : sortValue;
+ });
+
+ // Remove index from items
+ for (i = 0; i < length; i++) {
+ delete arr[i].ss_i; // stable sort index
+ }
+}
View
38 test/unit/UtilitiesTest.js
@@ -141,21 +141,53 @@ UtilTest.prototype.testPlaceBox = function () {
boxPoint = placeBox(tooltipSize.width, tooltipSize.height, chartRect.x, chartRect.y, chartRect.width, chartRect.height, dataPoint);
extend(boxPoint, tooltipSize);
- jstestdriver.console.log(boxPoint.x, boxPoint.y, boxPoint.width, boxPoint.height);
assertTrue('Left rectInRect chart', this.rectInRect(boxPoint, chartRect));
assertFalse('Left tooltip cover point', this.pointInRect(dataPoint.x, dataPoint.y, boxPoint));
dataPoint.x = 100;
boxPoint = placeBox(tooltipSize.width, tooltipSize.height, chartRect.x, chartRect.y, chartRect.width, chartRect.height, dataPoint);
extend(boxPoint, tooltipSize);
- jstestdriver.console.log(boxPoint.x, boxPoint.y, boxPoint.width, boxPoint.height);
assertTrue('Right rectInRect chart', this.rectInRect(boxPoint, chartRect));
assertFalse('Right tooltip cover point', this.pointInRect(dataPoint.x, dataPoint.y, boxPoint));
dataPoint.x = 50;
boxPoint = placeBox(tooltipSize.width, tooltipSize.height, chartRect.x, chartRect.y, chartRect.width, chartRect.height, dataPoint);
extend(boxPoint, tooltipSize);
- jstestdriver.console.log(boxPoint.x, boxPoint.y, boxPoint.width, boxPoint.height);
assertTrue('Mid rectInRect chart', this.rectInRect(boxPoint, chartRect));
assertFalse('Mid tooltip cover point', this.pointInRect(dataPoint.x, dataPoint.y, boxPoint));
};
+
+/**
+ * Tests that the stable sort utility works.
+ */
+UtilTest.prototype.testStableSort = function () {
+ // Initialize the array, it needs to be a certain size to trigger the unstable quicksort algorithm.
+ // These 11 items fails in Chrome due to its unstable sort.
+ var arr = [
+ {a: 1, b: 'F'},
+ {a: 2, b: 'H'},
+ {a: 1, b: 'G'},
+ {a: 0, b: 'A'},
+ {a: 0, b: 'B'},
+ {a: 3, b: 'J'},
+ {a: 0, b: 'C'},
+ {a: 0, b: 'D'},
+ {a: 0, b: 'E'},
+ {a: 2, b: 'I'},
+ {a: 3, b: 'K'}
+ ],
+ result = [];
+
+ // Do the sort
+ stableSort(arr, function (a, b) {
+ return a.a - b.a;
+ })
+
+ // Collect the result
+ for (var i = 0; i < arr.length; i++) {
+ result.push(arr[i].b);
+ }
+
+ assertEquals('Stable sort in action', 'ABCDEFGHIJK', result.join(''));
+ assertUndefined('Stable sort index should not be there', arr[0].ss_i);
+};

0 comments on commit 61d4b13

Please sign in to comment.