Added minmax function.

[pongad]

All tests passing. #5268

[huonw]

Could you put an empty line before this sentence? rustdoc will take the first paragraph as the summary, and I think just the first line is appropriate.

[huonw]

Also, it would be worth mentioning that this is more efficient that just min and max, since it does 1.5 comparisions on average.

And, these 3 points could be a markdown list:

The return type `MinMaxResult` is an enum of three variants:
- `NoElements` is returned ...
- `OneElement(x)` is returned ...
- ...

[pongad]

Fixed comments by @huonw .

[huonw]

s/vector/iterator/

[lilyball]

What is the value in using a custom enum with a state for OneElement instead of just using Option and treating the one-element case the same as a two-element case with identical elements (i.e. just return the one element for both min and max)?

[huonw]

That requires Clone, to duplicate the single element into min and max.

[lilyball]

Fair enough. Perhaps MinMaxResult could implement an .into_option() method when T: Clone, so users can benefit from the normal complement of Option methods if the value is cloneable.

[huonw]

Maybe, I'm personally not particularly fussed either way.

Also, @pongad, there appears to be a merge commit in the list of commits, it would be good to remove it before merging this PR (rebasing & force pushing is perfectly ok).

[pongad]

@kballard I'm a little confused by your suggestion, do you mean returning two Option's? With both None for NoElements, one Some and one None for OneElement, etc?
@huonw Will fix that in the next update.

[lilyball]

@pongad I was suggesting having fn min_max(&mut self) -> Option<A>. @huonw's point about requiring Clone is good, though, so now I'm suggesting having

impl<T: Clone> MinMaxResult<T> {
    fn into_option(self) -> Option<(T, T)> {
        match self {
            NoElements => None,
            OneElement(x) => Some((x.clone(), x)),
            MinMax(a, b) => Some((a, b))
        }
    }
}

[bill-myers]

Why not move there the code in "match left_over" and eliminate the left_over variable entirely?

That would slightly shorten the code.

[pongad]

@bill-myers We don't know the length of the iterator, so we don't know ahead of time if we need the left_over or not.

[huonw]

You can inline the match on left_over: i.e. have

let second = match self.next() {
    None => {
        if first < min {
            min = first
        } else if first > max {
            max = first
        }
        break;
    }
}

and just avoid using left_over completely. (It was a silly suggestion from me, sorry.)

Also, you appear to have git troubles: github is telling me that there are 238 changed files. (I've found it easier to just avoid merging entirely, rather than merging and fixing it up later. This also requires avoiding git pull since that merges by default (git fetch + git rebase or git pull --rebase are ok, though).)

[pongad]

@huonw I see it now. Sorry, I completely misunderstood. I just fixed the git problem (I think). I'll make the changes and test, will push when ready.

[pongad]

Fixed comments from @huonw

[lilyball]

I think "are more than one elements" is more grammatically correct as "is more than one element".

[lilyball]

Implementation looks good. Thanks for into_option(). r=me if you fix the comments.

[pongad]

r=kballard

[pongad]

Though, I'm not entirely sure why into_option uses self and not &self. Compilation doesn't like &self, and I'm not quite sure why :(

[lilyball]

@pongad Taking &self would require cloning all values, rather than the limited single clone for the OneElement() case. It would also be called to_option instead of into_option.

[pongad]

@kballard I'm not "fluent" with the way Rust handles borrowing yet, but I think I see what you are saying. That would mean after calling r.into_option(), the original r is rendered useless right? I've also fixed the typo.

[lilyball]

@pongad Yes, r.into_option() will consume the receiver (it will move it into the method call). And thanks for fixing the typo, we'll see what bors does when it finishes testing the previous version.