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

Implement collections/sortBy ( closes #1061 ) #1069

Merged
merged 5 commits into from
Aug 3, 2021
Merged

Conversation

LionC
Copy link
Contributor

@LionC LionC commented Jul 27, 2021

This PR adds a sortBy function to the collections module.

sortBy allows you to sort an array without mutating it by an arbitrary selected value like this:

const people = [
    { name: 'Anna', age: 34 },
    { name: 'Kim', age: 42 },
    { name: 'John', age: 23 },
]

const sortedByAge = sortBy(people, it => it.age)

sortBy was part of the original proposal (see #1065 and its links) and in the discussions following the merge of the initial set of functions, seemed to have consensus, which is why I went forward and implemented it.

Notes on the implementation: I chose to limit the selector function to common sortable types. I looked into different options and they all seemed hacky or inconvenient for users of the function. The approach in the PR seemed like the best compromise between not caring about return types at all (and thus not saving people from hard to track bugs) and specifying every possible case.

This closes #1061

@LionC LionC changed the title Implement collections/sortBy Implement collections/sortBy ( solves #1061 ) Jul 27, 2021
@LionC LionC changed the title Implement collections/sortBy ( solves #1061 ) Implement collections/sortBy Jul 27, 2021
@LionC LionC changed the title Implement collections/sortBy Implement collections/sortBy ( closes #1061 ) Jul 27, 2021
collections/sort_by.ts Outdated Show resolved Hide resolved
collections/sort_by.ts Outdated Show resolved Hide resolved
@kt3k
Copy link
Member

kt3k commented Jul 28, 2021

The test cases look good to me.

@LionC LionC requested a review from kt3k July 30, 2021 19:49
Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LionC LGTM. Nice work!

@kt3k kt3k merged commit 94a84bc into denoland:main Aug 3, 2021
@LionC LionC deleted the sort-by branch August 3, 2021 16:45
@LionC LionC restored the sort-by branch August 21, 2021 16:10
@LionC LionC deleted the sort-by branch August 23, 2021 10:54
@LionC LionC mentioned this pull request Aug 27, 2021
44 tasks
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 this pull request may close these issues.

Feature request: sortBy function in std/collections
2 participants