When collect() is a mistake

By this point in the track collect() is probably the method you type most. It is the one that turns a chain into something you can look at, print, return, and reason about. That makes it feel like the end of every pipeline, and it is not.

This is a judgement article, not a rule article. Every anti-pattern below has a legitimate twin, and the difference is never syntactic. If you leave with one sentence, make it this one: collect() allocates, and you should be able to say what the allocation is for.

What a collect actually costs

let v: Vec<i64> = src.iter().filter(|x| p(x)).copied().collect();

That line asks the allocator for memory, may reallocate and memcpy several times as it grows (item 9.22 explains when — it depends on size_hint), writes every surviving element into the new buffer, and eventually frees it. If you then walk v once and drop it, every one of those steps was work you did not need.

Chained adapters, by contrast, allocate nothing. filter is a struct holding an iterator and a closure. map is a struct holding an iterator and a closure. The whole tower lives in registers and stack slots, and monomorphisation collapses it into one loop. That is the “zero-cost” claim, and it is broadly true — with the caveats item 9.26 measures.

So the question at every collect() is: what do I need this buffer for?

Anti-pattern 1: collect to count

let n = src.iter().filter(|x| p(x)).collect::<Vec<_>>().len();   // don't
let n = src.iter().filter(|x| p(x)).count();                     // do

The Vec exists for exactly as long as it takes to ask its length. count() is a usize and a loop.

There is no defence for this one. It is the clearest case in the article, and it is also the one that appears most often in real code — usually because the author wrote the collect first while debugging, printed the vector, and then deleted only the println!.

Anti-pattern 2: collect, then immediately iterate again

let names: Vec<String> = rows.iter().map(extract_name).collect();
for n in names.iter() { .. }                                     // don't

for n in rows.iter().map(extract_name) { .. }                    // do

If the only thing you do with a collection is iterate it once, you did not want a collection. The variant with an intermediate binding is worth watching for specifically, because it looks like good style — a named intermediate value with a clear name — and the name is genuinely useful to a reader. Which brings us to the counter-argument, below.

Anti-pattern 3: collect to break a chain in half

let stage1: Vec<_> = src.iter().map(f).collect();
stage1.into_iter().filter(g).map(h).collect()                    // don't

Chains do not have a length limit and adapters compose without cost. src.iter().map(f).filter(g).map(h).collect() does one pass and one allocation instead of two of each.

Anti-pattern 4: collect to a Vec you only index once

let v: Vec<i64> = it.collect();
let first = v[0];                       // don't
let first = it.next();                  // do — or .nth(k), or .find(p)

Related: .collect::<Vec<_>>().into_iter().next() is .next(). .collect::<Vec<_>>().last() on a DoubleEndedIterator is .next_back(). .collect::<Vec<_>>().iter().any(p) is .any(p).

let (n, total) = it.fold((0usize, 0i64), |(n, t), x| (n + 1, t + x));

The counter-argument, in four parts

Now the honest half, because a learner who takes the anti-patterns as rules will write worse code than one who never read them.

1. Collect to end a borrow. This is the big one, and it is completely legitimate. Iterating a collection holds a shared borrow of it; you cannot mutate the collection while that borrow is live.

use std::collections::BTreeMap;

let mut map: BTreeMap<String, i64> = ...;
for k in map.keys() {
    if k.len() > 1 { map.remove(k); }        // E0502
}

error[E0502]: cannot borrow `map` as mutable because it is also borrowed as immutable
 --> src/main.rs:5:26
  |
4 |     for k in map.keys() {
  |              ----------
  |              |
  |              immutable borrow occurs here
  |              immutable borrow later used here
5 |         if k.len() > 1 { map.remove(k); }
  |                          ^^^^^^^^^^^^^ mutable borrow occurs here

The fix is to collect:

let keys: Vec<String> = map.keys().cloned().collect();   // borrow ends here
for k in keys {
    if k.len() > 1 { map.remove(&k); }
}

That Vec is not waste. It is the mechanism by which the borrow is released, and it is buying you a program that compiles. Anyone who tells you to remove it is telling you to fight the borrow checker to save an allocation you probably cannot measure. Say no.

2. Collect to force evaluation order. Iterators are lazy (item 9.8), so side effects inside a chain happen when the chain is consumed, interleaved with everything downstream. If you need all of stage one to happen before any of stage two — because stage one takes a lock, or writes a file, or you want a deterministic log — a collect is a synchronisation point with a clear meaning.

3. Collect because you use it more than once. An iterator is consumed by consuming it. If you need two passes, you need either two iterators or one buffer. Buffering is often both faster and clearer than regenerating the source.

4. Collect for readability. A named intermediate is a comment that the compiler checks. let eligible: Vec<Account> = ...; in the middle of a forty-line function is a real service to the next reader, and the allocation costs nanoseconds. In a hot loop this is a bad trade; in a config parser that runs once at startup it is a good one, and being unable to tell those apart is a more expensive habit than any Vec.

The lint you expect does not fire

Here is the fact worth carrying, because it changes what you can rely on.

clippy::needless_collect is allow-by-default. It lives in the nursery. A default cargo clippy — and the clippy -D warnings gate on this site — will not catch any of the four anti-patterns above.

Verified on clippy 0.1.95: this file passes a default -D warnings run except for one unrelated lint —

pub fn a(v: &[i64]) -> usize { v.iter().filter(|&&x| x > 3).collect::<Vec<_>>().len() }
pub fn b(v: &[i64]) -> usize { v.iter().count() }

error: called `.iter().count()` on a `slice`
  = note: `-D clippy::iter-count` implied by `-D warnings`

Only line 2 is flagged, by iter_count, and that is a different lint about .iter().count() on something that already knows its length. The obviously-wasteful collect().len() on line 1 goes unmentioned. Add #![deny(clippy::needless_collect)] and it appears — “avoid using collect() when not needed” — but nobody’s CI does that by default.

It is in the nursery for a good reason: it has false positives, precisely on the legitimate cases above. Ending a borrow, forcing evaluation order, and using the result twice are all hard for a lint to distinguish from waste. So clippy made the defensible call — a lint that fires on correct code is worse than a lint that misses some bugs — and left it off.

What that means for you: this one is on you. Do not learn “the gate will catch it”. It will not, here or in your job.

The rule that is actually a rule

Not “avoid collect”. Not “chain everything”. This:

Every collect() should have an answer to “what is this buffer for?” — and the answer must not be “so I could call .len() on it”.

If the answer is “to end a borrow”, “because I use it three times”, “because the next stage must not interleave with this one”, or “because a reader needs a name here”, it is a good collect. If you cannot answer at all, you wrote it while debugging and forgot to take it out.