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

Implement Iter::drain(&mut self) with feature iterator_helper #45168

Closed
wants to merge 1 commit into from

Conversation

dns2utf8
Copy link
Contributor

The doc text may be a little short

@rust-highfive
Copy link
Collaborator

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@leonardo-m
Copy link

leonardo-m commented Oct 10, 2017

Regarding the example:
xs.iter().map(|x| sum += x).drain();

Isn't it better written:
xs.iter().for_each(|x| sum += x);

@kennytm
Copy link
Member

kennytm commented Oct 10, 2017

@dns2utf8 I don't think there is consensus on the name of this method yet. See #44546. Furthermore, a new method should not be #[stable].

@aidanhs aidanhs added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 12, 2017
/// assert_eq!(6, sum);
/// ```
#[inline]
#[stable(feature = "iterator_helper", since = "1.22.0")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a minor thing, but to keep it as predictable as possible, iter_methodname or iterator_methodname would be a good feature name. Depends on what the method name will be, of course, just change it with the method name.

@ollie27
Copy link
Member

ollie27 commented Oct 14, 2017

This method should probably take self by value like all of the other consuming methods like count, last, for_each, collect, fold etc...

/// let xs = [0, 1, 2, 3];
/// let mut sum = 0;
///
/// xs.iter().map(|x| sum += x).drain();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a more complex way to write xs.iter().sum() or even xs.iter().for_each(|x| sum += x).

@sfackler
Copy link
Member

I'm not sure I understand why this needs to exist when for_each is already present.

@carols10cents carols10cents added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2017
@carols10cents
Copy link
Member

@dns2utf8 looks like @sfackler has a question about how this differs from for_each?

@carols10cents
Copy link
Member

Hi @dns2utf8, due to inactivity, I'm going to close this PR. Please feel free to reopen this when you have a chance to get back to it. Thank you for your contribution! ❤️

@carols10cents carols10cents added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants