Code review — what to look for — Coding notes

WHY does code review exist at all?

WHY it works (first principles):

The author is blind to their own assumptions — they "see" the code they meant to write, not the code they actually wrote. A fresh pair of eyes has no such blind spot.
Bugs get exponentially more expensive the later they're found. Rough cost ladder:

$\text{cost}_{\text{fix}} \;\approx\; c_0 \cdot k^{\,s}$

where $s$ is the stage (0 = at review, 1 = in QA, 2 = in production) and $k>1$ (often quoted $k \approx 10$ ). So catching a bug at review ( $s=0$ ) costs $c_0$ , but in production ( $s=2$ ) it costs $\approx c_0 k^2 = 100\,c_0$ . WHY review pays: you move bugs down the exponent.

Formula Why review is worth it (expected-value derivation)

Let a change contain $n$ latent bugs, each caught at review with probability $p$ .

Expected bugs caught at review: $n p$ , each costing $c_0$ .
Expected bugs escaping to production: $n(1-p)$ , each costing $c_0 k^2$ .

Expected total cost with review: $E_{\text{review}} = n p\,c_0 + n(1-p)\,c_0 k^2 + R$ where $R$ is the reviewer's time. Without review every bug reaches production: $E_{\text{none}} = n\,c_0 k^2$ Review is worth it when $E_{\text{review}} < E_{\text{none}}$ , i.e. $R < n p\,c_0\,(k^2 - 1)$ Read this: the more bugs your change has ( $n$ ↑), the better the reviewer ( $p$ ↑), and the more expensive production failures are ( $k$ ↑), the more reviewing pays off.

WHAT to look for — the priority ladder

Review top-down: high-impact, hard-to-reverse things first. If you nitpick spacing before checking correctness, you've optimized the wrong thing (80/20!).

Correctness — does it do what it claims? Edge cases: empty input, null, off-by-one, max/min values, concurrency.
Design / architecture — does it fit the system? Right place, right abstraction, not reinventing an existing utility. Hardest to fix later → highest priority.
Security — input validation, no secrets in code, SQL injection, authz checks.
Tests — do new tests actually exercise the new behaviour (incl. the failure path)?
Readability / naming — would a newcomer understand this in 6 months?
Performance — only where it matters (hot loops, big- $O$ regressions), not premature.
Style — let a linter/formatter do this; humans shouldn't.

HOW to actually do it (the practice)

HOW, step by step:

Read the PR description + linked ticket first — you can't judge a solution without the problem.
Forecast what you expect the diff to contain. Then read and verify (Forecast-then-Verify).
Pull the change locally / read the tests first — they tell you the intended behaviour.
Trace the happy path, then deliberately hunt edge cases.
Distinguish blocking comments from preferences. Prefix nits clearly: nit:.

Worked examples

Worked example Example 1 — spotting an off-by-one

def last_n(items, n):
    return items[len(items) - n:]   # return last n items

Why review this? Looks fine on the happy path. Forecast: what if n > len(items)? Then len(items) - n is negative, so slicing wraps from the end — silently returns the wrong items. Why this step? Edge cases (n bigger than the list) are where correctness bugs hide. Fix: return items[-n:] if n > 0 else [] and add a test for n > len(items).

Worked example Example 3 — security

query = f"SELECT * FROM users WHERE name = '{name}'"

Why flag it? name comes from user input → classic SQL injection. Why this step? Security beats style; an injected '; DROP TABLE users; -- is catastrophic. Fix: use a parameterized query: cursor.execute("... WHERE name = %s", (name,)).

Common mistakes (Steel-man + fix)

Recall Feynman: explain it to a 12-year-old (click to reveal)

Imagine you wrote a school essay and your friend reads it before you hand it in. You can't see your own spelling mistakes because your brain reads what you meant. Your friend has fresh eyes and catches them. Code review is exactly that — but for the instructions we give a computer. Your friend checks the biggest things first ("does your story even make sense?") before the tiny things ("you missed a comma"), because a confusing story is much worse than a missing comma.

Active-recall flashcards

What is a code review?

Systematic examination of a code change by someone other than the author, before it's merged.

Why can't the author reliably catch their own bugs?

They see the code they meant to write, not what they actually wrote — they're blind to their own assumptions.

In what order should you review?

Correctness → Design → Security → Tests → Readability → Performance → Style (CD STeReo PerSon).

Why is correctness/design reviewed before style?

They're the most expensive to fix later and hardest to reverse; style can be automated by a linter.

Why does bug-fix cost grow with stage?

It grows roughly as

c_0 k^s

— exponentially — so a production bug (

s=2

) can cost ~

k^2

times a review-stage bug.

Review is worth the reviewer time

R

when?

When

R < n p\,c_0\,(k^2-1)

— more bugs, better reviewer, costlier failures all favour reviewing.

Why should you read the tests first?

They document the intended behaviour, telling you what "correct" even means for this change.

Steel-man "it passes the tests so it's correct" — fix?

Tests only cover cases someone wrote; ask which inputs are NOT covered and request that test.

Why split large PRs?

Reviewer defect-detection collapses past ~200–400 lines; small diffs get real scrutiny.

What's the rule about who you comment on?

Comment on the code/change, never the person — keep the author motivated, not defensive.

Connections

Unit testing — tests are what you verify; review checks the tests themselves.
Pull requests and Git workflow — the mechanism reviews ride on.
Technical debt — design comments in review are debt-prevention.
SQL injection and input validation — the security layer of a review.
Big-O notation — needed to spot performance regressions.
Refactoring — readability comments often request small refactors.

Concept Map

Hinglish (regional understanding)

Intuition Hinglish mein samjho

Dekho, code review ka simple matlab hai: jo code tumne likha, usko merge karne se pehle koi doosra banda padhta hai. Kyun? Kyunki apni galti khud ko nahi dikhti — tum wahi padhte ho jo tumne sochke likha tha, na ki jo actually likha. Fresh eyes se chhoti-moti bugs, edge cases, aur design ki galtiyaan pakdi jaati hain. Yaad rakho: bug jitni der baad pakdi jaaye, utna mehnga fix hota hai — review mein 1x, QA mein 10x, production mein 100x. Isliye review sabse sasti jagah hai bug pakadne ki.

Ab kya dekhna hai, uska order important hai. Pehle Correctness (kya code sach mein sahi kaam karta hai, edge cases — empty list, null, off-by-one?), phir Design (sahi jagah pe code hai? pehle se koi utility thi?), phir Security (SQL injection, secrets), phir Tests, phir Readability, phir Performance, aur last mein Style. Mnemonic: "CD STeReo PerSon". Style pe time waste mat karo — woh linter automatically pakad lega. Bade kaam pehle.

Ek bada rule: comment hamesha code pe karo, banda pe nahi. "Yeh loop empty list pe crash karega" bolo, "tu hamesha bhool jaata hai" mat bolo — taaki author defensive na ho, balki fix karne ko motivate ho. Aur agar PR bahut bada hai (2000 lines), toh author se bolo split karo — kyunki 200-400 lines ke baad reviewer ka dhyaan kam ho jaata hai aur woh bina dekhe approve maar deta hai. Chhote diffs ka asli review hota hai.

Code review — what to look for

WHY does code review exist at all?

WHAT to look for — the priority ladder

HOW to actually do it (the practice)

Worked examples

Common mistakes (Steel-man + fix)

Active-recall flashcards

Connections

Concept Map

Hinglish (regional understanding)

Go deeper — visual, from zero

Test yourself — Software Engineering

Connections