Comments — Practice Tasks¶

Twelve "fix the comments" exercises. Every comment you keep should pay rent: it must say something the code cannot. Each task gives you a scenario, a snippet whose comments are pulling their weight badly, and an instruction. Work the fix in your head (or your editor) before opening the solution. Languages rotate between Go, Java, and Python on purpose — comment smells look the same in every syntax.

Table of Contents¶

1. Task 1 — Delete the redundant comment by improving the name (Java)
2. Task 2 — Remove commented-out code (Python)
3. Task 3 — Replace a journal/attribution comment with VCS (Go)
4. Task 4 — Delete the closing-brace comment by extracting a function (Java)
5. Task 5 — Convert a WHAT comment into a WHY comment (Go)
6. Task 6 — Fix the misleading, outdated comment that contradicts the code (Python)
7. Task 7 — Write a proper doc-comment for a public API (Go)
8. Task 8 — Turn an opaque comment into a clarifying one with a link (Java)
9. Task 9 — Replace a section-banner comment with extracted functions (Python)
10. Task 10 — Promote intent: the comment apologizes for the code (Java)
11. Task 11 — Encode an invariant the comment merely asserts (Go)
12. Task 12 — Comment audit (Python — open-ended)

How to Use¶

• Read the snippet and decide what each comment is for before you touch it. Most comments fall into one of three buckets: redundant (says what the code already says), misleading (says something the code does not do), or load-bearing (says something the code genuinely cannot — a reason, a warning, a link).
• Try the fix yourself, then expand the <details> block. Compare your reasoning to the solution's, not just your diff.
• The default move in this chapter is delete or replace, not "write a better comment." A comment you can erase by renaming a variable or extracting a function is a comment that was never worth maintaining.
• Difficulty climbs from Easy (mechanical deletions) to Hard (audits and invariants encoded in types). Do them in order the first time.

The decision you are practicing on every task:

Task 1 — Delete the redundant comment by improving the name (Java)¶

Difficulty: Easy

Scenario: A teammate sprinkled "helpful" comments above lines that already explain themselves. Each comment is a second copy of the code in English — and a second thing to keep in sync.

public class CartPricing {
    public BigDecimal total(List<Item> items) {
        // the running total
        BigDecimal t = BigDecimal.ZERO;
        // loop over every item
        for (Item i : items) {
            // multiply price by quantity
            BigDecimal lt = i.getPrice().multiply(BigDecimal.valueOf(i.getQty()));
            // add to the running total
            t = t.add(lt);
        }
        // return the total
        return t;
    }
}

Instruction: Remove every comment. The code must remain at least as readable — achieve that by improving the names, not by adding new comments.

public class CartPricing {
    public BigDecimal total(List<Item> items) {
        BigDecimal runningTotal = BigDecimal.ZERO;
        for (Item item : items) {
            BigDecimal lineTotal = item.getPrice().multiply(BigDecimal.valueOf(item.getQty()));
            runningTotal = runningTotal.add(lineTotal);
        }
        return runningTotal;
    }
}

Task 2 — Remove commented-out code (Python)¶

Difficulty: Easy

Scenario: During a refactor someone "kept the old version just in case." Three months later nobody knows whether the dead block is the safe fallback or a known-bad attempt.

def normalize_phone(raw: str) -> str:
    # old implementation — keep for reference
    # digits = "".join(c for c in raw if c.isdigit())
    # if len(digits) == 10:
    #     return "+1" + digits
    # return digits

    digits = "".join(ch for ch in raw if ch.isdigit())
    if len(digits) == 10:
        return "+1" + digits
    if len(digits) == 11 and digits.startswith("1"):
        return "+" + digits
    raise ValueError(f"cannot normalize phone: {raw!r}")

Instruction: Decide what to do with the commented-out block and apply it. Justify the call.

def normalize_phone(raw: str) -> str:
    digits = "".join(ch for ch in raw if ch.isdigit())
    if len(digits) == 10:
        return "+1" + digits
    if len(digits) == 11 and digits.startswith("1"):
        return "+" + digits
    raise ValueError(f"cannot normalize phone: {raw!r}")

Task 3 — Replace a journal/attribution comment with VCS (Go)¶

Difficulty: Easy

Scenario: This file has grown a changelog at the top and an "added by" tag on a function. The code is being maintained by hand-rolling what git already records perfectly.

// Package invoicing computes invoice totals.
//
// Changelog:
//   2023-04-01 Alice: initial version
//   2023-06-12 Bob: fixed rounding bug (issue #214)
//   2024-01-09 Alice: added EU VAT handling
//   2024-03-22 Carol: refactored discount logic
package invoicing

// added by Bob, 2023-06-12
// reviewed by Alice
func applyRounding(amount Cents) Cents {
    return (amount + 50) / 100 * 100
}

Instruction: Remove the journal and attribution comments. Keep anything that is still genuinely useful, and say where the deleted information now lives.

// Package invoicing computes invoice totals.
package invoicing

// applyRounding rounds amount to the nearest whole currency unit
// (banker-style half-up at the 50-cent boundary).
func applyRounding(amount Cents) Cents {
    return (amount + 50) / 100 * 100
}

Task 4 — Delete the closing-brace comment by extracting a function (Java)¶

Difficulty: Medium

Scenario: The nesting got deep enough that someone annotated each closing brace so they could tell which block was ending. The comments are a symptom; the disease is depth.

public void process(List<Order> orders) {
    for (Order order : orders) {
        if (order.isValid()) {
            for (LineItem item : order.getItems()) {
                if (item.isInStock()) {
                    if (item.getQty() > 0) {
                        reserve(item);
                    } // end if qty > 0
                } // end if in stock
            } // end for items
        } // end if valid
    } // end for orders
}

Instruction: Get rid of the // end ... comments. Do not do it by simply deleting them — remove the reason they were written.

public void process(List<Order> orders) {
    orders.stream()
          .filter(Order::isValid)
          .forEach(this::reserveAvailableItems);
}

private void reserveAvailableItems(Order order) {
    for (LineItem item : order.getItems()) {
        if (isReservable(item)) {
            reserve(item);
        }
    }
}

private boolean isReservable(LineItem item) {
    return item.isInStock() && item.getQty() > 0;
}

Task 5 — Convert a WHAT comment into a WHY comment (Go)¶

Difficulty: Medium

Scenario: The comment describes the mechanics the reader can already see, while the genuinely surprising part — why this exact value — goes unexplained. Six months later someone will "clean up" the magic number and break production.

func retryDelay(attempt int) time.Duration {
    // multiply 100ms by 2 to the power of attempt, capped at 30 seconds
    backoff := 100 * time.Millisecond * (1 << attempt)
    if backoff > 30*time.Second {
        backoff = 30 * time.Second
    }
    return backoff
}

Instruction: Rewrite the comment so it explains something the code does not already say. Delete what is redundant.

func retryDelay(attempt int) time.Duration {
    const (
        base = 100 * time.Millisecond
        // The upstream payment gateway closes idle connections at 30s,
        // so capping here keeps us inside one connection lifecycle and
        // avoids a reconnect storm under sustained failure (see INC-4821).
        cap = 30 * time.Second
    )
    backoff := base * (1 << attempt)
    if backoff > cap {
        backoff = cap
    }
    return backoff
}

Task 6 — Fix the misleading, outdated comment that contradicts the code (Python)¶

Difficulty: Medium

Scenario: The function was changed but its comment was not. The comment now actively lies, which is worse than no comment: a reader who trusts it will make a wrong decision.

def is_eligible(user) -> bool:
    # Returns True only for users over 18 with a verified email.
    if user.age < 21:
        return False
    if not user.email_verified:
        return False
    if user.country in BLOCKED_COUNTRIES:
        return False
    return True

Instruction: The comment and the code disagree on two points (the age threshold and the country check). Resolve the contradiction. State your assumption about which one is correct, and make the comment and code agree.

def is_eligible(user) -> bool:
    """Eligibility gate. See policy POL-2024-07 for the rules below."""
    if user.age < MIN_AGE:               # 21: raised from 18 for regulatory reasons
        return False
    if not user.email_verified:
        return False
    if user.country in BLOCKED_COUNTRIES:
        return False
    return True

Task 7 — Write a proper doc-comment for a public API (Go)¶

Difficulty: Medium

Scenario: This is an exported function in a library other teams import. It has no doc-comment, so consumers must read its body to learn how to call it — including the parts they cannot see, like the error contract and the concurrency guarantee.

func ParseConfig(r io.Reader) (*Config, error) {
    data, err := io.ReadAll(r)
    if err != nil {
        return nil, fmt.Errorf("read config: %w", err)
    }
    var c Config
    if err := yaml.Unmarshal(data, &c); err != nil {
        return nil, fmt.Errorf("parse config: %w", err)
    }
    if c.Port == 0 {
        c.Port = 8080
    }
    return &c, nil
}

Instruction: Write a doc-comment that follows Go conventions and tells a caller everything they need to use this without reading the body: what it does, the input contract, the defaults it applies, the error behavior, and any safety notes.

// ParseConfig reads YAML configuration from r and returns the parsed Config.
//
// It reads r to EOF; the caller retains ownership of r and is responsible for
// closing it. Any field omitted in the YAML keeps its zero value, except Port,
// which defaults to 8080 when unset or zero.
//
// ParseConfig returns an error if r cannot be read or if the bytes are not
// valid YAML for the Config schema; the returned error wraps the underlying
// cause and can be inspected with errors.Is / errors.As. On error the returned
// *Config is nil.
//
// ParseConfig performs no I/O beyond reading r and is safe for concurrent use.
func ParseConfig(r io.Reader) (*Config, error) {
    data, err := io.ReadAll(r)
    if err != nil {
        return nil, fmt.Errorf("read config: %w", err)
    }
    var c Config
    if err := yaml.Unmarshal(data, &c); err != nil {
        return nil, fmt.Errorf("parse config: %w", err)
    }
    if c.Port == 0 {
        c.Port = 8080
    }
    return &c, nil
}

Task 8 — Turn an opaque comment into a clarifying one with a link (Java)¶

Difficulty: Hard

Scenario: The code does something that looks wrong — a deliberate, non-obvious workaround. The existing comment gestures at it without giving the reader anything actionable, so every future maintainer re-investigates from scratch.

// hack, don't remove
list.add(null);
list.addAll(results);
list.remove(0);

Instruction: Replace this useless comment with a clarifying comment that actually equips the next reader: explain why the odd dance is necessary and link to the authoritative source.

// Work around OpenJDK-8261236: ArrayList.addAll on an empty list with a
// presized backing array throws under -XX:+UseParallelGC on JDK 11.0.2.
// Seeding one element forces the slow-path copy, which is unaffected; we then
// drop the sentinel. Remove once we drop JDK < 11.0.4.
// Repro + analysis: https://bugs.openjdk.org/browse/JDK-8261236
list.add(null);          // sentinel: forces ArrayList onto the safe copy path
list.addAll(results);
list.remove(0);          // drop the sentinel

Task 9 — Replace a section-banner comment with extracted functions (Python)¶

Difficulty: Hard

Scenario: A long function is organized into phases marked by banner comments. The banners are honest — they name real phases — but a comment that labels a block is usually a function asking to be born.

def onboard_employee(emp, hr_system, payroll, it):
    # ----- validate -----
    if not emp.email:
        raise ValueError("email required")
    if emp.start_date < date.today():
        raise ValueError("start date in the past")

    # ----- create accounts -----
    account = it.create_account(emp.email)
    it.add_to_groups(account, emp.department.default_groups)
    mailbox = it.provision_mailbox(emp.email)

    # ----- enroll in payroll -----
    record = payroll.create_record(emp.ssn, emp.salary)
    payroll.set_tax_withholding(record, emp.tax_form)

    # ----- notify -----
    hr_system.send_welcome(emp.email)
    hr_system.schedule_orientation(emp, emp.start_date)
    return account

Instruction: Each banner marks a conceptual step. Refactor so the banners become unnecessary, and the top-level function reads as a sequence of named steps.

def onboard_employee(emp, hr_system, payroll, it):
    validate(emp)
    account = create_accounts(emp, it)
    enroll_in_payroll(emp, payroll)
    notify(emp, hr_system)
    return account


def validate(emp):
    if not emp.email:
        raise ValueError("email required")
    if emp.start_date < date.today():
        raise ValueError("start date in the past")


def create_accounts(emp, it):
    account = it.create_account(emp.email)
    it.add_to_groups(account, emp.department.default_groups)
    it.provision_mailbox(emp.email)
    return account


def enroll_in_payroll(emp, payroll):
    record = payroll.create_record(emp.ssn, emp.salary)
    payroll.set_tax_withholding(record, emp.tax_form)


def notify(emp, hr_system):
    hr_system.send_welcome(emp.email)
    hr_system.schedule_orientation(emp, emp.start_date)

Task 10 — Promote intent: the comment apologizes for the code (Java)¶

Difficulty: Hard

Scenario: The comment is doing emotional labor — apologizing, hedging, warning — instead of the code being clear enough not to need apology. Some of these comments hint at real risk; the others should evaporate once the code is fixed.

public Money convert(Money amount, Currency to) {
    // sorry this is ugly, rates is a global, careful
    double rate = Rates.GLOBAL.get(amount.currency() + "_" + to);
    // this might be null sometimes, not sure when
    double converted = amount.value() * rate;
    // TODO: handle the case where rate is missing (it crashes)
    return new Money(converted, to);
}

Instruction: Three comments, three different problems. Eliminate each by fixing the code rather than improving the prose. Keep a comment only where one is genuinely warranted.

public Money convert(Money amount, Currency to) {
    Rate rate = rates.lookup(amount.currency(), to)
        .orElseThrow(() -> new UnknownRateException(amount.currency(), to));
    return amount.times(rate.factor()).withCurrency(to);
}

Task 11 — Encode an invariant the comment merely asserts (Go)¶

Difficulty: Hard

Scenario: A comment states a precondition the caller must honor. Comments cannot enforce anything; the compiler and the type system can. Where the rule can be made unrepresentable, a comment is the weakest possible guardrail.

type Percentage struct {
    // Value must always be between 0 and 100. Callers must check before setting.
    Value float64
}

func ApplyDiscount(price Cents, p Percentage) Cents {
    // assumes p.Value is in [0, 100] — undefined behavior otherwise
    return price - Cents(float64(price)*p.Value/100)
}

Instruction: The comments describe an invariant ("must be between 0 and 100"). Move the invariant from prose into the construction of the type so that an out-of-range Percentage cannot exist. Keep a comment only for what the types still cannot express.

// Percentage is a value in the closed range [0, 100]. The zero value (0%) is
// valid; construct other values with NewPercentage, which is the only way to
// obtain one outside this package — guaranteeing every Percentage is in range.
type Percentage struct {
    value float64 // unexported: cannot be set out of range from outside
}

func NewPercentage(v float64) (Percentage, error) {
    if v < 0 || v > 100 {
        return Percentage{}, fmt.Errorf("percentage out of range [0,100]: %v", v)
    }
    return Percentage{value: v}, nil
}

func (p Percentage) Fraction() float64 { return p.value / 100 }

func ApplyDiscount(price Cents, p Percentage) Cents {
    // No range check needed: a Percentage cannot exist outside [0, 100].
    return price - Cents(float64(price)*p.Fraction())
}

Task 12 — Comment audit (Python — open-ended)¶

Difficulty: Hard

Scenario: Below is a realistic snippet carrying every comment smell from this chapter at once. List each comment, classify it, and write a one-line fix. This is the exercise that proves you can see the smells in the wild.

# CacheManager.py
# created 2022-08-01 by Dana
# modified 2023-05-17 by Eli: added TTL
# modified 2024-02-03 by Dana: thread safety (?)

import threading, time

class CacheManager:
    def __init__(self, ttl):
        self.ttl = ttl          # the ttl
        self.store = {}         # the store
        self.lock = threading.Lock()
        # self.max_size = 1000  # commented out, may re-add later

    def get(self, key):
        # acquire the lock
        with self.lock:
            # check if key in store
            if key in self.store:
                value, ts = self.store[key]
                # if not expired, return it
                if time.time() - ts < self.ttl:
                    return value
            return None

    def set(self, key, value):
        # store the value with a timestamp
        with self.lock:
            self.store[key] = (value, time.time())
            # NOTE: this never evicts, the dict grows forever. don't worry about it for now
        # end set

Instruction: Produce a table: comment → category → fix. Then state the order in which you'd attack them.

import threading
import time


class CacheManager:
    """A thread-safe in-memory cache with per-entry TTL.

    NOTE: entries are evicted lazily on read; there is no size bound yet.
    Tracked in CACHE-112.
    """

    def __init__(self, ttl: float):
        self.ttl = ttl
        self.store: dict = {}
        self.lock = threading.Lock()

    def get(self, key):
        with self.lock:
            entry = self.store.get(key)
            if entry and self._is_fresh(entry):
                return entry[0]
            return None

    def set(self, key, value):
        with self.lock:
            self.store[key] = (value, time.time())

    def _is_fresh(self, entry) -> bool:
        return time.time() - entry[1] < self.ttl

Self-Assessment¶

Rate yourself honestly. You have internalized this chapter when you can answer yes to all of these:

• I delete a comment by improving a name or extracting a function before I consider rewriting it.
• I never leave commented-out code in a commit — I trust git to remember.
• I do not write journal or attribution comments; I let version control own history and ownership.
• When I see a WHAT comment, I either delete it (the code already says it) or upgrade it to a WHY.
• I treat a comment that contradicts the code as a priority bug, and I fix code and comment in the same commit.
• I write real doc-comments for public APIs covering contract, defaults, errors, and concurrency.
• I replace opaque "hack, don't remove" notes with a cause, a mechanism, an exit condition, and a link.
• I recognize banner and closing-brace comments as functions waiting to be extracted.
• I move invariants from prose into types/constructors whenever the language lets me.
• I keep exactly the comments that say something the code cannot — and no others.

Scoring: 9–10 yes → you have the chapter. 6–8 → re-do Tasks 5, 8, and 11. Below 6 → re-read the chapter README and start over from Task 1.

Related Topics¶

• junior.md — the beginner-level walkthrough of each comment anti-pattern.
• find-bug.md — snippets where a comment hides (or causes) a real bug.
• optimize.md — turning mediocre comments into load-bearing ones.
• Chapter README — the positive rules: when a comment does earn its place.
• Refactoring: code smells — Extract Function and Rename underlie most fixes here; the comment smells are the same smells seen from the documentation side.