S11: collect() then iterate again

S11Severity: Low

An intermediate collection that allocates for no reason.

Clippy: needless_collect

What it looks like

Three questions about the unpaid orders, and a Vec for each step:

pub fn unpaid_summary(orders: &[Order]) -> (usize, u64, bool) {
    let unpaid: Vec<&Order> = orders.iter().filter(|o| !o.paid).collect();
    let count = unpaid.len();

    let totals: Vec<u64> = unpaid.iter().map(|o| o.total_cents).collect();
    let sum = totals.iter().sum();

    let big: Vec<&&Order> = unpaid.iter().filter(|o| o.total_cents > 10_000).collect();
    let has_big = !big.is_empty();

    (count, sum, has_big)
}

Why AI agents produce it

It is a step-by-step style. “First get the unpaid orders. Then get their totals. Then sum.” Each step becomes a variable with a Vec type.
Types are easier to see. let unpaid: Vec<&Order> has a type that the agent can name. An iterator type cannot be written out, so the agent collects to get one.
Java streams can be used only once. In Java, collect then stream() again is the usual way to use a result twice. Agents carry the habit over.

Why it is a problem

Each collect() allocates a Vec and copies the items into it, only to read them once and drop the Vec.
len() after collect() is count(). !v.is_empty() after filter().collect() is any(), which also stops at the first match.
Vec<&&Order>: collecting from an iterator of references gives references to references. The type is a sign that the code took a detour.
It hides the pipeline. One chain reads as one computation. Four variables read as four separate steps.

The better version

WorseThree collect() calls, each read once: for a length, for a sum, and for an emptiness check.

pub fn unpaid_summary(orders: &[Order]) -> (usize, u64, bool) {
    let unpaid: Vec<&Order> = orders.iter().filter(|o| !o.paid).collect();
    let count = unpaid.len();

    let totals: Vec<u64> = unpaid.iter().map(|o| o.total_cents).collect();
    let sum = totals.iter().sum();

    let big: Vec<&&Order> = unpaid.iter().filter(|o| o.total_cents > 10_000).collect();
    let has_big = !big.is_empty();

    (count, sum, has_big)
}

BetterA closure that makes the iterator, and count, sum, and any on it. No allocation.

pub fn unpaid_summary(orders: &[Order]) -> (usize, u64, bool) {
    let unpaid = || orders.iter().filter(|o| !o.paid); // a recipe, not a list
    let count = unpaid().count();
    let sum = unpaid().map(|o| o.total_cents).sum();
    let has_big = unpaid().any(|o| o.total_cents > 10_000); // stops at the first match
    (count, sum, has_big)
}

let unpaid = || orders.iter().filter(...) is a closure that returns a new iterator each time it is called. It is cheap: an iterator over a slice is two pointers. The three passes read the orders three times, which is faster than allocating for a list this size. For one pass with three results, a fold or a for loop also works.

Instead of	Write
`.collect::<Vec<_>>().len()`	`.count()`
`!....collect::<Vec<_>>().is_empty()`	`.any(...)` or `.next().is_some()`
`.collect::<Vec<_>>().iter().sum()`	`.sum()`
`.collect::<Vec<_>>().into_iter().map(...)`	`.map(...)` on the first iterator

When it is acceptable

When the next step needs all the items at once:

/// Sorting needs all items at once, so `collect` is necessary here.
pub fn top_customers(orders: &[Order], n: usize) -> Vec<&str> {
    let mut paid: Vec<&Order> = orders.iter().filter(|o| o.paid).collect();
    paid.sort_by_key(|o| std::cmp::Reverse(o.total_cents));
    paid.iter().take(n).map(|o| o.customer.as_str()).collect()
}

Sorting, as here, or anything else that needs random access.
Several passes over an expensive iterator, for example one that reads a file or parses text. Then collect once, and iterate the Vec.
The result must outlive its source, or go to another thread.
Borrowing problems: collect keys into a Vec before you change the map that they came from.

How to detect it

Clippy needless_collect (nursery group, off by default) reports some cases. In the example, it reports only the first Vec, the one used for len(). Enable it with -W clippy::nursery or on its own.
Look for a let with a Vec type that is used only once, on the next line.
Look for Vec<&&T>.

What to tell your agent

Remove the intermediate collect() calls. Use count() instead of collecting
for len(), any() instead of collecting for is_empty(), and continue the
chain instead of collecting and iterating again. Keep a collect() only where
the next step needs all items at once, such as a sort. Run the tests.