Kihagyás

Should I Comment or Refactor?

When you encounter a piece of code that seems unclear or complex, you might wonder whether to add a comment to explain it or to refactor the code itself for better clarity. Here are some examples you to consider when making this decision.

Express your opinion by standing to the comment side or to the refactor side of the room.

Magic numbers, unknown units 
1
2
3
def charge(amount):
    # applies platform fee
    return int(amount * 1.0375 + 199)
Misleading names 
1
2
3
4
5
def do_it(a, b, c):
    r = a - b
    if c:
        r = r * -1
    return r
Overlong function with hidden steps 
1
2
3
4
5
6
7
def onboard(user):
    # validate input
    ...
    # sanitize input
    ...
    # create user record
    ...
Non-obvious business rule (legal constraint?) 
1
2
def can_sell_to(country_code: str) -> bool:
    return country_code not in {"IR", "KP", "SY", "CU", "CR"}
Domain units unclear 
1
2
3
4
def move(x, y, speed):
    pos_x = x + speed * 0.27777778
    pos_y = y
    return pos_x, pos_y
Complex regex for production 
1
EMAIL_RE = re.compile(r"^(?=.{1,254}$)(?=.{1,64}@)[A-Za-z0-9._%+-]+@(?:[A-Za-z0-9-]+\.)+[A-Za-z]{2,}$")
Duplicated logic in two branches 
1
2
3
4
if premium:
    total = base + addon + tax(base + addon)
else:
    total = base + addon + tax(base + addon)
Boolean flag doing too much 
1
2
3
4
5
6
def render(data, fast=False):
    if fast:
        # skip safety checks & caching
        ...
    else:
        ...
Early return maze 
1
2
3
4
5
6
7
def authorize(u, r):
    if not u: return False
    if not r: return False
    if u.is_admin(): return True
    if r.owner_id == u.id: return True
    if u.team_id and u.team_id == r.team_id: return True
    return False
Magic time windows 
1
2
def is_active(ts):
    return (now() - ts).seconds < 172800
Hidden side effects 
1
2
3
4
def get_user(id):
    user = db.fetch(id)
    cache.set(f"user:{id}", user)  # side effect? what happens when database changes?
    return user
Long parameter list vs object 
1
2
def book_flight(fn, ln, dep, arr, dt, seat, meal, extra_bag, insurance):
    ...
Obscure math without source 
1
2
def normalize(v):
    return (v - 0./ 1.28255  # ?
Comment drifting vs self-explanatory names 
1
2
3
# subtract fee if user is premium and not expired
if u.is_active() and u.level == 0:
    price -= fee
Temporary workaround for upstream bug 
1
2
3
4
5
6
def fetch_orders():
    # TODO: until API v3 fixes null 'items', treat None as []
    raw = client.get("/orders")
    for o in raw:
        o["items"] = o.get("items") or []
    return raw
Error handling that swallows details 
1
2
3
4
try:
    process()
except Exception:
    return None
Switch on strings vs polymorphism/strategy 
1
2
3
4
5
def price(kind, base):
    if kind == "student": return base * 0.8
    if kind == "vip": return base * 0.6
    if kind == "corp": return base * 0.9
    return base
Optional returns vs Result type 
1
2
3
4
5
def parse_amount(s: str) -> float | None:
    try:
        return float(s.replace(",", "."))
    except ValueError:
        return None
Hidden coordinate system transformation 
1
2
def to_screen(px, py):
    return int(px*1.5+240), int(180-py*1.5)
Comment vs rename (negated boolean) 
1
2
if not disabled:
    enable_feature()
Overloaded method does two concerns 
1
2
// validates and also persists?
public User save(User u, boolean validate) { ... }
Public utility with edge-case traps 
1
2
3
def slugify(text):
    # removes diacritics; may break locales; stable across versions?
    ...
Repeated constants 
1
2
3
if retries > 3: ...
if timeout < 3: ...
if backoff == 3: ...
Non-obvious security rationale 
1
2
def sanitize(filename):
    return filename.replace("..", "").replace("/", "")
Test names unclear vs docstring 
1
2
def test_case_17():
    assert calculate_discount(100, "VIP") == 70
Inline comment vs extract method 
1
2
# apply promotions
total = sum(items) - loyalty_discount(user, items)
API boundary: parameters contract 
1
2
// accepts ISO-8601 only; ignores timezone
public Instant parseTimestamp(String s) { ... }