views:

73

answers:

3

I just got a report that some code I wrote is breaking on Chrome. I've tracked it down to a custom method I'm using to sort an array of objects. I am really tempted to call this a bug, but I'm not sure it is.

In all other browsers when you sort an array of objects, if two objects resolve to the same value their order in the updated array is left unchanged. In Chrome, their order is seemingly randomized. Run the code below in Chrome and any other browser you want. You should see what I mean.

I have two questions:

First, was I right in assuming that when your custom sorter returns 0 that the two compared items should remain in their original order (I have a feeling I was wrong).

Second, is there any good way of fixing this? The only thing I can think of is adding an auto-incrementing number as an attribute to each member of the array before sorting, and then using that value when two items sort is comparing resolve to the same value. In other words, never return 0.

Here's the sample code:

var x = [
{'a':2,'b':1},
{'a':1,'b':2},
{'a':1,'b':3},
{'a':1,'b':4},
{'a':1,'b':5},
{'a':1,'b':6},
{'a':0,'b':7},
]

var customSort = function(a,b) {
    if (a.a === b.a) return 0;
    if (a.a > b.a) return 1;
    return -1;
};

console.log("before sorting");
for (var i = 0; i < x.length; i++) {
    console.log(x[i].b);
}
x.sort(customSort);

console.log("after sorting");
for (var i = 0; i < x.length; i++) {
    console.log(x[i].b);
}

In all other browsers, what I see is that only the first member and the last member of the array get moved (I see 7,2,3,4,5,6,1) but in Chrome the inner numbers are seemingly randomized.

[EDIT] Thank you very much to everyone who answered. I guess that 'inconsistent' doesn't necessary mean it's a bug. Also, I just wanted to point out that my b property was just an example. In fact, I'm sorting some relatively wide objects on any one of about 20 keys according to user input. Even keeping track of what the user last sorted by still won't solve the problem of the randomness I'm seeing. My work-around will probably be a close variation of this (new code is highlighted):

var x = [
{'a':2,'b':1},
{'a':1,'b':2},
{'a':1,'b':3},
{'a':1,'b':4},
{'a':1,'b':5},
{'a':1,'b':6},
{'a':0,'b':7},
];
var i;

var customSort = function(a,b) {
    if (a.a === b.a) return a.customSortKey > b.customSortKey ? 1 : -1; /*NEW CODE*/
    if (a.a > b.a) return 1;
    return -1;
};

console.log("before sorting");
for (i = 0; i < x.length; i++) {console.log(x[i].b);}

for (i = 0; i < x.length; i++) {                      /*NEW CODE*/
    x[i].customSortKey = i;                           /*NEW CODE*/
}                                                     /*NEW CODE*/
x.sort(customSort);

console.log("after sorting");
for (i = 0; i < x.length; i++) {console.log(x[i].b);}

EDIT: According to the ECMA spec, when two objects are determined to be equal in a custom sort, JavaScript is not required to leave those two objects in the same order. Both Chrome and Opera are the only two major browsers to choose to have non-stable sorts, but others include Netscape 8&9, Kazehakaze, IceApe and a few others. The Chromium team has marked this bug as "Working as intended" so it will not be "fixed". If you need your arrays to stay in their original order when values are equal, you will need to employ some additional mechanism (such as the one above). Returning 0 when sorting objects is effectively meaningless, so don't bother.

+4  A: 

The V8 sort is not stable, unfortunately. I'll see if I can dig up the Chromium bug about this.

lawnsea
[V8 issue 90: V8 doesn't stable sort](http://code.google.com/p/v8/issues/detail?id=90 "V8 bug 90")
lawnsea
+6  A: 

The ECMAScript standard does not guarantee Array.sort is a stable sort. Chrome (the V8 engine) uses in-place QuickSort internally (for arrays of size ≥ 22, else insertion sort) which is fast but not stable.

To fix it, make customSort compare with .b as well, eliminating the need of stability of the sorting algorithm.

KennyTM
+1  A: 

May be you know it already but you can use an array to sort on multiple columns and avoid this bug:

var customSort = function(a,b) {
    return [a.a, a.b] > [b.a, b.b] ? 1:-1;
}
Mic