Rust Code Review: 5 Comments Every Senior Will Make

Introduction: Why Rust is so strict, and how code review helps keep your sanity

You wrote some code in Rust. It compiled — hooray! But here's the trouble: a senior developer opens your Pull Request, sighs, and starts writing comments. Sound familiar? Rust is a language with a unique philosophy: it trusts you, but checks everything. However, the compiler doesn't see what an experienced eye sees. It won't say: "Hey, this could be done more elegantly" or "Your code is leaking — not memory, but performance."

In this article, we will conduct a Rust code review and break down typical senior developer comments. You will learn how to turn "just working code" into idiomatic and reliable code. Let's go!

1. Ignoring the Type System: "Let me just wrap everything in String"

The most common mistake for beginners is using String or generic types where specific types are needed. A senior will say: "Your type should reflect the business logic, not just be a string."

The Problem: Loss of Semantics

Imagine you are writing a function to work with email addresses:

fn send_email(to: String, subject: String, body: String) {\n    // ...\n}

What's wrong here? You could mix up to and subject — the compiler won't notice. A senior would suggest using the newtype pattern:

struct Email(String);\nstruct Subject(String);\nstruct Body(String);\n\nfn send_email(to: Email, subject: Subject, body: Body) {\n    // ...\n}

Seems excessive? In a Rust code review, this approach is the gold standard. It makes the code self-documenting and eliminates an entire class of errors.

2. Overusing clone(): Copying Doesn't Mean Owning

Junior developers often use .clone() everywhere the compiler complains about borrows. A senior will say: "You're just giving up against the borrow checker. Think differently."

Example: Unnecessary clone

fn process(data: Vec<i32>) {\n    let first = data[0].clone();\n    let second = data[1].clone();\n    // ...\n}

i32 implements the Copy trait — clone() here is completely unnecessary. But the problem goes deeper: if you clone large structures (like Vec or String), you take a performance hit.

The correct approach is to work with references:

fn process(data: &[i32]) {\n    let first = &data[0];\n    let second = &data[1];\n    // ...\n}

During a Rust code review, you must explain why you are cloning. If the answer is "to stop the compiler from complaining" — get ready for a scathing comment.

3. Poor Error Handling: .unwrap() Is Not a Final Solution

Seniors hate unwrap() and expect() in production code. Why? Because they turn a potentially recoverable error into a panic. And a panic in Rust means an abrupt termination.

What's Wrong with This Code?

let file_content = std::fs::read_to_string("config.txt").unwrap();

If the file doesn't exist — the program crashes. A senior would suggest:

let file_content = std::fs::read_to_string("config.txt")\n    .map_err(|e| format!("Failed to read config: {}", e))?;

Use the ? operator and custom error types. This is the foundation of idiomatic Rust. In a Rust code review, this is one of the first comments you'll get.

4. Forgotten Performance: Allocations and Iterators

Rust is a systems language. A senior will look not only at safety but also at speed. One typical problem is creating unnecessary collections.

Example: Unnecessary Collection

let numbers: Vec<i32> = (0..100).collect();\nlet doubled: Vec<i32> = numbers.iter().map(|x| x * 2).collect();