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

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: A tracking issue for an RFC or an unstable feature. 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

@jonas-schievink jonas-schievink added B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. 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
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
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
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
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
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. 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
Development

Successfully merging a pull request may close this issue.

5 participants