Step 7 of 7

Code review: from working to idiomatic

At work, most of the Rust you write gets reviewed before it's merged, and much of what reviewers flag is the same short list. Many of those comments are automated by clippy (cargo clippy), and the rest you now know from this course. Here are the classics, with the clippy lint name where there is one:

You wrote	Reviewers ask for	clippy lint
`for i in 0..v.len() { v[i] ... }`	`for x in &v`, or an iterator chain	`needless_range_loop`
`fn f(v: &Vec<T>)`, `fn g(s: &String)`	`&[T]`, `&str`	`ptr_arg`
`.clone()` to calm the borrow checker	borrow, or restructure	`redundant_clone` (opt-in)
`v.len() == 0`	`v.is_empty()`	`len_zero`
`if x == true`	`if x`	`bool_comparison`
`return x;` as the last line	just `x`	`needless_return`
`match o { Some(x) => Some(f(x)), None => None }`	`o.map(f)`	`manual_map`
`match o { Some(x) => x, None => d }`	`o.unwrap_or(d)`	`manual_unwrap_or`
`if a { if b { .. } }`	`if a && b { .. }`	`collapsible_if`
a loop that sets `found = true`	`.any(..)` / `.find(..)` / `.position(..)`	(`manual_find` and friends)
`.unwrap()` on input	`?`, `match`, a typed error	`unwrap_used` (opt-in)
`status: String` with `"shipped"`	`enum Status`	(design; no lint)

Teams switch on stricter lints per project, in Cargo.toml:

[lints.clippy]
unwrap_used = "warn"
pedantic = { level = "warn", priority = -1 }

and CI runs cargo clippy -- -D warnings so warnings fail the build.

A small refactor

Both versions below print the same thing. The first is what a newcomer from another language writes; the second is what a reviewer hopes to see.

fn average_score(scores: &Vec<u32>) -> f64 {
    if scores.len() == 0 {
        return 0.0;
    }
    let mut sum = 0;
    for i in 0..scores.len() {
        sum = sum + scores[i];
    }
    return sum as f64 / scores.len() as f64;
}

fn main() {
    println!("{:.2}", average_score(&vec![70, 85, 90]));
    println!("{:.2}", average_score(&vec![]));
}

81.67
0.00

fn average_score(scores: &[u32]) -> f64 {
    if scores.is_empty() {
        return 0.0;
    }
    let sum: u32 = scores.iter().sum();
    sum as f64 / scores.len() as f64
}

fn main() {
    println!("{:.2}", average_score(&[70, 85, 90]));
    println!("{:.2}", average_score(&[]));
}

81.67
0.00

(The early return for the empty case is fine: needless_return only flags a return on the last line.)

How to refactor safely

Pin the behavior first: make sure tests cover what the code does today, including edge cases (empty input, ties, bad input). On this site, the hidden tests play that role.
One change at a time, re-running the tests after each: types first (&Vec to &[T], strings to enums), then loops to iterators, then error handling.
Watch for behavior changes hiding in "cleanups": max_by_key returns the last maximum on ties while a hand-written > loop keeps the first; replacing unwrap with unwrap_or_default turns a crash into silently wrong data. A refactor that changes behavior isn't a refactor.
Turning panics into errors does change the signature (-> Result<..>). That's an improvement, but it's an API change your callers must know about.

Your task

This order-processing code works on valid input, but it failed review. Refactor it:

parse_orders takes &str and returns Result<Vec<Order>, String> instead of panicking. Errors name the line: line 2: bad id, line 1: missing customer (also for an empty customer field), line 3: unknown status "lost", line 1: bad price "abc".
The status becomes enum Status { Pending, Shipped, Cancelled } (derive Debug, Clone, Copy, PartialEq); the text values stay pending, shipped, cancelled.
Slices instead of &Vec, iterators instead of index loops, no .clone(), no .unwrap(), no == true/== false.

total, shipped_revenue and customers_with_pending must behave exactly as before (the names come out sorted, without duplicates).

Challenge 2

A smaller review. Refactor these two functions: take &[String], return Option<&str> from longest_name (None for an empty list, instead of an empty String), and use iterators. Keep the behavior: on a tie, longest_name returns the first of the longest names. No .clone(), no index loops, no return, no len() == 0, no == true.

Challenge 3

Clippy flagged every function here. Rewrite each one with the idiom its comment names, keeping the behavior: no match, no for loops, no return, and no nested ifs left.

Previous: Returning iterators