-
Notifications
You must be signed in to change notification settings - Fork 439
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
Add capability to sort on a custom mapping (e.g., on a particular member) #89
Comments
@xwu Thanks for opening this issue! I definitely agree that this it would be nice to fill this hole — let's talk a bit about the different ways we could fill it. My primary concern with this approach is that it requires a couple overloads for every method that takes one of the binary /// Wraps a transform's result in a type that reverses the direction of `Comparable` comparisons.
func descending<T, U: Comparable>(_ transform: @escaping (T) -> U) -> (T) -> DescendingComparisons<U> { ... }
let a = persons.sorted(on: descending(\.name))
let b = persons.sorted(on: \.age)
// We _might_ want to add a kind of no-op `ascending` function as well... While this cuts the number of overloads in half, both approaches using A different alternative would be to provide functions that convert the transforming closures/key paths that are most convenient into the binary predicates that the methods expect. If we define one for ascending and one for descending order, we end up with code like this: func ascending<T, U: Comparable>(_ transform: @escaping (T) -> U) -> (T, T) -> Bool { ... }
func descending<T, U: Comparable>(_ transform: @escaping (T) -> U) -> (T, T) -> Bool { ... }
let c = persons.sorted(by: descending(\.name))
let d = persons.sorted(by: ascending(\.age)) These don't include the sort-targeting type in the resulting function type, so it's also possible to compose them without new language features. Of course, the "hero shot" isn't quite as nice this way. Sorting strikes me as the 90% usage case, so perhaps a combined approach, using What do you think? I'm not saying the approach in the PR is the wrong one — just want to explore the space a little more fully. |
@natecook1000 even though you've smuggled this into the existing API, it reads quite fluently: let c = persons.sorted(by: descending(\.name))
let d = persons.sorted(by: ascending(\.age)) |
@natecook1000 I agree with @kylemacomber and actually quite like the latter option: it reads well and composes nicely with APIs we already have. The drawbacks I can see are that I would say in support of the |
Agreed on this — you have to know they're there, and there's a risk of overlapping names with other domains. That said, you don't want to add more context to the name, since the goal is to have a nice, readable call-site.
I can see that, but here I think the brevity of the // With 'on:' parameter:
let e = persons.sorted(on: \.specificGravity, by: { !$1.isTotallyOrdered(belowOrEqualTo: $0) })
// Just using property-access syntax:
let f = persons.sorted(by: { !$1.specificGravity.isTotallyOrdered(belowOrEqualTo: $0.specificGravity) }) If that's the only kind of use case we need it for, I don't think we're getting enough oomph out of the |
I wonder if we can smuggle this into let f = persons.sorted(by: ascending(\.specificGravity, { $0.isTotallyOrdered(belowOrEqualTo: $1) })) Where: func ascending<T, U: Comparable>(
_ transform: @escaping (T) -> U,
_ areInIncreasingOrder: @escaping (U, U) -> Bool = { $0 < $1 }
) -> (T, T) -> Bool {
...
} |
@natecook1000 Sadly not so soon for tuple conformances I think, as the proposal has now expired, I believe. Where the let g = persons.sorted {
$0.specificGravity.squareRoot()
} by: {
$0.isTotallyOrdered(belowOrEqualTo: $1)
}
// As compared to:
let g = persons.sorted {
$0.specificGravity.squareRoot()
.isTotallyOrdered(belowOrEqualTo: $1.specificGravity.squareRoot()))
} (Since we are making contrived examples, ignore the illogic of sorting on the square root of the specific gravity; imagine another function in its place that wouldn't be redundant.) |
For presenting data to a user, it would be nice if there was a way to break a tie, e.g. let c = persons.sorted(by: descending(\.yearOfBirth), ascending(\.name)) Though I realize this could be done by sorting on name first and relying on the sort being stable at the expense of some overhead. |
There seems to be broad appetite for sorting on, say, surnames or ages for a collection of
Person
values. This has been made even more ergonomic now that key path literals can be used where a function takes a closure as an argument.I'd propose, therefore, to add the following
sorted(on:by:)
andsort(on:by:)
additions:The use site would look like the following:
Any interest in such a change?
The text was updated successfully, but these errors were encountered: