Add cmp::{min_by, min_by_key, max_by, max_by_key}#64047
Add cmp::{min_by, min_by_key, max_by, max_by_key}#64047bors merged 2 commits intorust-lang:masterfrom timvermeulen:cmp_min_max_by
cmp::{min_by, min_by_key, max_by, max_by_key}#64047Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
Ping from triage Thanks. |
|
☔ The latest upstream changes (presumably #64281) made this pull request unmergeable. Please resolve the merge conflicts. |
Did you try making this change? |
|
@cuviper I did indeed, here's the diff: timvermeulen@aed2e46 It's not a huge win, but it gets rid of the stability logic. And I guess it's also nice to have them in terms of the more general |
|
I like it! Care to add that motivation to this PR? Fill in a tracking issue too, and I think we can land this. |
|
@cuviper Thanks, I added it. |
|
I meant to actually include the iterator commit here, but it can be a followup if you prefer. The tracking issue number needs to be set in the |
|
Ah, I was going to submit a follow-up, but I'm happy to include it here too. |
|
Looks great, thanks! @bors r+ |
|
📌 Commit 7217591 has been approved by |
bors
added
S-waiting-on-bors
Add `cmp::{min_by, min_by_key, max_by, max_by_key}`
This adds the following functions to `core::cmp`:
- `min_by`
- `min_by_key`
- `max_by`
- `max_by_key`
`min_by` and `max_by` are somewhat trivial to implement, but not entirely because `min_by` returns the first value in case the two are equal (and `max_by` the second). `min` and `max` can be implemented in terms of `min_by` and `max_by`, but not as easily the other way around.
To give an example of why I think these functions could be useful: the `Iterator::{min_by, min_by_key, max_by, max_by_key}` methods all currently hard-code the behavior mentioned above which is an ever so small duplication of logic. If we delegate them to `cmp::{min_by, max_by}` methods instead, we get the correct behavior for free. (edit: this is now included in the PR)
I added `min_by_key` / `max_by_key` for consistency's sake but I wouldn't mind removing them. I don't have a particular use case in mind for them, and `min_by` / `max_by` seem to be more useful.
Tracking issue: rust-lang#64460
|
⌛ Testing commit 7217591 with merge 660b5c8cbf99ec24131ddefe0d667303b2bcb15a... |
Add `cmp::{min_by, min_by_key, max_by, max_by_key}`
This adds the following functions to `core::cmp`:
- `min_by`
- `min_by_key`
- `max_by`
- `max_by_key`
`min_by` and `max_by` are somewhat trivial to implement, but not entirely because `min_by` returns the first value in case the two are equal (and `max_by` the second). `min` and `max` can be implemented in terms of `min_by` and `max_by`, but not as easily the other way around.
To give an example of why I think these functions could be useful: the `Iterator::{min_by, min_by_key, max_by, max_by_key}` methods all currently hard-code the behavior mentioned above which is an ever so small duplication of logic. If we delegate them to `cmp::{min_by, max_by}` methods instead, we get the correct behavior for free. (edit: this is now included in the PR)
I added `min_by_key` / `max_by_key` for consistency's sake but I wouldn't mind removing them. I don't have a particular use case in mind for them, and `min_by` / `max_by` seem to be more useful.
Tracking issue: rust-lang#64460
|
@bors retry rolled up |
Add `cmp::{min_by, min_by_key, max_by, max_by_key}`
This adds the following functions to `core::cmp`:
- `min_by`
- `min_by_key`
- `max_by`
- `max_by_key`
`min_by` and `max_by` are somewhat trivial to implement, but not entirely because `min_by` returns the first value in case the two are equal (and `max_by` the second). `min` and `max` can be implemented in terms of `min_by` and `max_by`, but not as easily the other way around.
To give an example of why I think these functions could be useful: the `Iterator::{min_by, min_by_key, max_by, max_by_key}` methods all currently hard-code the behavior mentioned above which is an ever so small duplication of logic. If we delegate them to `cmp::{min_by, max_by}` methods instead, we get the correct behavior for free. (edit: this is now included in the PR)
I added `min_by_key` / `max_by_key` for consistency's sake but I wouldn't mind removing them. I don't have a particular use case in mind for them, and `min_by` / `max_by` seem to be more useful.
Tracking issue: #64460
|
☀️ Test successful - checks-azure |
7 participants
This adds the following functions to
core::cmp:min_bymin_by_keymax_bymax_by_keymin_byandmax_byare somewhat trivial to implement, but not entirely becausemin_byreturns the first value in case the two are equal (andmax_bythe second).minandmaxcan be implemented in terms ofmin_byandmax_by, but not as easily the other way around.To give an example of why I think these functions could be useful: the
Iterator::{min_by, min_by_key, max_by, max_by_key}methods all currently hard-code the behavior mentioned above which is an ever so small duplication of logic. If we delegate them tocmp::{min_by, max_by}methods instead, we get the correct behavior for free. (edit: this is now included in the PR)I added
min_by_key/max_by_keyfor consistency's sake but I wouldn't mind removing them. I don't have a particular use case in mind for them, andmin_by/max_byseem to be more useful.Tracking issue: #64460