Comments — Find the Bug¶

12 snippets where a comment lies and the code does something else. The reader trusts the comment, the compiler trusts the code, and the gap between them ships a real bug. Find the lie, then fix the truth.

Table of Contents¶

1. How to Use
2. The Core Failure Mode
3. Snippet 1 — "Returns seconds" but returns milliseconds (Go)
4. Snippet 2 — "Thread-safe" on code that isn't (Java)
5. Snippet 3 — Stale @param after a signature change (Java)
6. Snippet 4 — Commented-out validation that was load-bearing (Python)
7. Snippet 5 — // TODO: handle null that NPEs (Java)
8. Snippet 6 — Comment hides an off-by-one (Python)
9. Snippet 7 — Doc promises an exception no longer thrown (Java)
10. Snippet 8 — "Inclusive" range comment, exclusive code (Go)
11. Snippet 9 — "Idempotent" comment on a non-idempotent handler (Python)
12. Snippet 10 — Comment claims a unit the constant contradicts (Go)
13. Snippet 11 — "Already validated upstream" lie (Java)
14. Snippet 12 — "Sorted ascending" docstring, descending code (Python)
15. Snippet 13 — "Caller owns the lock" comment nobody reads (Go)
16. Scorecard
17. Related Topics

How to Use¶

Each snippet shows code where a comment makes a promise the code breaks. Read the comment first — exactly as a tired engineer at 2 a.m. would: trusting it. Then read the code. The bug lives in the gap.

1. Read the comment. Note what it promises (a unit, a guarantee, a contract).
2. Read the code. Decide what it actually does.
3. Predict what breaks when a caller trusts the comment instead of the code.
4. Open the answer. Confirm the comment-vs-reality mismatch and the fix.

The fix is rarely "edit the comment to match." A lying comment usually marks a place where the contract itself drifted. The right fix makes the code enforce the contract (types, asserts, tests) so a comment can never lie again — or deletes the comment because the code now says it.

The Core Failure Mode¶

A comment is unverified prose sitting next to verified code. The compiler reads the code; the human reads the comment. When they disagree, the human is wrong — but the human is the one writing the next caller. Three ways the lie does damage:

• Wrong unit / shape. "Returns seconds" leads the caller to multiply by 1000 again.
• False guarantee. "Thread-safe" leads the caller to skip the lock; data races.
• Phantom contract. "Throws on invalid input" leads the caller to skip the null check; NPE.

Every snippet below is one of these three. The tell is always: the comment is a claim no test backs up.

Snippet 1 — "Returns seconds" but returns milliseconds (Go)¶

Difficulty: Easy

// SessionAge returns the age of the session in seconds.
func (s *Session) SessionAge() int64 {
    return time.Since(s.CreatedAt).Milliseconds()
}

// Caller: expire sessions older than 30 minutes.
const maxSessionSeconds = 30 * 60 // 1800

func (s *Session) IsExpired() bool {
    return s.SessionAge() > maxSessionSeconds
}

What's wrong?

func (s *Session) Age() time.Duration {
    return time.Since(s.CreatedAt)
}

const maxSessionAge = 30 * time.Minute

func (s *Session) IsExpired() bool {
    return s.Age() > maxSessionAge
}

Snippet 2 — "Thread-safe" on code that isn't (Java)¶

Difficulty: Medium

/**
 * A thread-safe hit counter.
 * All methods may be called concurrently from any number of threads.
 */
public class HitCounter {
    private final Map<String, Long> hits = new HashMap<>();

    public void record(String key) {
        hits.merge(key, 1L, Long::sum);
    }

    public long count(String key) {
        return hits.getOrDefault(key, 0L);
    }
}

What's wrong?

/** A thread-safe hit counter. All methods may be called concurrently. */
public class HitCounter {
    private final ConcurrentHashMap<String, LongAdder> hits = new ConcurrentHashMap<>();

    public void record(String key) {
        hits.computeIfAbsent(key, k -> new LongAdder()).increment();
    }

    public long count(String key) {
        LongAdder adder = hits.get(key);
        return adder == null ? 0L : adder.sum();
    }
}

Snippet 3 — Stale @param after a signature change (Java)¶

Difficulty: Medium

/**
 * Charges a card.
 *
 * @param amountCents the amount to charge, in cents
 * @param currency    ISO-4217 currency code
 * @return the gateway transaction id
 */
public String charge(BigDecimal amountDollars, String currency) {
    // gateway API wants cents as an integer
    long cents = amountDollars.movePointRight(2).longValueExact();
    return gateway.submit(cents, currency);
}

// Caller, written from the Javadoc hover:
String txn = payments.charge(new BigDecimal("1999"), "USD"); // "amount in cents"

What's wrong?

/**
 * Charges a card.
 *
 * @param amount the amount to charge (a Money value carrying its own currency and scale)
 * @return the gateway transaction id
 */
public String charge(Money amount) {
    return gateway.submit(amount.toMinorUnits(), amount.currencyCode());
}

// Caller — no unit to guess:
String txn = payments.charge(Money.of("19.99", "USD"));

Snippet 4 — Commented-out validation that was load-bearing (Python)¶

Difficulty: Easy

def transfer(account, amount):
    # Temporarily disabled for the load test on 04-12, re-enable after:
    # if amount <= 0:
    #     raise ValueError("amount must be positive")
    # if amount > account.balance:
    #     raise InsufficientFunds(account.id, amount)

    account.balance -= amount
    ledger.record(account.id, -amount)
    return account.balance

What's wrong?

def transfer(account, amount):
    if amount <= 0:
        raise ValueError("amount must be positive")
    if amount > account.balance:
        raise InsufficientFunds(account.id, amount)

    account.balance -= amount
    ledger.record(account.id, -amount)
    return account.balance

Snippet 5 — // TODO: handle null that NPEs (Java)¶

Difficulty: Easy

public String formatGreeting(User user) {
    // TODO: handle null user (e.g. anonymous sessions) before launch
    return "Welcome back, " + user.getDisplayName() + "!";
}

// In the new anonymous-checkout flow:
String banner = formatGreeting(session.getUser()); // getUser() returns null for guests

What's wrong?

public String formatGreeting(User user) {
    if (user == null) {
        return "Welcome!";
    }
    return "Welcome back, " + user.getDisplayName() + "!";
}

@Test
void greetsAnonymousUsers() {
    assertEquals("Welcome!", formatGreeting(null));
}

Snippet 6 — Comment hides an off-by-one (Python)¶

Difficulty: Medium

def last_n_days(events, n):
    """Return the most recent n calendar days of events, including today."""
    today = date.today()
    cutoff = today - timedelta(days=n)
    # keep events strictly after the cutoff -> exactly n days including today
    return [e for e in events if e.date > cutoff]

What's wrong?

def last_n_days(events, n):
    """Return events from the last n calendar days, including today."""
    today = date.today()
    cutoff = today - timedelta(days=n - 1)   # n days inclusive of today
    return [e for e in events if cutoff <= e.date <= today]

def test_n_equals_1_is_just_today():
    today = date.today()
    events = [E(today), E(today - timedelta(days=1)), E(today + timedelta(days=1))]
    assert last_n_days(events, 1) == [E(today)]

Snippet 7 — Doc promises an exception no longer thrown (Java)¶

Difficulty: Medium

/**
 * Looks up a product by SKU.
 *
 * @param sku the product SKU
 * @return the product
 * @throws ProductNotFoundException if no product matches the SKU
 */
public Product findBySku(String sku) {
    return cache.getOrDefault(sku, null); // fast path, added in the cache refactor
}

// Caller relies on the documented exception for control flow:
public Money priceFor(String sku) {
    try {
        return findBySku(sku).getPrice();
    } catch (ProductNotFoundException e) {
        return Money.ZERO; // unknown SKU -> treat as "not for sale"
    }
}

What's wrong?

/**
 * Looks up a product by SKU.
 *
 * @return the product, or empty if no product matches
 */
public Optional<Product> findBySku(String sku) {
    return Optional.ofNullable(cache.get(sku));
}

public Money priceFor(String sku) {
    return findBySku(sku)
        .map(Product::getPrice)
        .orElse(Money.ZERO);
}

Snippet 8 — "Inclusive" range comment, exclusive code (Go)¶

Difficulty: Hard

// PageItems returns the items on the given 1-based page.
// The returned slice covers indices [start, end] INCLUSIVE on both ends.
func PageItems(items []Item, page, size int) []Item {
    start := (page - 1) * size
    end := start + size // half-open: items[start:end] excludes index `end`
    if end > len(items) {
        end = len(items)
    }
    return items[start:end]
}

// Caller computes how many items it just received, trusting "inclusive":
func ReceivedCount(page []Item) int {
    // inclusive range of length (end - start + 1)... but page came from a slice
    return len(page) + 1
}

What's wrong?

// PageItems returns the items on the given 1-based page (up to `size` items).
func PageItems(items []Item, page, size int) []Item {
    start := (page - 1) * size
    if start > len(items) {
        return nil
    }
    end := min(start+size, len(items))
    return items[start:end]
}

// Caller: the slice already knows its length.
func ReceivedCount(page []Item) int {
    return len(page)
}

Snippet 9 — "Idempotent" comment on a non-idempotent handler (Python)¶

Difficulty: Hard

class PaymentWebhook:
    """
    Handles 'payment.succeeded' webhooks.

    This handler is idempotent: the payment provider may deliver the same
    event multiple times, and re-processing it is safe and has no side effects.
    """

    def handle(self, event):
        order = self.orders.get(event["order_id"])
        order.status = "PAID"
        self.wallet.credit(order.merchant_id, event["amount"])   # add funds
        self.emailer.send_receipt(order.customer_email, order)   # email customer
        return {"ok": True}

What's wrong?

class PaymentWebhook:
    """Handles 'payment.succeeded' webhooks. Idempotent: duplicate event ids are ignored."""

    def handle(self, event):
        event_id = event["id"]
        if not self.processed_events.add_if_absent(event_id):   # atomic claim
            return {"ok": True, "duplicate": True}

        order = self.orders.get(event["order_id"])
        order.status = "PAID"
        self.wallet.credit(order.merchant_id, event["amount"])
        self.emailer.send_receipt(order.customer_email, order)
        return {"ok": True}

Snippet 10 — Comment claims a unit the constant contradicts (Go)¶

Difficulty: Easy

// rateLimitWindow is the sliding-window length in seconds.
const rateLimitWindow = 60 * time.Second

func (l *Limiter) Allow(key string) bool {
    // reject if more than maxHits requests in the last `rateLimitWindow` seconds
    count := l.store.CountSince(key, time.Now().Add(-rateLimitWindow))
    return count < l.maxHits
}

// Elsewhere, a metrics exporter reads the "seconds" the comment promised:
func (l *Limiter) WindowSeconds() int64 {
    return int64(rateLimitWindow) // "it's in seconds per the comment"
}

What's wrong?

const rateLimitWindow = 60 * time.Second // a Duration; do not read its raw integer

func (l *Limiter) WindowSeconds() int64 {
    return int64(rateLimitWindow.Seconds())
}

Snippet 11 — "Already validated upstream" lie (Java)¶

Difficulty: Medium

public class CouponService {

    public Discount apply(String couponCode, Cart cart) {
        // couponCode is already validated and trimmed by the controller layer,
        // so we can use it directly without re-checking.
        Coupon coupon = coupons.findByCode(couponCode);
        return new Discount(coupon.getPercentOff(), cart.subtotal());
    }
}

// A new gRPC entry point added later calls the service directly:
public DiscountResponse rpcApply(DiscountRequest req) {
    Discount d = couponService.apply(req.getCode(), buildCart(req));
    return toResponse(d);
}

What's wrong?

public Discount apply(String couponCode, Cart cart) {
    String code = Objects.requireNonNull(couponCode, "couponCode").trim();
    if (code.isEmpty()) {
        throw new IllegalArgumentException("couponCode is blank");
    }
    Coupon coupon = coupons.findByCode(code)
        .orElseThrow(() -> new CouponNotFoundException(code));
    return new Discount(coupon.getPercentOff(), cart.subtotal());
}

Snippet 12 — "Sorted ascending" docstring, descending code (Python)¶

Difficulty: Medium

def top_scores(scores):
    """
    Return scores sorted in ascending order (lowest first).
    """
    return sorted(scores, reverse=True)


def median(scores):
    sorted_scores = top_scores(scores)
    n = len(sorted_scores)
    # the list is ascending, so the lower half is the front
    lower_half = sorted_scores[: n // 2]
    return lower_half[-1] if lower_half else None

What's wrong?

def scores_high_to_low(scores):
    """Return scores sorted in descending order (highest first)."""
    return sorted(scores, reverse=True)


def median(scores):
    ordered = sorted(scores)           # ascending, locally and unambiguously
    n = len(ordered)
    if n == 0:
        return None
    mid = n // 2
    return ordered[mid] if n % 2 else (ordered[mid - 1] + ordered[mid]) / 2

Snippet 13 — "Caller owns the lock" comment nobody reads (Go)¶

Difficulty: Hard

type Cache struct {
    mu      sync.Mutex
    entries map[string]string
}

// evict removes the oldest entry.
// NOTE: caller must hold c.mu before calling evict.
func (c *Cache) evict() {
    for k := range c.entries {
        delete(c.entries, k)
        return
    }
}

func (c *Cache) Set(key, val string) {
    c.mu.Lock()
    defer c.mu.Unlock()
    if len(c.entries) >= maxEntries {
        c.evict()
    }
    c.entries[key] = val
}

// Added later by someone who read evict's doc as "evict handles its own safety":
func (c *Cache) Trim() {
    for len(c.entries) > softLimit {
        c.evict() // no lock held here
    }
}

What's wrong?

// evictLocked removes the oldest entry. The "Locked" suffix encodes the precondition
// in the name, and it is unexported so only same-package, lock-holding code can call it.
func (c *Cache) evictLocked() {
    for k := range c.entries {
        delete(c.entries, k)
        return
    }
}

func (c *Cache) Set(key, val string) {
    c.mu.Lock()
    defer c.mu.Unlock()
    if len(c.entries) >= maxEntries {
        c.evictLocked()
    }
    c.entries[key] = val
}

func (c *Cache) Trim() {
    c.mu.Lock()
    defer c.mu.Unlock()
    for len(c.entries) > softLimit {
        c.evictLocked()
    }
}

Scorecard¶

Tally one point per snippet where you identified the comment-vs-reality mismatch and named the real-world failure (wrong charge, double credit, crash, skewed metric) before opening the answer.

The deeper lesson across all 13: a comment is the weakest place to store a contract. Every fix moved the guarantee somewhere a machine can check it — a type (time.Duration, Money, Optional), a name (evictLocked, AgeMillis, scores_high_to_low), a guard, or a test. When you find a lying comment, don't just correct the prose; ask why the contract was ever entrusted to prose, and move it where it can't lie.

Related Topics¶

• junior.md — what makes a comment good vs. noise; the categories of comment to avoid.
• tasks.md — hands-on exercises: delete redundant comments, rewrite lying ones, promote contracts into code.
• Comments chapter README — the positive rules this find-bug set inverts.
• Anti-Patterns — broader catalog of code-level smells, including comment smells in context.
• Refactoring — Extract Method, Rename, and Introduce Parameter Object: the moves that turn a lying comment into self-documenting code.