Code Review Workshop

Review three provided code samples and write review comments for each. The goal is fluency in the rubric (correctness, clarity, design, tests) and in the comment style (specific, constructive, evidence-based).

Retrieval Prompts

1. Name the four rubric buckets.
2. Give the shape of a good review comment.
3. What is the difference between a nit and a substantive comment?

Sample 1: The Quiet Bug

# src/checkout/totals.py
def compute_total(items, discount_pct=0):
    subtotal = 0
    for item in items:
        subtotal += item["price"] * item["qty"]
    if discount_pct:
        subtotal = subtotal - (subtotal * discount_pct / 100)
    tax = subtotal * 0.08
    return subtotal + tax

Write at least five review comments. Include at least:

• one [correctness, substantive]
• one [clarity, nit]
• one [design, substantive]
• one [tests, substantive]
• one comment that labels itself explicitly as non-blocking

Hints (don't read unless stuck): integer math and negative discounts; magic number 0.08; is discount applied before or after tax; no tests at all.

Sample 2: The Cathedral of Patterns

public interface UserFetcher { User fetch(long id); }

public class DefaultUserFetcher implements UserFetcher {
    private final UserFetcherStrategy strategy;
    private final UserFetcherStrategyFactory factory;
    public DefaultUserFetcher(UserFetcherStrategyFactory factory) {
        this.factory = factory;
        this.strategy = factory.createDefault();
    }
    public User fetch(long id) {
        return strategy.fetch(id);
    }
}

public class UserFetcherStrategyFactory {
    public UserFetcherStrategy createDefault() {
        return new DatabaseUserFetcherStrategy();
    }
}

public class DatabaseUserFetcherStrategy implements UserFetcherStrategy {
    public User fetch(long id) {
        return Database.getUser(id);
    }
}

There is one implementation of UserFetcherStrategy. There are no callers doing anything besides fetcher.fetch(id).

Write review comments covering:

• which patterns are not pulling their weight (and which specific rule of simple design they violate)
• what the author should have done instead
• what you would merge and what would block

Sample 3: The Almost-Clean Feature

// src/promotions/apply.ts
export async function applyPromoCode(cartId: string, code: string) {
  const cart = await db.carts.get(cartId);
  const promo = await db.promotions.get(code);
  if (!promo) return { ok: false, error: "not_found" };
  if (promo.expiresAt < Date.now()) return { ok: false, error: "expired" };
  if (promo.uses >= promo.maxUses) return { ok: false, error: "exhausted" };
  const discount = promo.kind === "pct"
    ? cart.total * (promo.amount / 100)
    : promo.amount;
  promo.uses++;
  await db.promotions.put(promo);
  return { ok: true, newTotal: cart.total - discount, auditId: uuid() };
}

Write comments that cover:

• at least one concurrency bug (hint: race between two redemptions)
• at least one testability issue (Date.now(), uuid())
• at least one design comment about where this logic belongs
• at least one missing-test comment

Receiving Feedback Drill

For two of your own review comments, write the author's reply using the 5-step pattern (acknowledge, address, agree/modify/push-back, ask, move forward). Make at least one reply disagree constructively with your own comment.

Evidence Check

This page is complete when:

• You produced at least 12 review comments across the three samples.
• Each comment is labeled by bucket and nit/substantive.
• At least 3 comments cite concrete evidence (test, fixture, incident, link to a concept page).
• You practiced one disagreement reply.