Kihagyás

Good Comment or Not?

In the following examples, decide whether the comment is helpful or not. If not, suggest a better alternative (e.g., refactoring the code, removing the comment, or rewriting the comment).

Express your opinion by standing to the "Good Comment" side or to the "Not a Good Comment" side of the room.

Explains non-obvious why (business rule) 
1
2
3
4
def can_checkout(cart):
    # Business rule: gift cards cannot be purchased with a gift card.
    # Prevents money laundering loophole reported in SEC-1720.
    return not cart.contains_gift_card or not cart.is_paid_with_gift_card
Restates the code (noise) 
1
2
# increment i by 1
i = i + 1
Documents units and source 
1
2
# Altitude in meters (sensor: BMP390, datasheet v1.3). Negative allowed in valleys.
altitude_m = read_baro_altitude()
Outdated lie 
1
2
# uses SHA-256
digest = hashlib.sha1(data).hexdigest()
Contract for public function 
1
2
3
4
5
6
7
def parse_iso8601(s: str) -> datetime:
    """
    Returns UTC-aware datetime.
    Accepts: 'YYYY-MM-DDTHH:MM:SSZ' or with offset, raises ValueError otherwise.
    Idempotent for equivalent strings.
    """
    return dateutil.parser.isoparse(s).astimezone(timezone.utc)
Excuses instead of information 
1
2
# Don't touch this, it works!!!
compute()
Marks surprising side effect 
1
2
3
4
5
def get_user(id):
    # Also refreshes session TTL to avoid auto-logout (product decision PRD-291).
    user = db.fetch(id)
    session.extend(id, minutes=30)
    return user
Explains algorithm reference 
1
2
3
# Welford's online algorithm for mean/variance (Knuth TAOCP 4.2.2).
def update_stats(s, x):
    ...
Narrative of history (git belongs here) 
1
2
3
4
# On 2021-03-02 John changed this from 7 to 8
# On 2022-11-15 Alice changed this from 8 to 10
# On 2023-06-30 Bob changed this from 10 to 12
THRESHOLD = 8
Clarifies intent of guard 
1
2
3
4
# Avoids double-charging when webhook retries (Stripe retries up to 3 times).
if is_duplicate(event):
    return
charge_customer(event)
Jargon without context 
1
2
# teleport the foo into bar then do baz
do_the_thing()
Temporary workaround clearly labeled 
1
2
# TODO(backend-1245): Remove when API v3 returns non-null items[].
items = resp.get("items") or []
TODO with no explanation 
1
2
# TODO: fix later
hacky()
Clarifies sentinel and range 
1
2
3
# max_inclusive because the device's counter saturates at 255
for n in range(0, max_inclusive + 1):
    ...
Comments competing with names 
1
2
3
# subtract fee for active premium
if user.is_active() and user.tier == Tier.PREMIUM:
    price -= fee
Encodes rationale behind 'magic' constant 
1
2
# 3571 seconds = ~59m; prime TTL reduces cache stampede alignment
cache.set(key, value, ttl=3571)
Sarcastic / emotional tone 
1
2
# uggh this stupid bug from data team again :)
sanitize()
Documents performance caveat 
1
2
3
# Hot path (~20M/s). Avoid allocations; reuse buffer.
for item in stream:
    buf.append(transform(item))
Security rationale 
1
2
3
4
def sanitize_filename(name: str) -> str:
    # Reject path traversal. Keep only basename and safe chars.
    base = os.path.basename(name)
    return SAFE_RE.sub("", base)
Commented-out code (dead) 
1
2
3
# old = compute_v1(data)
# newer = compute_v2(data)  # keep just in case
result = compute(data)
Warning about precision/rounding method 
1
2
# We round half away from zero to match accounting expectations.
total = Decimal(amount).quantize(Decimal("0.01"), rounding=ROUND_HALF_UP)
Narrative comment instead of small function 
1
2
# apply promotions, loyalty, and coupons, but not shipping or tax
total = subtotal - promos(user, items) - loyalty(user) - coupons(cart)
Reference to external policy 
1
2
3
# GDPR: erase PII on deletion; keep invoice lines for 8 years (TAX-7).
def delete_user(u):
    ...
Misleading comment due to rename 
1
2
# price in EUR
price_usd = convert(price_eur, "USD")
Document precondition/postcondition succinctly 
1
2
// Pre: account is active. Post: balance >= 0 or throws InsufficientFunds.
public void withdraw(Account a, Money m) { ... }
Boasts about obvious code 
1
2
3
# loop through users
for u in users:
    ...
Marks concurrency constraint 
1
2
3
# Only leader updates metrics; followers read (raft semantics).
if is_leader():
    update_metrics()
Blames vs informs 
1
2
# HACK because frontend is trash
parse()
Notes unexpected library behavior 
1
2
# pandas read_csv drops trailing empty columns unless keep_default_na=False
df = pd.read_csv(p, keep_default_na=False)
Explains coordinate system 
1
2
# Screen origin top-left, y grows downward.
draw(x, y)
Version-pinned behavior documented 
1
2
# numpy<=1.26: np.unique returns sorted; >=2.0: sorting optional.
uniq = np.unique(arr)
Overcommented with jokes 
1
2
# unleash the kraken of performance!!!
sort_fast()
Clarifies non-intuitive domain term 
1
2
# 'business day' excludes company holidays (see calendar svc).
next_day = calendar.next_business_day(d)
Comment contradicts logic 
1
2
3
# allow only admins
if not user.is_admin:
    return True