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.

Sign up for GitHub

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

Jump to bottom

Tracking issue for cmp::{min_by, min_by_key, max_by, max_by_key} #64460

Closed
timvermeulen opened this issue Sep 14, 2019 · 3 comments · Fixed by #81047
Closed

Tracking issue for cmp::{min_by, min_by_key, max_by, max_by_key} #64460

timvermeulen opened this issue Sep 14, 2019 · 3 comments · Fixed by #81047
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@timvermeulen
Copy link
Contributor

Tracking issue for the cmp_min_max_by feature, which adds the min_by / min_by_key / max_by / max_by_key functions to std::cmp.

PR: #64047
@timvermeulen timvermeulen mentioned this issue Sep 14, 2019
Merged
@jonas-schievink jonas-schievink added B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 14, 2019
tmandry added a commit to tmandry/rust that referenced this issue Sep 20, 2019
@tmandry
Rollup merge of rust-lang#64047 - timvermeulen:cmp_min_max_by, r=cuviper
29188c5 
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
tmandry added a commit to tmandry/rust that referenced this issue Sep 21, 2019
@tmandry
Rollup merge of rust-lang#64047 - timvermeulen:cmp_min_max_by, r=cuviper
14e672c 
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 added a commit that referenced this issue Sep 21, 2019
@bors
Auto merge of #64047 - timvermeulen:cmp_min_max_by, r=cuviper
5349e69 
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
@Coder-256
Copy link
Contributor

Is there anything blocking this from stabilizing?

@KodrAus KodrAus added I-nominated Libs-Tracked Libs issues that are tracked on the team's project board. labels Jul 29, 2020
@KodrAus
Copy link
Contributor

KodrAus commented Oct 30, 2020

Doesn't look like it! If somebody would like to open a stabilization PR then we can start a FCP 🙂

@glittershark glittershark mentioned this issue Jan 15, 2021
Merged
@glittershark
Copy link
Contributor

Stabilization PR up at #81047

@bors bors closed this as completed in ef2ef92 Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Stabilize cmp_min_max_by glittershark/rust
5 participants
@glittershark @jonas-schievink @timvermeulen @KodrAus @Coder-256