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

Add for_each function to Iterator trait #14911

Closed
wants to merge 1 commit into from
Closed

Add for_each function to Iterator trait #14911

wants to merge 1 commit into from

Conversation

aochagavia
Copy link
Contributor

No description provided.

/// a.iter().for_each(|x| println!("{}", x));
/// ```
#[inline]
fn for_each(mut self, f: |A|) {
Copy link
Member

Choose a reason for hiding this comment

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

This should take &mut self like the other consumers (e.g. advance and count), and should be moved down towards the bottom of the trait with them.

@huonw
Copy link
Member

huonw commented Jun 15, 2014

My second comment brings me onto something else... I don't think this method is particularly useful; it's just a constrained version of the advance method (advance can break, for_each cannot) which itself is a constrained version of for (for can return from the outer function, advance cannot). And advance is essentially a relic of the old for loops, which may be removed.

I'm personally quite happy writing for loops for side-effect only behaviours.

@aochagavia
Copy link
Contributor Author

I agree with you that closures with side-effects are ugly. I think, however, that it is tiresome to write 3 lines of code to do something so simple as println!. That is actually the reason I thought it would be nice to have a for_each function. I think other people may also find it useful.

@schmee
Copy link
Contributor

schmee commented Jun 15, 2014

I was looking for something like this yesterday, nice! ref #14666.

EDIT: thestinger has a valid point though.

@aochagavia
Copy link
Contributor Author

I was pointed to an article of Eric Lippert on why they didn't implement a ForEach extension method in C# (http://blogs.msdn.com/b/ericlippert/archive/2009/05/18/foreach-vs-foreach.aspx). Do his arguments apply to Rust as well? Maybe this is too philosophical...

There is an interesting discussion at the end of the article, where arguments in favor of ForEach are given as well. However, the discussion remains philosophical.

I still think it would be nice to have this, because it is very practical.

@thestinger
Copy link
Contributor

I don't understand the reasoning behind adding a method like this. It's strictly uglier than using for loops even with a single expression (usually the closure will need braces too) and lacks the ability to continue and break. It's only going to make the API more complex and will create a needless style decision.

a.iter().for_each(|x| println!("{}", x));
for x in a.iter() { println!("{}", x) }

@bachm
Copy link

bachm commented Jun 15, 2014

Agreed with thestinger, this is pointless.

@aochagavia
Copy link
Contributor Author

After the example of @thestinger it is clear that this is not so convenient as it looked like to me before. If nobody has new arguments, I think this PR should be closed.

@thestinger thestinger closed this Jun 15, 2014
@aochagavia aochagavia deleted the foreach branch July 5, 2014 16:16
cuviper added a commit to cuviper/rust that referenced this pull request Jun 20, 2017
This works like a `for` loop in functional style, applying a closure to
every item in the `Iterator`.  It doesn't allow `break`/`continue` like
a `for` loop, nor any other control flow outside the closure, but it may
be a more legible style for tying up the end of a long iterator chain.

This was tried before in rust-lang#14911, but nobody made the case for using it
with longer iterators.  There was also `Iterator::advance` at that time
which was more capable than `for_each`, but that no longer exists.

The `itertools` crate has `Itertools::foreach` with the same behavior,
but thankfully the names won't collide.  The `rayon` crate also has a
`ParallelIterator::for_each` where simple `for` loops aren't possible.

> I really wish we had `for_each` on seq iterators. Having to use a
> dummy operation is annoying.  - [@nikomatsakis][1]

[1]: https://github.com/nikomatsakis/rayon/pull/367#issuecomment-308455185
@cuviper cuviper mentioned this pull request Jun 20, 2017
bors added a commit that referenced this pull request Jun 30, 2017
Add `Iterator::for_each`

This works like a `for` loop in functional style, applying a closure to
every item in the `Iterator`.  It doesn't allow `break`/`continue` like
a `for` loop, nor any other control flow outside the closure, but it may
be a more legible style for tying up the end of a long iterator chain.

This was tried before in #14911, but nobody made the case for using it
with longer iterators.  There was also `Iterator::advance` at that time
which was more capable than `for_each`, but that no longer exists.

The `itertools` crate has `Itertools::foreach` with the same behavior,
but thankfully the names won't collide.  The `rayon` crate also has a
`ParallelIterator::for_each` where simple `for` loops aren't possible.

> I really wish we had `for_each` on seq iterators. Having to use a
> dummy operation is annoying.  - [@nikomatsakis][1]

[1]: https://github.com/nikomatsakis/rayon/pull/367#issuecomment-308455185
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants