-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: cmp: add CompareBy
function
#71238
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
Comments
Related Issues
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.) |
It would help to show a couple of examples from the standard library or elsewhere where existing code could be changed to use the proposed function. Would the result look better? Thanks. |
Message to Maintainer: Hello 👋, I would like to submit a proposal for integrating the new Key Details:
Relevant PR:
I’ve included test cases to ensure the new function works as expected, and the results are passing successfully. Looking forward to your feedback! 🙏💬 Thanks :)
|
Change https://go.dev/cl/642395 mentions this issue: |
From the proposed doc string:
Presumably that was supposed to be |
I don't quite understand the use cases. In #71241 a use case that you mentioned is sorting an array of structs. But is the code below not what you are trying to do? type Person struct {
age int
}
ppl := []Person{{age: 2}, {age: 1}}
slices.SortFunc(ppl, func(a, b Person) int {
return cmp.Compare(a.age, b.age)
})
fmt.Println(ppl) |
It would be nice if there were a slices.SortBy(ppl, func(p Person) int { return p.age }) That would be very handy. I'm not sure of other places where a key func would be useful like that, though. |
Example from SortFunc. names := []string{"Bob", "alice", "VERA"}
slices.SortFunc(names, func(a, b string) int { return cmp.Compare(strings.ToLower(a), strings.ToLower(b)) })
slices.SortFunc(names, CompareBy(func(n string) string { return strings.ToLower(n) }))
slices.SortFunc(names, CompareBy(strings.ToLower)) The readability improvement will be proportional to how much logic is duplicated, so simple attribute access is the least compelling. Relatedly, the more logic there is the more likely the function or method will already exist - or should - and can be in-lined. type Person struct {
children map[string]int // name: age
}
func (p Person) NumChildren() int {
return len(p.children)
}
func (p Person) ChildAge(name string) int {
return p.children[name]
}
slices.SortFunc(names, func(a, b string) int { return cmp.Compare(p.ChildAge(a), p.ChildAge(b)) })
slices.SortFunc(names, CompareBy(func(n string) int { return p.ChildAge(n) }))
slices.SortFunc(names, CompareBy(p.ChildAge))
slices.SortFunc(ppl, func(a, b Person) int { return cmp.Compare(a.NumChildren(), b.NumChildren()) })
slices.SortFunc(ppl, CompareBy(func(p Person) int { return p.NumChildren() }))
slices.SortFunc(ppl, CompareBy(Person.NumChildren)) I'm not opposed to |
Applying the key function to every pair of V elements on demand at comparison time (O(n log n) calls, each computing two elements) may be significantly less efficient than using the "decorate, sort, undecorate" approach, which calls the key function exactly once for each V element---especially if the key function allocates memory. |
Which happens now with the binary |
I get the concerns about performance with CompareBy. The idea was to make sorting more readable for simple cases, but I see how the repeated key function calls could be less efficient. I agree that SortFunc or SortBy might still be better for more complex or performance-critical situations. The SortBy idea sounds great too, and I’m open to refining this based on your thoughts. Appreciate the input! |
Here's a rundown of the binary
Which leaves these 4, where the limitation of a key function could be justified by improving performance. func CompactBy[K comparable, V any](seq iter.Seq[V], key func(V) K) iter.Seq2[K, []V]
func MaxBy[K cmp.Ordered, V any](seq iter.Seq[V], key func(V) K) []V
func MinBy[K cmp.Ordered, V any](seq iter.Seq[V], key func(V) K) []V
func SortedBy[K cmp.Ordered, V any](seq iter.Seq[V], key func(V) K) iter.Seq[V] { So some basic trade-offs.
|
|
Proposal Details
An addition to the
cmp
package to support a common use case in ordered comparison.The majority of custom comparison functions apply the same transformation to each element. Analogous to Python's key functions, and would integrate well with
Reverse
proposed in #65632.The text was updated successfully, but these errors were encountered: