When collect() is a mistake

collect() consumes an iterator and stores its items in a new collection, such as a Vec, String or HashMap. It is useful whenever you need to keep the items, return them, or look at them more than once. It is also easy to add where it is not needed. An unneeded collect() usually costs an allocation, and if the collection is then looped over, a second pass over the data.

This article is about judgement rather than syntax. Each wasteful pattern below has a legitimate version that looks almost the same. The question to ask at every collect() is: what is this collection for?

In the examples, src is a slice such as &[i64], it is an iterator, and names such as extract_name, f, g and h stand for ordinary functions or closures.

What a collect costs

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

When at least one element survives the filter, this line asks the allocator for a buffer and writes every surviving element into it. Because filter cannot know in advance how many elements will survive, the Vec starts small and may have to grow, which can mean asking for a larger buffer and copying the elements already stored. (The problem size_hint and DoubleEndedIterator explains how size_hint lets collect reserve space up front.) When v is dropped, the buffer is freed. If you walk v once and then drop it, the allocation, any growth and the extra pass were all avoidable.

Iterator adapters such as filter and map do not allocate by themselves. Each one is a small struct that holds the previous iterator and a closure, and it does its work only when something asks it for the next item. When the optimizer inlines the chain, which it usually does in release builds but does not guarantee, the result is often a loop similar to one you would write by hand. Chains versus loops looks at where that comparison holds and where it does not.

Anti-pattern 1: collect to count

let n = src.iter().filter(|x| **x > 3).collect::<Vec<_>>().len(); // avoid
let n = src.iter().filter(|x| **x > 3).count();                   // prefer

The Vec exists only so that its length can be read. Both lines run the filter on every element and produce the same usize, but count() keeps only a running total and never allocates a buffer. There is no reason to keep the first version.

Anti-pattern 2: collect, then iterate once

let names: Vec<String> = rows.iter().map(extract_name).collect();
for n in &names { /* ... */ }                                     // avoid

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

If the only thing you do with a collection is loop over it once, you can loop over the iterator directly. The intermediate version can look like good style, because names is a helpful name. Reason 4 under "When collect is the right choice" below covers when that name is worth the allocation.

One difference to keep in mind: in the first version extract_name runs for every row before the loop body runs at all. In the second, each call happens just before the loop body sees that name. The results are the same when extract_name and the loop body do not affect each other.

Anti-pattern 3: collect to split a chain in two

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

let out: Vec<_> = src.iter().map(f).filter(g).map(h).collect();  // prefer

A chain can be as long as you need. The single chain makes one pass and builds one Vec. The split version makes two passes and builds two collections, although the standard library may reuse the first buffer for the second when the element types allow it. That reuse is an optimization the documentation permits but does not promise, so it is not a reason to split a chain.

As in anti-pattern 2, splitting changes the order of calls: all calls to f happen before any call to g. If f, g and h have no side effects, both versions produce the same output.

Anti-pattern 4: collect, then read one element

let v: Vec<i64> = it.collect();
let first = v[0];                       // avoid: panics if `it` was empty
let first = it.next();                  // prefer: Option<i64>, None if empty

To read one element you do not need to store them all. .next() takes the first item, .nth(k) skips k items and takes the next one, and .find(p) returns the first item for which p is true. Each returns an Option, so an empty iterator gives None instead of a panic.

Similar replacements:

• .collect::<Vec<_>>().into_iter().next() gives the same result as .next().
• .collect::<Vec<_>>().last() returns a reference to the last item. For an iterator that implements DoubleEndedIterator (one that can also produce items from the back), .next_back() returns the last item itself without walking the rest. Iterator::last() works on any iterator but may walk to the end.
• .collect::<Vec<_>>().iter().any(p) gives the same answer as .any(p), except that the closure receives the item rather than a reference to it.

These replacements give the same results when the closures earlier in the chain have no side effects. They are not identical in what they run: the collect versions consume the whole iterator and run every closure in the chain on every item, while .next(), .find() and .any() stop as soon as they have an answer.

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

When collect is the right choice

The anti-patterns are not rules. Here are four common reasons a collect() is needed.

1. Collect to end a borrow. A loop over map.keys() holds a shared borrow of map for as long as the loop runs. While that borrow is in use, you cannot also borrow map mutably, so this does not compile:

use std::collections::BTreeMap;

fn main() {
    let mut map = BTreeMap::from([("a".to_string(), 1), ("bb".to_string(), 2)]);
    for k in map.keys() {
        if k.len() > 1 {
            map.remove(k);
        }
    }
    println!("{map:?}");
}

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

Collecting copies of the keys first ends the shared borrow before the loop that changes the map:

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

Here the Vec is what makes the program valid. It costs one buffer plus a cloned String per key. For this particular job, removing entries that match a condition, the standard library also offers map.retain(|k, _| k.len() <= 1), which needs no buffer. When the change you want to make does not fit a method like retain, collecting first is the usual approach.

2. Collect to control evaluation order. Iterators are lazy (Trace a lazy map, filter and take pipeline): a closure in a chain runs only when the chain is consumed, and its calls interleave with the stages after it. If every item must finish stage one before any item starts stage two, for example because you want to validate all input and report every error before writing anything, a collect between the stages makes that order explicit.

3. Collect because you use the result more than once. Each pass over an iterator consumes it. If you need two passes, you need either a source you can iterate again (a slice, or an iterator that implements Clone) or a stored copy of the items. Which is cheaper depends on how expensive the items are to produce; if producing them means parsing or reading input that cannot be read twice, storing them is the only option.

4. Collect for readability. A named intermediate value such as let eligible: Vec<Account> = ...; can make a long function easier to follow, and the compiler checks its type. In code that runs once, such as reading a configuration file at startup, the allocation is unlikely to matter. In a loop that runs millions of times, it might, and measuring is the way to find out.

Clippy does not catch these by default

Clippy is Rust's official linter. It has a lint for this problem, clippy::needless_collect, but the lint belongs to the nursery group. Clippy's documentation describes nursery lints as still under development, and they are not enabled by default. A default cargo clippy run, and this site's Rust gate (Clippy with the default lint groups and -D warnings, which turns warnings into errors), will not report it.

We checked this with Clippy 0.1.98 (Rust 1.98.1). This file:

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

produces exactly one error under cargo clippy -- -D warnings:

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

It is for b, from a different lint, iter_count, which points out that a slice already knows its length (v.len()). The wasteful collect::<Vec<_>>().len() in a is not reported. With the same toolchain, versions of the four anti-patterns above produced no needless_collect report at all under the default settings.

You can turn the lint on yourself, with #![warn(clippy::needless_collect)] at the top of the crate or -W clippy::needless_collect on the command line. Then a is reported with "avoid using collect() when not needed". In the same check, the lint also reported anti-pattern 1 and anti-pattern 3, but not the for loop of anti-pattern 2 or any of the anti-pattern 4 forms. Lint coverage changes between Clippy versions, so treat these results as a description of 1.98.1, not a guarantee.

The practical consequence: passing Clippy does not tell you a collect() was needed. Reviewing that is up to you.

The rule to keep

Not "avoid collect", and not "chain everything". Instead:

Every collect() should have an answer to "what is this collection for?", and the answer should not be "so I could call .len() on it".

Good answers include "to end a borrow", "because I use it three times", "because the next stage must not interleave with this one" and "because a reader needs a name here". If there is no answer, the collect() is probably left over from debugging and can be removed.