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

min_by/max_by on iterators #1722

Closed
phimuemue opened this issue Aug 19, 2016 · 3 comments
Closed

min_by/max_by on iterators #1722

phimuemue opened this issue Aug 19, 2016 · 3 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@phimuemue
Copy link

It seems that there is min, max (simple computation of min/max), min_by_key, max_by_key (min/max by comparing mapped values) but no min_by and max_by (min/max according to comparison function). However, e.g. on vectors or slices there is sort, sort_by_key and sort_by.

Wouldn't it be convenient to have min_by/max_by on iterators?

@phimuemue
Copy link
Author

As far as I could tell, we might reuse select_fold1 as follows:

/// Returns the element that gives the maximum value from the
/// specified comparison function.
///
/// Returns the rightmost element if the comparison determines two elements
/// to be equally maximum.
///
/// # Examples
///
/// ```
/// let a = [-3_i32, 0, 1, 5, -10];
/// assert_eq!(*a.iter().max_by(|x, y| x.cmp(y)).unwrap(), 5);
/// ```
#[inline]
#[unstable(feature = "iter_max_by", issue="1722")]
fn max_by<F>(self, mut compare: F) -> Option<Self::Item>
    where Self: Sized, F: FnMut(&Self::Item, &Self::Item) -> Ordering,
{
    select_fold1(self,
                 |_| (),
                 // switch to y even if it is only equal, to preserve
                 // stability.
                 |_, x, _, y| Ordering::Greater != compare(x, y))
        .map(|(_, x)| x)
}

/// Returns the element that gives the minimum value from the
/// specified comparison function.
///
/// Returns the latest element if the comparison determines two elements
/// to be equally minimum.
///
/// # Examples
///
/// ```
/// let a = [-3_i32, 0, 1, 5, -10];
/// assert_eq!(*a.iter().min_by(|x, y| x.cmp(y)).unwrap(), -10);
/// ```
#[inline]
#[unstable(feature = "iter_min_by", issue="1722")]
fn min_by<F>(self, mut compare: F) -> Option<Self::Item>
    where Self: Sized, F: FnMut(&Self::Item, &Self::Item) -> Ordering,
{
    select_fold1(self,
                 |_| (),
                 // switch to y even if it is strictly smaller, to
                 // preserve stability.
                 |_, x, _, y| Ordering::Greater == compare(x, y))
        .map(|(_, x)| x)
}

@SimonSapin
Copy link
Contributor

Given that all of these are already in the standard library:

  • Iterator::min
  • Iterator::min_by_key
  • Iterator::max
  • Iterator::max_by_key
  • [T]::sort
  • [T]::sort_by
  • [T]::sort_by_key
  • [T]::binary_search
  • [T]::binary_search_by
  • [T]::binary_search_by_key

I think it’s clear that Iterator::min_by and max_by belong there as well and don’t need to go through the full RFC process. Feel free to make pull request on rust-lang/rust to add them.

@nrc nrc added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Aug 20, 2016
@brson
Copy link
Contributor

brson commented Aug 29, 2016

Agree with @SimonSapin. Closing in favor of rust-lang/rust#35856. Thanks @phimuemue.

@brson brson closed this as completed Aug 29, 2016
bors added a commit to rust-lang/rust that referenced this issue Sep 3, 2016
Introduce max_by/min_by on iterators

See rust-lang/rfcs#1722 for reference.

It seems that there is `min`, `max` (simple computation of min/max), `min_by_key`, `max_by_key` (min/max by comparing mapped values) but no `min_by` and `max_by` (min/max according to comparison function). However, e.g. on vectors or slices there is `sort`, `sort_by_key` and `sort_by`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

4 participants