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

Feature request: sortBy function in std/collections #1061

Closed
andreubotella opened this issue Jul 24, 2021 · 0 comments · Fixed by #1069
Closed

Feature request: sortBy function in std/collections #1061

andreubotella opened this issue Jul 24, 2021 · 0 comments · Fixed by #1069

Comments

@andreubotella
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Array.prototype.sort sorts an array using a callback that takes two elements of the array and must return a number, whose sign will determine their sort order. This is not very intuitive, especially when you just want to compare the elements by the natural ordering of one of their fields, for example.

A related issue is that, if you don't pass a callback to Array.prototype.sort, the default behavior is to stringify the array elements, which is very counterintuitive in cases like [10, 20, 110].sort().

Describe the solution you'd like

Add a sortBy utility function in std/collections, whose callback parameter maps an array element to a value which will then be compared with the mapped values for other array elements, much like the key callback parameter to Python's list.sort. The mapped values would be compared using their natural ordering (a.k.a. by using the regular comparison operators).

And since (as per above), Array.prototype.sort without a callback also has problems, the callback attribute to sortBy should also be optional and default to the identity function.

Describe alternatives you've considered

An alternative would be to have a function that would take a mapping callback and return a callback that can be used with Array.prototype.sort. This would incur additional complexity that probably isn't needed.

Additional context

N/A

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

Successfully merging a pull request may close this issue.

1 participant