Review for Readability — Code Reviews That Help, Part 6

Code Reviews That Help · Part 6

Code is read far more often than it's written, usually by someone with no memory of why it's shaped the way it is — and that someone is often you, in six months. Reviewing for readability is reviewing for that future reader. This walks through what to look for — naming, nesting, the why-comment — and, just as important, when readability isn't worth blocking a merge over.

Code is read 10x more than written. A reviewer who optimizes for the reader does the team a long-term favor.

Step 1: The Reader Test (5 min)

When reading code, ask: "Could someone new to the team understand this in 6 months?"

If no: feedback opportunity. Not necessarily a blocker — but a signal.

You're not nitpicking style. You're protecting future maintainability.

Step 2: Naming (15 min)

Bad names tax every future reader.

✗ data
✗ x, y, z
✗ tmp
✗ result
✗ handleData
✗ doStuff

✓ activeUsers
✓ pendingOrders
✓ cachedBalance
✓ filterExpiredSessions

A function called process(data) is meaningless. validateEmailAddress(input) reads itself.

Exception: short names for short scope. for i in range(10) is fine.

Step 3: Function Length (10 min)

A 200-line function:

• Holds too much in your head

• Mixes concerns

• Hard to test

Rule of thumb: under 30 lines.

Long function = refactoring opportunity. Comment:

"Consider splitting this into validateInput, computeResult, and notifyCaller."

Not blocking unless the function is a maintenance nightmare. Suggestion.

Step 4: Comment the Why, Not the What (10 min)

# Bad
i = i + 1   # increment i

# Good
i = i + 1   # skip the first row (header)

Code shows what. Comments add why.

If you find comments explaining what — the code probably needs renaming.

Step 5: Avoid Nesting (10 min)

# Bad
def process(items):
    if items:
        if items[0].is_valid:
            if items[0].user:
                if items[0].user.active:
                    return items[0].user.id
    return None

# Better — early returns
def process(items):
    if not items:
        return None
    item = items[0]
    if not item.is_valid:
        return None
    if not item.user or not item.user.active:
        return None
    return item.user.id

Deep nesting = mental load. Early returns flatten.

Step 6: Single Concern (10 min)

# Bad — many things at once
def save_user(user_input):
    parsed = json.loads(user_input)
    if not parsed.get("email"):
        raise ValueError()
    user = User(name=parsed["name"], email=parsed["email"])
    db.save(user)
    send_email(user, "welcome")
    log(f"Saved: {user.id}")
    return user.id

# Better
def save_user(user_input):
    parsed = parse(user_input)
    validate(parsed)
    user = create_user(parsed)
    notify_new_user(user)
    return user.id

Each function does one named thing.

Step 7: Magic Numbers (5 min)

# Bad
if user.attempts > 5:
    lockout(user)

# Better
MAX_LOGIN_ATTEMPTS = 5

if user.attempts > MAX_LOGIN_ATTEMPTS:
    lockout(user)

Named constants explain intent. Easier to change.

Exception: 0, 1, -1 are fine.

Step 8: Consistency (5 min)

In a file:

• Mixed naming styles (getUserById and fetch_user(id))

• Mixed string types (single quotes here, double there)

• Mixed import styles

Reviewer flags: "let's be consistent with the rest of the file."

Linters catch most. Humans catch the rest.

Step 9: Test Readability (10 min)

Tests are documentation:

# Bad
def test_user():
    assert do_thing(...) == True

# Better
def test_user_with_expired_token_returns_unauthorized():
    response = client.get("/me", headers={"Authorization": "Bearer expired"})
    assert response.status_code == 401
    assert response.json()["error"] == "token_expired"

Name tells the story. Body confirms it.

Step 10: Don't Block on Readability Alone (5 min)

Code can be ugly and correct.

Reviewer judgment:

• Correctness issue → block

• Major readability issue with future maintenance impact → strong suggestion

• Minor style issue → comment for the author's discretion

The goal isn't perfect code. It's code that works and future engineers can understand.

What You Just Did

Readability review: reader test, naming, length, comments, nesting, single concern, magic numbers, consistency, test names, judgment on blocking. Long-term maintainability.

Common Failure Modes

Block on every readability nit. PR never merges.

Skip readability; only correctness. Codebase rots.

"What" comments. Noise; rot when code changes.

Mixed style in same file. Cognitive overhead per line.

Inconsistent naming. Reader can't predict next pattern.

---

Continue the Code Reviews That Help path

• Previous — Part 5: Spot Security Issues in Review

• Next — Part 7: Build Review Conventions

Part of the Code Reviews That Help learning path.