Reading code, and reviewing it without being unbearable

Code review is the part of the job nobody teaches and everybody does. It is also, for a junior engineer, the single fastest way to get better — both directions of it.

Reading code you did not write

You will join a team with a codebase of a hundred thousand lines and be asked to fix something in week one. Nobody will explain all of it, because nobody can.

What actually works:

Start from a behaviour, not from the top. Do not read the codebase. Pick something it does — "what happens when a delivery is recorded?" — and follow it. Depth-first, one path at a time.

Find the entry point. main, a controller method, a test. In the capstone it is Main.record. Read it, note what it calls, go one level down. Stop when you have your answer, not when you have read everything.

Use the debugger as a reading tool. Put a breakpoint at the entry, run a real case, and step. Ten minutes of stepping beats an hour of guessing, and the call stack tells you the structure faster than any diagram.

Read the tests first when you can. A test says what the code is for, in the author's own words, with a worked example. SubscriberTest tells you the pincode rules faster than Subscriber does.

git log on a confusing file. "Why is this here?" is often answered by the commit that added it — which is a strong argument for writing commit messages that explain why.

Resist rewriting on sight. Code you do not understand looks worse than it is. Assume there is a reason, and go looking for it. Sometimes there is not, and by then you can say so with evidence.

Reviewing somebody else's change

The point of review is finding problems while they are cheap, and spreading knowledge of the codebase. It is not proving you are clever.

What to look for, roughly in order of value:

Does it do what it says? Read the description, then the diff. A change doing more than it claims is the commonest real problem, and the easiest to miss.

What happens when it fails? The happy path is usually fine. Ask about the empty list, the null, the network timeout, the two users at once.

Is it tested, and does the test actually assert something? A test calling a method with no assertion is coverage, not a test.

Would you be able to debug this at 2am? Names, error messages, log lines.

Is there a simpler version? Ask, do not assert — there is often a reason.

What not to spend review on: formatting, import order, brace style. A formatter and a linter settle those, and arguing about them in review is how review gets a reputation as an obstacle.

How to word it

The difference between a review people act on and one they resent is almost entirely tone.

"This is wrong."
"Why didn't you use a map here?"
"I would never write it this way."

versus

"If `subscribers` is empty, does this return 0 or throw? I think the report
would show a total of zero, which might read as 'nothing sold'."

"A `Map<Plan, Pricing>` here would mean adding a plan touches one line — worth
it, or is three plans not enough to bother?"

"Nit: `d` is fine in the loop, but `delivery` in the field above would help."

Three habits that do most of the work:

Ask rather than assert. You are frequently missing context. A question gets the reason; a statement gets defensiveness.

Mark the small stuff as small. Prefixing "Nit:" tells the author it is optional and separates it from the thing that matters.

Say what is good. Genuinely — "this test is a much better example than the one I wrote last week". Review that is only ever criticism is review people start avoiding.

And the most important: review the code, not the person. "This method does two things" rather than "you did two things in this method". It reads as a smaller difference than it is.

Taking a review

Harder than giving one, especially early on.

It is not about you. A comment on your code is a comment on your code. Every engineer you admire has had their work pulled apart in review; that is where they learned it.

Assume good faith even when the wording is blunt. Plenty of good engineers write terse reviews. Terse is not hostile.

Answer every comment, even if only "done" or "good catch". Silence reads as disagreement.

Disagree when you are right. "I tried that and it deadlocked under load, which is why the lock is there" is a complete answer, and a reviewer who gets it learns something. Reviewers are not always right, and a junior who never pushes back is not learning to think.

Ask when you do not understand. "I do not know what you mean by idempotent here" costs you nothing and is how the vocabulary in this course got into your head.

What makes a change reviewable

Half of a good review is the author's doing.

Small. A 60-line diff gets real review. A 2,000-line diff gets "looks good to me", which is not review at all — it is a rubber stamp with your name on it.

One thing. Do not mix a refactor with a behaviour change. The reviewer cannot tell which lines changed what, and neither can git blame later.

A description that says why. What changed is in the diff. Why it changed is not, and that is the thing a reviewer needs to judge whether the approach is right.

Already reviewed by you. Read your own diff before sending it. You will find the debug print, the commented-out block and the badly named variable yourself — and every one of those you catch buys attention for the things only another person can see.

What this is really for

An intern joining a team is often surprised that review is where most of the learning happens. Not the tutorials — the fifteen comments on your first pull request, each one a piece of context that nobody would have thought to tell you otherwise.

So: send small changes early, ask why rather than defending, and review other people's work even when nobody asked you to. Reading other people's code attentively is the fastest way to get good at writing your own.

Check your work

How to read an unfamiliar codebase: start from a behaviour, follow one path depth-first, and stop when you have the answer.

Why the debugger is a reading tool: the call stack shows structure faster than any diagram.

Why read tests first: they say what the code is for, with worked examples.

What review is for: finding problems while they are cheap, and spreading knowledge.

The order to look in: does it do what it says, what happens when it fails, is it really tested, could you debug it at 2am, is there a simpler version.

What not to review: formatting. A tool settles it.

The three tone habits: ask rather than assert, mark nits as nits, say what is good.

The framing that matters: review the code, not the person.

How to take a review: answer everything, disagree when you are right, ask when you do not understand.

What makes a change reviewable: small, one thing, a description saying why, and read by you first.

Why a 2,000-line diff is not reviewed: "looks good to me" is a rubber stamp, not a review.

Practice

1. Open the capstone and trace what happens for report 2026-09, from Main to the output. Write the call chain down.
2. Do the same with a debugger and a breakpoint. Compare how long each took.
3. Read SubscriberTest before Subscriber. Write down the rules you learned.
4. Find a confusing line in any project and run git log -p on that file until you find the commit that added it.
5. Review a pull request in an open-source Java project. Write three comments you would leave — then check them against the tone habits above.
6. Rewrite this as something you would send: "this is wrong, use a map".
7. Take your own last commit and review it as a stranger. List what you find.
8. Split a change you have made into two commits — one refactor, one behaviour.
9. Write a commit message explaining why rather than what.
10. Ask somebody to review something of yours, and answer every comment even if only to agree.

Next: the capstone — building the whole thing to the standard these two modules just set.