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

[Bug] sorting #393

Closed
jw-y opened this issue Apr 4, 2024 · 4 comments
Closed

[Bug] sorting #393

jw-y opened this issue Apr 4, 2024 · 4 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@jw-y
Copy link
Contributor

jw-y commented Apr 4, 2024

local com = (a, b) -> a >= b
local a = List(0, 0, 0, 0, 1, 1, 1, 1, 1, 1)
b = a.sortWith(com)

expected output:

b = List(1, 1, 1, 1, 1, 1, 0, 0, 0, 0)

got:

b = List(1, 1, 1, 1, 0, 0, 0, 0, 1, 1)
@jw-y jw-y changed the title [Bug] sorting order [Bug] sorting Apr 4, 2024
@holzensp
Copy link
Contributor

holzensp commented Apr 4, 2024

This is wild! I managed to reproduce. Thanks for the bug report.

@holzensp holzensp added bug Something isn't working good first issue Good for newcomers labels Apr 4, 2024
@translatenix
Copy link
Contributor

translatenix commented Apr 4, 2024

I don't think this is a bug.
The documentation of List.sortWith says:

comparator should return true if its first argument comes before its second argument in the sort order, and false otherwise.

So it should be local com = (a, b) -> a > b instead of local com = (a, b) -> a >= b.

I checked Swift's and Scala's sortWith functions, and they have the same requirement. Swift additionally states that the comparator function must be irreflexive.

@jw-y
Copy link
Contributor Author

jw-y commented Apr 4, 2024

Either case, I don't think this is any way intended.

FYI,
found the location of the bug.

if (comparator.executeWith(array[mid], array[mid], function)) {

first mid should be mid-1

@jw-y jw-y mentioned this issue Apr 4, 2024
@translatenix
Copy link
Contributor

translatenix commented Apr 4, 2024

That indeed looks like a bug. I guess a reflexive comparator function should only give you an unstable/inefficient sort, not a wrong sort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants