-
Notifications
You must be signed in to change notification settings - Fork 341
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
add stream::min_by method #146
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
use std::cmp::Ordering; | ||
use std::pin::Pin; | ||
|
||
use super::stream::Stream; | ||
use crate::future::Future; | ||
use crate::task::{Context, Poll}; | ||
|
||
/// A future that yields the minimum item in a stream by a given comparison function. | ||
#[derive(Clone, Debug)] | ||
pub struct MinBy<S: Stream, F> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is a future, it should be named |
||
stream: S, | ||
compare: F, | ||
min: Option<S::Item>, | ||
} | ||
|
||
impl<S: Stream + Unpin, F> Unpin for MinBy<S, F> {} | ||
|
||
impl<S: Stream + Unpin, F> MinBy<S, F> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of requiring |
||
pub(super) fn new(stream: S, compare: F) -> Self { | ||
MinBy { | ||
stream, | ||
compare, | ||
min: None, | ||
} | ||
} | ||
} | ||
|
||
impl<S, F> Future for MinBy<S, F> | ||
where | ||
S: futures_core::stream::Stream + Unpin, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, this bound may need to be updated to just be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmm, you probably mean #145 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes indeed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this still unresolved? |
||
S::Item: Copy, | ||
F: FnMut(&S::Item, &S::Item) -> Ordering, | ||
{ | ||
type Output = Option<S::Item>; | ||
|
||
fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> { | ||
let next = futures_core::ready!(Pin::new(&mut self.stream).poll_next(cx)); | ||
|
||
match next { | ||
Some(new) => { | ||
cx.waker().wake_by_ref(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this line needed at all? I believe it can be deleted. |
||
match self.as_mut().min.take() { | ||
None => self.as_mut().min = Some(new), | ||
Some(old) => match (&mut self.as_mut().compare)(&new, &old) { | ||
Ordering::Less => self.as_mut().min = Some(new), | ||
_ => self.as_mut().min = Some(old), | ||
}, | ||
} | ||
Poll::Pending | ||
} | ||
None => Poll::Ready(self.min), | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,10 +21,12 @@ | |
//! # }) } | ||
//! ``` | ||
|
||
use std::cmp::Ordering; | ||
use std::pin::Pin; | ||
|
||
use cfg_if::cfg_if; | ||
|
||
use super::min_by::MinBy; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since |
||
use crate::future::Future; | ||
use crate::task::{Context, Poll}; | ||
use std::marker::PhantomData; | ||
|
@@ -118,6 +120,39 @@ pub trait Stream { | |
} | ||
} | ||
|
||
/// Returns the element that gives the minimum value with respect to the | ||
/// specified comparison function. If several elements are equally minimum, | ||
/// the first element is returned. If the stream is empty, `None` is returned. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// # fn main() { async_std::task::block_on(async { | ||
/// # | ||
/// use std::collections::VecDeque; | ||
/// use async_std::stream::Stream; | ||
/// | ||
/// let s: VecDeque<usize> = vec![1, 2, 3].into_iter().collect(); | ||
/// | ||
/// let min = Stream::min_by(s.clone(), |x, y| x.cmp(y)).await; | ||
/// assert_eq!(min, Some(1)); | ||
/// | ||
/// let min = Stream::min_by(s, |x, y| y.cmp(x)).await; | ||
/// assert_eq!(min, Some(3)); | ||
/// | ||
/// let min = Stream::min_by(VecDeque::<usize>::new(), |x, y| x.cmp(y)).await; | ||
/// assert_eq!(min, None); | ||
/// # | ||
/// # }) } | ||
/// ``` | ||
fn min_by<F>(self, compare: F) -> MinBy<Self, F> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The return type here should use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is
|
||
where | ||
Self: Sized + Unpin, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
F: FnMut(&Self::Item, &Self::Item) -> Ordering, | ||
{ | ||
MinBy::new(self, compare) | ||
} | ||
|
||
/// Tests if every element of the stream matches a predicate. | ||
/// | ||
/// `all()` takes a closure that returns `true` or `false`. It applies | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since
MinBy
is here in a separate file, it'd be a good idea to moveTake
and others into their own files, too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we can do that in a follow-up PR :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #150!