Code review is the skill that decides whether you look like a junior or a senior, because most of your career is spent reading code someone else wrote, not writing fresh code on an empty screen. This post teaches you how to walk into a messy file you have never seen, understand it, prove you understand it, and improve it without breaking anything.

“Indeed, the ratio of time spent reading versus writing is well over 10 to 1. We are constantly reading old code as part of the effort to write new code.”

Robert C. Martin, Clean Code

Last Updated: July 2026 | Tested on: Python 3.14.6 | Difficulty: Intermediate | Reading Time: 16 minutes

Here is the part nobody warns you about. On your first day at a real job, nobody hands you a blank file and says “build something.” They point at a script that has been running in production for three years, written by a person who left the company, and say “make it faster” or “add a discount rule.” At Amazon and Meta, the interview loop even has a round for exactly this: read unfamiliar code out loud and reason about it. Yet almost no free course teaches it. That is the gap we close today.

Think of code review like a mechanic inheriting a car another shop half-fixed. You do not rip the engine out on day one. You listen to it run, you read the service history, you make one small change, and you check the engine still starts before you touch the next thing. That patience is the whole job.

What Code Review Really Is

Code review has two faces, and they are really the same skill. The first is reading code to understand it, which you do every time you open a file that is not yours. The second is reading a teammate’s proposed change on a pull request and leaving comments, which is the gate that keeps bad code out of the main branch. Both come down to the same question: does this code do what it claims, safely, and can the next person understand it?

The goal of a review is never to show off. It is to catch bugs early, spread knowledge so no single person is the only one who understands a file, and keep the codebase readable a year from now. A good reviewer is a second pair of eyes, not a judge. We will build the reading muscle first on a real messy script, then learn the etiquette of leaving comments.

Meet the Messy Inventory Script

Say a colleague named Aviraj left the team and handed you this shop inventory script. It runs, the manager relies on it every morning, and now you have to add a feature. Save it as stock.py and read it once before you judge it. This is the kind of code you inherit, single-letter names, a bare except, magic numbers pasted mid-file, and everything at module level with no functions.

📄 stock.py: the script you inherited

# stock.py -- inventory report. it works, but nobody wants to touch it.
import json

items = [
    {"n": "rice", "q": 3, "p": 45.0, "s": 12},
    {"n": "lentils", "q": 20, "p": 80.0, "s": 4},
    {"n": "flour", "q": 1, "p": 30.0, "s": 25},
    {"n": "sugar", "q": 8, "p": 42.0, "s": 9},
    {"n": "salt", "q": 0, "p": 20.0, "s": 15},
    {"n": "oil", "q": 6, "p": 150.0, "s": 7},
]

t = 0
r = []
for i in items:
    try:
        t = t + i["p"] * i["s"]
    except:
        pass
    if i["q"] < 5:
        r.append(i["n"])

# apply the festival discount
t2 = t
if t > 5000:
    t2 = t * 0.9

print("revenue", t)
print("after discount", t2)
print("reorder", r)
print("count", len(r))

▶ Output

revenue 3338.0
after discount 3338.0
reorder ['rice', 'flour', 'salt']
count 3

What happened here: the script sums each item’s price times units sold into t, collects the names of low-stock items into r, and would apply a ten percent discount if revenue crossed 5000. Revenue landed at 3338, so no discount applied, and three items are below the reorder line. It works. The problem is not that it is wrong today, it is that t, r, q, s, the number 5000, and that silent except make it a minefield the moment anyone edits it. That import of json is never even used.

How to Read Code You Did Not Write

The mistake beginners make is reading top to bottom like a novel. Experienced reviewers read in a deliberate order instead: find where the program starts, follow the data as it moves, then hunt for side effects, the moments the code touches the outside world (files, the network, a database, global variables, anything printed). Only once you understand what it does do you write anything down. The flowchart below is the exact loop I run in my head on every unfamiliar file.

Applied to stock.py: the entry point is the top-level loop (there is no main, which is itself a note to make). The data flow is items going in and t and r coming out. The side effects are the four print calls and the unused json import hinting someone once saved to a file. Reading in that order, the shape of the program appears in about a minute, long before you understand every line.

What the Bare except Was Hiding

The scariest line in that script is except: pass. A bare except catches every error, including ones you never meant to ignore, and pass throws them away without a word. The danger is not theoretical. Here is the same pattern on data with one missing field, the kind of row that sneaks into any real dataset.

📄 silent_except.py: the trap in miniature

# What a bare "except: pass" quietly hides.
rows = [
    {"p": 45.0, "s": 12},
    {"p": 30.0},            # someone forgot the "s" (sold) field
    {"p": 20.0, "s": 15},
]

# the messy way: swallow every error, say nothing
total_a = 0
for row in rows:
    try:
        total_a += row["p"] * row["s"]
    except:
        pass
print("bare except total:", total_a)

# the reviewed way: catch the specific error and speak up
total_b = 0
for n, row in enumerate(rows):
    try:
        total_b += row["p"] * row["s"]
    except KeyError as missing:
        print(f"row {n} is missing field {missing}, skipping it")
print("explicit total:", total_b)

▶ Output

bare except total: 840.0
row 1 is missing field 's', skipping it
explicit total: 840.0

What happened here: both versions produce the same total, 840, but only the second one tells you that a row was dropped. In the first version, a real bug (a missing field, a typo, a None that should have been a number) vanishes silently and your report is quietly wrong forever. Catching KeyError specifically, and saying something when it happens, turns an invisible data problem into a visible line you can act on. That is the single highest-value comment you will leave on most reviews: name the exception you actually expect.

Pin the Behavior with a Characterization Test

Before you change one character of inherited code, you write a test that captures what it does right now, bugs and all. This is called a characterization test, and it is your safety net. It does not ask “is this correct?” It asks “did my change alter the output?” If the answer stays no, you can refactor fearlessly. Think of it like photographing a room before you rearrange the furniture, so you can prove exactly what moved.

📄 test_characterization.py: freeze the current output

"""Pin the CURRENT behavior before changing anything."""
import subprocess
import sys

GOLDEN = (
    "revenue 3338.0\n"
    "after discount 3338.0\n"
    "reorder ['rice', 'flour', 'salt']\n"
    "count 3\n"
)


def run(script):
    result = subprocess.run(
        [sys.executable, script], capture_output=True, text=True
    )
    return result.stdout


def check(script):
    output = run(script)
    if output == GOLDEN:
        print(f"PASS: {script} matches the golden output")
    else:
        print(f"FAIL: {script} changed behavior")
        print("--- expected ---")
        print(GOLDEN)
        print("--- got ---")
        print(output)


check(sys.argv[1])

▶ Output: python test_characterization.py stock.py

PASS: stock.py matches the golden output

What happened here: we ran the untouched script, captured its exact printed bytes as GOLDEN, then wrote a test that runs the script again and compares. Right now it trivially passes because nothing changed. The value comes next: after every edit we rerun this, and the moment the output drifts, we know the refactor broke something. Running the script as a subprocess lets us test a plain script that has no functions to import yet, which is common with inherited code. When you learn pytest you will write these as real test functions, but the idea is identical.

Refactor in Small, Reviewed Commits

Now, with the net in place, we clean up. The rule is one idea per commit: name the constants, then extract functions, then fix the risky except, running the characterization test after each step. Small commits are easier to review and trivial to undo with git restore or revert if one goes wrong. Here is stock.py after the refactor, output identical to the original.

📄 stock.py: after review, same behavior

"""Inventory report: revenue, festival discount, and the reorder list."""

REORDER_THRESHOLD = 5           # restock an item when stock falls below this
DISCOUNT_MIN_REVENUE = 5000.0   # festival discount applies above this revenue
DISCOUNT_RATE = 0.10            # 10 percent off

items = [
    {"name": "rice", "stock": 3, "price": 45.0, "sold": 12},
    {"name": "lentils", "stock": 20, "price": 80.0, "sold": 4},
    {"name": "flour", "stock": 1, "price": 30.0, "sold": 25},
    {"name": "sugar", "stock": 8, "price": 42.0, "sold": 9},
    {"name": "salt", "stock": 0, "price": 20.0, "sold": 15},
    {"name": "oil", "stock": 6, "price": 150.0, "sold": 7},
]


def total_revenue(items):
    return sum(item["price"] * item["sold"] for item in items)


def apply_festival_discount(revenue):
    if revenue > DISCOUNT_MIN_REVENUE:
        return revenue * (1 - DISCOUNT_RATE)
    return revenue


def items_to_reorder(items):
    return [item["name"] for item in items if item["stock"] < REORDER_THRESHOLD]


def main():
    revenue = total_revenue(items)
    discounted = apply_festival_discount(revenue)
    reorder = items_to_reorder(items)
    print("revenue", revenue)
    print("after discount", discounted)
    print("reorder", reorder)
    print("count", len(reorder))


if __name__ == "__main__":
    main()

▶ Output: python test_characterization.py stock.py

PASS: stock.py matches the golden output

What happened here: the single-letter names became words, the magic numbers 5, 5000, and 0.9 became named constants with comments explaining the intent, each job moved into its own small function, and the dead json import is gone. The characterization test still prints PASS, which is the proof that matters: we made the code far easier to read without changing a single number in the report. That is a clean review. If the test had printed FAIL, we would revert that commit and try again, exactly the loop in the diagram.

How to Give and Receive Review Comments

On a pull request you leave comments, and tone decides whether people dread your reviews or learn from them. The single most useful habit is to label severity so the author knows what actually blocks the merge. A blocking comment must be fixed (a bug, a security hole, a missing test). A nit is a small preference the author can take or leave. Prefixing with “nit:” removes half the friction in code review instantly.

The second habit is to ask questions instead of issuing orders. “Why a bare except here?” invites a conversation and sometimes teaches you a reason you missed. “Remove this bare except” starts an argument. Compare these two comments on the exact same line:

• Harsh: “This is wrong, never use bare except.”
• Better: “Blocking: this bare except will hide real errors like a missing key. Could we catch KeyError specifically and log the row? Here is why it bit us last month.”

Receiving reviews is a skill too. Do not take comments personally, the review is about the code, not you. Reply to every comment, even if only with “done” or “good catch.” If you disagree, say why with a reason, and if you are still stuck, move the debate to a quick call rather than a twenty-message thread. Teams often encode who must approve which files in a CODEOWNERS file, so the right expert is always pulled in. At companies like Amazon and Meta, “responds well to review feedback” is something interviewers quietly grade, because it predicts whether you can work on a team at all.

The 12-Point Python Review Checklist

Keep this list next to you when you review a pull request or your own code before you open one. It is the concrete version of everything above. It is a reference, not program output, so read it as the working checklist it is.

1. Naming: do variables and functions say what they hold and do? No t, r, or data2.
2. Functions: does each function do one job and fit on a screen?
3. Error handling: are exceptions specific, never a bare except that swallows everything?
4. Magic numbers: are literals like 5000 named constants with intent explained?
5. Tests: is there a test proving the change works, and did it fail before the fix?
6. Secrets: no passwords, API keys, or tokens hard-coded or committed.
7. Input validation: is untrusted input checked before it is used?
8. Complexity: is there a simpler way, or a nested loop that could be a dict lookup?
9. Duplication: is the same logic copied in three places instead of shared?
10. Dead code: unused imports, variables, or commented-out blocks removed?
11. Docs: does anything surprising have a short comment or docstring?
12. Scope: does the change do one thing, or is it three features smuggled into one pull request?

AI Review Bots: Assistants, Not Replacements

At the time of writing, most teams have an AI reviewer wired into their pull requests, and tools that comment automatically are common. They are genuinely useful for the boring layer: spotting an unused import, a missing type hint, an obvious off-by-one. Treat them exactly like a fast, tireless junior reviewer who has never seen your business. They do not know that revenue must never go negative, that this endpoint is called a million times a day, or that last quarter a silent except cost the company a week of bad reports.

So let the bot clear the easy stuff and free your attention for judgment: is this the right design, does it handle the failure case that matters, will the next person understand it? The checklist above is still yours to run. Tool names will change, and the specific bot your team uses in a year may not exist today, but the human review skill you are building here does not expire.

Common Mistakes

Mistake 1: Refactoring before writing a test

Cleaning code with no safety net is how a “harmless tidy-up” quietly changes a number in a financial report. Always pin the current behavior with a characterization test first, then refactor. If you cannot easily test it, that difficulty is itself telling you the code needs seams (functions, arguments) before it needs cleaning.

Mistake 2: Rewriting instead of reviewing

The urge to delete everything and start over is strong and almost always wrong. A working messy script encodes years of bug fixes and edge cases you cannot see. Improve it in small reviewed steps, keeping it green the whole way, instead of gambling on a big-bang rewrite that reintroduces every old bug.

Mistake 3: Reviewing style while missing the bug

It is easy to leave ten comments about spacing and miss the one line that loses data. Spend your first pass on correctness and safety (does it work, does it handle failure, are there secrets), and only then on naming and style. A linter can enforce style for free, so save your human attention for what a linter cannot see.

Best Practices

• DO read for entry point, data flow, then side effects, in that order, before writing anything
• DO pin behavior with a characterization test before you touch inherited code
• DO label comments as blocking or nit, and ask questions instead of giving orders
• DO keep commits and pull requests small, one idea each, so review is fast and undo is safe
• DON’T use a bare except that hides the very bugs you would want to know about
• DON’T take review comments personally, the feedback is about the code, not about you

Conclusion

You just did what most of a real software job actually looks like: you took a working but messy script you did not write, read it in a deliberate order, pinned its behavior with a test, cleaned it up without changing what it does, and learned how to talk about code with a teammate. That loop, read then pin then improve then discuss, is the same whether the file is 150 lines or 15,000. Master it and inheriting a codebase stops being scary and starts being ordinary.

Next, grab any old script of your own from earlier in this series, write a characterization test for it, and refactor it green. That is the drill that turns this into a habit. And if you want to see everything this series covers, from first steps to AI and machine learning, browse the Python + AI/ML tutorial series home.

Frequently Asked Questions

Interview Questions on Code Review

If you can walk through these without peeking, you are ready for this topic in an interview.

Q: You inherit an undocumented 500-line script and must add a feature. What are your first three steps?

First, read for the entry point, the data flow, and the side effects, so I understand what it does before I change it. Second, write a characterization test that pins the current output, giving me a safety net. Third, make the smallest possible change to add the feature, rerunning the test so I know I did not break existing behavior. Only then do I clean up anything else, in separate commits.

Q: Why write a characterization test instead of just carefully refactoring?

Because careful is not proof. A characterization test captures the exact current behavior and fails the moment my edit changes any output, catching mistakes I would never spot by eye. It lets me refactor aggressively and quickly instead of tiptoeing, and it documents what the code actually did, which is valuable when the code has no other tests or docs.

Q: A teammate leaves you a harsh review comment. How do you respond?

I separate the message from the tone and act on the substance, because the review is about the code, not me. I reply to acknowledge it, fix what is valid, and if I disagree I explain my reasoning with a concrete reason rather than defensiveness. If it keeps going in circles, I suggest a short call. Responding well to feedback is part of being a teammate, and interviewers watch for it.

Q: What is the single most important thing to check in a code review?

Correctness and safety first: does the code do what it claims, does it handle the failure cases that matter, and does it leak any secrets. Style and naming matter, but a linter can enforce those automatically, so I spend my human attention on the bugs and risks a tool cannot see. Missing a data-losing bug because I was busy commenting on spacing is the classic reviewer failure.

Q: How do you keep a refactor from accidentally changing behavior?

I work in small commits, one idea at a time, and rerun the characterization test after each one. If a commit turns the test red, I revert just that commit and try again, so I never accumulate mystery changes. Small, reviewed, tested steps mean that if something breaks, the cause is the last tiny thing I did, not one of fifty edits.

Reference: the complete, always-current details live in the official Python documentation.

Related Posts

Previous: Git Branching and Pull Requests: The GitHub Team Workflow

Next: Python: Modules, Creating, Importing, Organizing Code

Series Home: Python + AI/ML Tutorial Series