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

for x in y { if x.something() { z }} should be for x in y.filter(|x| !x.something()) { z } #678

Closed
oli-obk opened this issue Feb 17, 2016 · 7 comments

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Feb 17, 2016

encountered in the wild: https://github.com/Manishearth/rust-clippy/issues/600#issuecomment-184735383

for x in y {
    if x.something() {
        operate_on(x);
    }
}

and

for x in y {
    if !x.something() { continue }
    operate_on(x);
}

should be

for x in y.filter(|x| x.something()) {
    operate_on(x);
}
@fhartwig
Copy link
Contributor

This is too subjective to make it Warn by default, in my opinion,.

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 17, 2016

This is too subjective to make it Warn by default, in my opinion,.

allow is fine by me

@llogiq
Copy link
Contributor

llogiq commented Feb 17, 2016

Yeah, it's less readable IMHO.

@mcarton
Copy link
Member

mcarton commented Feb 17, 2016

I’m not sure I would want that by default either.

@killercup
Copy link
Member

If we suggest

for x in y.iter().filter() {
    stuff(x);
}

we might as well suggest itertool's foreach because it reads nicer:

y.iter().filter().foreach(stuff);

(Another option would of course be to write

let y = y.iter().filter();
for x in &y {
   stuff(x);
}

but that just seems to split the logic.)

@llogiq
Copy link
Contributor

llogiq commented Feb 21, 2016

A recent thread on internals suggests that different versions of zips that should do the same thing exhibit differing performance characteristics, so we should tread lightly here until they have this figured out.

I personally like for loops for doing stuff when no result is collected and using iterator methods when manipulating sequences or collections of data.

@oli-obk oli-obk closed this as completed May 10, 2017
@oli-obk
Copy link
Contributor Author

oli-obk commented May 10, 2017

I agree that this is not a good idea.

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

No branches or pull requests

5 participants