Reading code like a reviewer

On a real team, almost nothing reaches production without another developer reading it first. That is code review, and it is one of the most valuable things a team does — and a skill you are judged on from your first week, both as the person whose code is reviewed and, soon, as a reviewer. This lesson closes the best-practices module by turning everything in it into a lens you can read code through.

Why review exists

Code review is not about catching typos — the compiler and tests do that. It exists to:

Catch bugs a test would miss — an edge case, a race, a null the author did not consider.
Spread knowledge — now two people understand this code, not one, so the bus factor is not one.
Keep the codebase coherent — consistent style, naming, and structure, so it reads as if one careful person wrote it.
Teach, in both directions — the reviewer learns the change; the author learns from the feedback.

The mindset that matters: review the code, not the coder. Feedback is about the change, never the person. "This could be null here" is useful; "you always forget null" is not. As the author, receive feedback the same way — it is about making the code better, not a verdict on you. Teams where review is a shared craft, not a gauntlet, ship better software and are better to work in.

The reviewer's checklist

When you read a change (your own before you submit it, or a teammate's), work down these, roughly in order of importance. Notice that every item is a lesson from this course.

1. Correctness — does it do the right thing?

Does it actually solve the problem, including the edge cases?
Nulls: is every nullable handled, or is there a hopeful !!? (Null safety module.)
Concurrency: is there shared mutable state accessed from coroutines without a guard? (The race condition — the counter that lost two-thirds of its updates.)
Off-by-one and ranges: .. versus ..<, empty collections, reduce on an empty list.
Errors: are failures handled, or is there an empty catch { } swallowing them?

2. Readability — can you understand it without asking the author?

Do the names reveal intent and units? (Naming lesson.)
Does each function do one thing at one level of abstraction? Could you read it without reading what it calls? (Functions lesson.)
Is it idiomatic — expression when, ?: for defaults, val by default — or translated Java? (Idiomatic lesson.)
Is anything too clever — a nested chain of scope functions that a plain val would beat?

3. Design — does it fit?

Is the data modelled honestly? Right structure (List/Set/Map), right immutability, a sealed class where the cases are fixed?
Does it duplicate something that already exists?
Are the failure modes represented well — exception, nullable, or sealed type as appropriate? (Errors lesson.)

4. Tests — is it proven?

Is there a test for the new behaviour, including the edge cases and the failure paths?
Would the test actually fail if the code were wrong? (A test that passes no matter what proves nothing — the testing module's point.)

If a change is correct, readable, well-designed, and tested, it is ready. If not, the review says specifically what would make it so.

Reading a change: a worked eye

Here is a snippet as it might arrive in a review. Read it as a reviewer before reading the notes:

fun process(data: List<String>): Int {
    var result = 0
    for (i in 0..data.size) {
        val n = data[i].toInt()
        if (n > 0) result = result + n
    }
    return result
}

A careful reviewer flags several things, each a lesson from this course:

0..data.size is off by one — it should be 0..<data.size; data[data.size] throws IndexOutOfBoundsException. (Ranges.)
data[i].toInt() throws on non-numeric input — no handling; toIntOrNull() and a skip would be safer, and the failure mode was not considered. (Errors.)
The names say nothing — process, data, result, n. What is this summing? (Naming.)
It is a loop that should be a collection operation — the whole function is data.mapNotNull { it.toIntOrNull() }.filter { it > 0 }.sum(). (Collections.)

The idiomatic, correct version:

fun sumOfPositives(numbers: List<String>): Int =
    numbers.mapNotNull { it.toIntOrNull() }
        .filter { it > 0 }
        .sum()

Shorter, correct at the boundaries, safe on bad input, named for what it does, and a single expression. A good review turns the first version into the second — not by rewriting it for the author, but by pointing at each issue so the author learns to see it too.

Giving and receiving feedback well

The human half matters as much as the technical:

Be specific and kind. "Consider 0..<size here — data[size] would throw" is actionable. "This is buggy" is not.
Distinguish must-fix from nice-to-have. A crash is a blocker; a naming preference is a suggestion. Say which is which, so the author knows what is required.
Ask, do not command, when it is a judgement call. "Would a when read better here?" invites thought; "change this to a when" shuts it down. Sometimes the author has a reason.
As the author, do not defend — understand. If a reviewer misread the code, that is a signal the code was unclear. Explain, then usually make it clearer anyway.
Praise good code too. Review is not only for faults; noting a neat solution teaches and encourages.

The whole course, as one habit

Step back and see what this module was really for. Naming, function size, idioms, error handling, and review are not separate rules — they are one habit: write code for the human who reads it next. The compiler does not care about your names; tests do not care about your abstraction levels; the program runs the same either way. All of it is for the reader — your teammate, your reviewer, and your future self. An engineer who internalises that — who writes every line thinking "will the next person understand this quickly and safely?" — is the one worth hiring, and the one a team wants to keep. That is the difference this whole course has been building toward, and the capstone is where you put it into practice.

Check your work

What code review is for. Catching bugs tests miss, spreading knowledge, keeping the codebase coherent, and teaching — not catching typos.

The core mindset. Review the code, not the coder; feedback is about the change, never the person.

The four review dimensions, in order. Correctness, readability, design, tests.

Correctness things to check. Edge cases, null handling, shared mutable state in coroutines, off-by-one/ranges, swallowed errors.

Readability things to check. Intent-revealing names, one-thing functions, idiomatic style, and nothing too clever.

What makes a test worth having. It would actually fail if the code were wrong.

How to give feedback well. Specific and kind, must-fix versus nice-to-have, ask on judgement calls, and praise good work.

How to receive it well. Do not defend — if it was misread, the code was unclear; explain, then clarify.

The one habit behind the whole module. Write code for the human who reads it next.

Practice

Review the buggy process function yourself before reading the notes. List every issue you find, then compare.
Rewrite it into the idiomatic, correct version and confirm it handles the empty list and bad input.
Take a piece of your own code and review it against the four-dimension checklist. Find at least three things to improve.
Rewrite one review comment from "this is wrong" into specific, kind, actionable feedback.
Find a !!, an empty catch, or an off-by-one range in code you can access, and write the review note that would flag it.
Pair with someone (or imagine it): review their small change, marking must-fix versus nice-to-have.
Write down the one-sentence habit this module comes down to, in your own words, and pin it where you code.

Official documentation

Kotlin — Coding conventions — The shared style a review checks against.
Google — Engineering practices: how to do a code review — A widely-used, language-agnostic reviewer's guide.
Google — The CL author's guide — The other half: how to be reviewed well.

Next module — Design Patterns, the Kotlin way: which classic patterns the language dissolves, and which few still earn their place.