Skip to content

Suggest replacing simple loops with specialized folds #2401

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
10 tasks
killercup opened this issue Jan 25, 2018 · 3 comments
Open
10 tasks

Suggest replacing simple loops with specialized folds #2401

killercup opened this issue Jan 25, 2018 · 3 comments

Comments

@killercup
Copy link
Member

killercup commented Jan 25, 2018

A lot of simple loops can be written using fold-like methods in iterators. Clippy should have a set of lints (maybe even a lint group1) that promotes the more functional style.

Have a look at this code:

let xs = &[1, 2, 3, 4]; // <- ignore that this is a slice for a second

let mut res = vec![];
for x in xs {
    res.push(x);
}

Seasoned functional programmers recognize that is is just a fold/reduce. In Rust, you'd write it like this:

xs.iter().fold(vec![], |mut acc, x| { acc.push(x); acc })

Seasoned Rust programmers would use a specialized fold here (basically a shortcut that makes use of FromIterator):

let res: Vec<_> = xs.iter().collect();
  • clippy should suggest using .collect::<Vec<_>>() for loops that just build vectors

Let's make this a bit more complicated:

let xs = &[1, 2, 3, 4];

let mut res = vec![];
for x in xs {
    if *x > 2 { res.push(x); } 
}

This is still a pretty simple loop, and we can rewrite it like this:

let res: Vec<_> = xs.iter().filter(|&x| *x > 2).collect();

(Sadly, it's not a trivial as it first appeared to be because filter passes a reference to the closure, and we need to rewrite that in the comparison we copied from the if. Instead of |&x| *x > 2 we could also generate |x| **x > 2 or |&&x| x > 2.)

  • clippy should suggest using .filter.collect for these kinds of loops

There are of course a bunch more specialized folders we can recognize. For example:

let mut res = false;
for x in xs {
    if *x > 2 { res = true; } 
}

Isn't it wonderfully procedural? It takes 4 lines and inline mutation to find out if an item in that iterator is greater than two. How about suggesting any instead? (Also see this exisiting lint.)

let res = xs.iter().any(|x| *x > 2);
  • clippy should suggest using any instead of simple loops (that on condition match set a boolean to true and break)
  • clippy should suggest using all instead of simple loops (that on condition match set a boolean to false and break)
  • clippy should suggest using find instead of simple loops (that on condition match set an optional value to Some and break)
  • clippy should suggest using position/rposition instead of simple loops (that count iterations and on condition match store the current iteration number in a variable and break)
  • clippy should suggest using max{,_by,_by_key} instead of simple loops (that compare values and store larger one in variable)
  • clippy should suggest using min{,_by,_by_key} instead of simple loops (that compare values and store smaller one in variable)
  • clippy should suggest using sum/product instead of simple loops
  • ➡️ suggest using contains(y) instead of iter().any(|x| x == y) (Suggest replacing 'iter().any(|x| x == y)' with 'contains' where appropriate #2534)

Haven't found an open issue about that, searched for "loop", "reduce", "fold".

Footnotes

  1. Please name the group after an Office Assistant!

@shaleh
Copy link

shaleh commented Feb 9, 2018

This is a great idea. Whenever I bring up fold it is like I unveiled an unknown maths or something :-)

@scottmcm
Copy link
Member

And as a bonus, fold (or all or position or ...) is sometimes faster than for, and is never slower.

@TheIronBorn
Copy link

TheIronBorn commented Mar 17, 2018

Instead of |&x| *x > 2 we could also generate |x| **x > 2 or |&&x| x > 2

I prefer |&&x| in most situations because I find it removes the need to type *x for each mention (similar to match_ref_pats). It doesn't work for every case though.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants