Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sortBy function improve #1247

Closed
wants to merge 1 commit into from
Closed

sortBy function improve #1247

wants to merge 1 commit into from

Conversation

@sjpsega
Copy link
Contributor

@sjpsega sjpsega commented Aug 7, 2013

I use underscore in Adobe PhotoShop js writing;It's js environment is special,it's Array sort function is special,It may happen left and right is equal.

@braddunbar
Copy link
Collaborator

@braddunbar braddunbar commented Aug 7, 2013

Hi @sjpsega! I could be wrong, but I don't think left and right can ever be === since they are created in the _.map above and will most certainly be different objects.

@jdalton

This comment has been minimized.

Copy link

@jdalton jdalton commented on 955d3c7 Aug 8, 2013

I can confirm that in InDesign:

[1,2].sort(function(a,b) { alert([a, b]); return 1; });
// -> alerts 2, 1
// -> alerts 1, 1 <-- issue

This comment has been minimized.

Copy link

@jdalton jdalton replied Aug 8, 2013

But I can't seem to get that behavior to cause a side effect in _.sortBy. Do you have an example of an issue?

alert([1,2,1].sort(function(a,b) { return 0; }));
// alerts 2, 1, 1 (incorrect)
alert(_.sortBy([1,2,1], function(a,b) { return 0; }));
// alerts 1, 2, 1 (correct)

This comment has been minimized.

Copy link

@jdalton jdalton replied Aug 8, 2013

@sjpsega Thanks for the repro, I'll confirm this evening : )

This comment has been minimized.

Copy link

@jdalton jdalton replied Aug 9, 2013

Ok I checked it out and confirmed the issue and the patch fixes it.
I did something similar but modified the last return statement into:

return ai < bi ? -1 : (ai > bi ? 1 : 0);

instead of adding another at the top of the function

This comment has been minimized.

Copy link

@braddunbar braddunbar replied Aug 9, 2013

Thanks @jdalton! I've patched it here as well. I think you can just return ai - bi and get the same effect.

This comment has been minimized.

Copy link

@jdalton jdalton replied Aug 9, 2013

@braddunbar Ah, good catch : )

@sjpsega
Copy link
Contributor Author

@sjpsega sjpsega commented Aug 8, 2013

@braddunbar InDesign(Adobe PhotoShop is) is special.I run my example in chrome、firefox and IE6 is corrent,but in Adobe PhotoShop is incorrect.

@jdalton

[1,2].sort(function(a,b) { alert([a, b]); return 1; });
// -> alerts 2, 1
// -> alerts 1, 1 <-- issue

your example is proof in InDesign may happen left and right be equal.
but your second example is to sample.

My example is:

var testArr = [{num:991},{num:212},{num:11},{num:16},{num:74},{num:0},{num:1515}];
var result = _.sortBy(testArr,function(obj) { 
                                    return obj.num;
                                    });
//in chrome:     [{"num":0},{"num":11},{"num":16},{"num":74},{"num":212},{"num":991},{"num":1515}] (correct)
//in InDesign:   [{"num":0},{"num":212},{"num":11},{"num":16},{"num":74},{"num":991},{"num":1515}] (incorrect)
@braddunbar
Copy link
Collaborator

@braddunbar braddunbar commented Aug 11, 2013

Fixed by #1253.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants