Skip to main content
the readable codebase

The Review Checklist for Cognitive Load

6 min read Chapter 24 of 27

The Review Checklist for Cognitive Load

The Smell

The logistics platform team reviewed 340 pull requests in the last quarter. Of those, 312 were approved with no comments. Of the 28 that received comments, 22 comments were about formatting, naming conventions, or missing tests. Six comments were about design. The team has code review. It does not have design review.

The approval rate is the symptom. A 92% no-comment approval rate means reviewers are either skipping the review, reviewing only for compilation and tests, or avoiding the discomfort of design feedback. Each silent approval is a vote for the status quo. The codebase degrades at the rate of silent approvals.

The Cognitive Cost

A review that catches only what automation catches is redundant. The reviewer’s time is wasted. The review process becomes a ritual. Developers learn that review is a gate to pass, not a conversation to value.

A review that catches design problems is rare and valuable. It catches the method that will need a comment in three months. It catches the dependency that will create a cycle next quarter. It catches the abstraction that adds a layer of indirection nobody needs. These catches prevent cognitive load from accumulating. They are worth the time. They are worth the friction.

The Before

Review anti-patterns that make reviews ineffective:

// ANTI-PATTERN: Approval to avoid conflict
// Name: "The Rubber Stamp"
// The reviewer scans the diff for 30 seconds, sees nothing obviously broken,
// and approves. They do not read the code carefully enough to notice
// that a new method was added to a God class.
// Cost: Every rubber stamp trains the team that review is a formality.

// ANTI-PATTERN: Nitpicking that misses the design issue
// Name: "The Comma Police"
// The reviewer leaves twelve comments about variable naming, spacing,
// and import ordering. They do not notice that the pull request introduces
// a circular dependency between two packages.
// Cost: The author feels reviewed (twelve comments!) but the real problem ships.

// ANTI-PATTERN: The rewrite review
// Name: "The Shadow Author"
// The reviewer disagrees with the approach and rewrites the entire solution
// in the comments. The review becomes a design argument. Three rounds of
// comments later, the original author gives up and implements the reviewer's
// solution without understanding it.
// Cost: The author learns nothing. The reviewer becomes a bottleneck.

// ANTI-PATTERN: The delayed perfection review
// Name: "The Perfect is the Enemy"
// The reviewer blocks the pull request for days, requesting increasingly
// minor improvements. The author has moved on to other work.
// Context switching to return to the PR is expensive.
// Cost: The team learns to avoid this reviewer's queue.

The Fix

The cognitive load review checklist. Use this for every Java pull request:

1. Naming check (30 seconds) For each new or renamed method: can you predict what it does without reading the body? For each new variable: can you predict what it holds without reading the assignment? If a name requires reading the implementation to understand, flag it.

Comment template: “I can’t predict what processItems does from the name alone. What specific processing does this method do? allocateWarehouseSlots or validateOrderLines would help the next reader.”

2. Responsibility check (60 seconds) Does this change add a new method to a class that already has more than 7 public methods? Does it add a new dependency to a class that already has more than 5? Does it add a new field to a data bag class?

Comment template: ShipmentService has 47 methods. This PR adds a 48th. Can handleCustomsClearance be a separate CustomsClearanceService? It shares no dependencies with the other 47 methods.”

3. Boundary check (60 seconds) Does this change import a class from a package that should not be directly depended on? Does it make a class public that should be package-private? Does it create a new dependency path between packages that should be independent?

Comment template: “This imports CarrierConnectionPool from the carrier package. That class is an internal implementation detail. Can you use CarrierGateway instead, which is the carrier package’s public API?”

4. Abstraction check (60 seconds) Does this change add a new abstraction (interface, abstract class, wrapper)? If so, does the abstraction have more than one implementation? If it has exactly one implementation, the abstraction adds indirection without value.

Comment template: “The new ShipmentProcessor interface has one implementation: DefaultShipmentProcessor. What is the second implementation? If there is no plan for one, the interface adds a layer of indirection the next reader must trace through without gaining flexibility.”

5. Complexity hotspot check (30 seconds) Does this change modify a method whose SonarQube cognitive complexity is already above 15? Does the change increase the score? If a high-complexity method is being modified, the PR should reduce its complexity, not increase it.

Comment template: calculateRate already has a cognitive complexity of 22. This change adds another nested conditional, pushing it to 26. Can the new logic be extracted to a separate method?”

Total checklist time: 4 minutes per pull request. Four minutes to prevent the design debt that takes four hours to fix later.

// USEFUL: This review comment targets cognitive load.
// It identifies the specific burden and suggests a concrete fix.

// Review comment on a PR that adds a method to ShipmentService:
// "This method takes a ShipmentContext (15 fields) but uses only
//  origin, destination, and items. A method signature like
//  createCustomsDeclaration(ShipmentRoute route, ShipmentCargo cargo)
//  tells the next reader exactly what this method needs. The Context
//  parameter hides the actual dependency surface."

// USEFUL: This review comment targets design trajectory.
// It identifies where the codebase is heading, not just where it is.

// Review comment on a PR that adds a third boolean parameter:
// "This is the third boolean parameter on createShipment. Three booleans
//  create 8 execution paths. Chapter reference aside, can these be a
//  ShipmentOptions sealed interface? The call site would read
//  ShipmentOptions.hazardousExpress() instead of (true, false, true)."

The Rule

Spend the first 4 minutes of every review on the 5-point cognitive load checklist: naming, responsibility, boundaries, abstractions, and complexity hotspots. Leave at most one comment per concern. If a PR has no design issues, approve without comments. If it has design issues, block until the issues are addressed. The goal of a review is not to find something to comment on. The goal is to prevent the codebase from becoming harder to read after the merge than it was before. If the code is fine, say so. If the code ships a design problem, say so and block. There is no middle ground where you leave the comment and approve anyway. That middle ground is how standards erode.