Simplifying Method Calls — Find the Bug¶

12 wrong refactors.

Bug 1 — Rename Method but missed a reflection call (Java)¶

Original:

public Money getCharge() { ... }
// Reflection caller:
Method m = c.getClass().getMethod("getCharge");

"Refactored":

public Money totalIncludingTax() { ... }
// Reflection caller still says "getCharge" — runtime NoSuchMethodError

Bug

IDE rename doesn't update string-based reflection or Spring SpEL or framework annotations. **Fix:** Search the codebase for the old name as a string. Update all references including configuration, JSON keys, etc.

Bug 2 — Add Parameter without updating overrides (Java)¶

abstract class Validator { abstract boolean validate(Order o); }
class StrictValidator extends Validator { boolean validate(Order o) { ... } }

Refactor base: boolean validate(Order o, Context ctx).

But StrictValidator.validate(o) no longer overrides — it's a different signature. Compile error or hidden bug if no @Override.

Bug

When changing an abstract method, every subclass must update. IDE refactoring usually handles this; manual edits don't. **Fix:** Always use `@Override` annotation. The compiler then complains.

Bug 3 — Separate Query from Modifier breaks atomicity (Java)¶

Original:

synchronized String getTotalAndSetReady() {
    String t = computeTotal();
    readyForSummary = true;
    return t;
}

"Refactored":

String getTotal() { return computeTotal(); }
void setReady() { readyForSummary = true; }

Caller now does s.getTotal(); s.setReady(); — without synchronization, race condition if another thread is between.

Bug

The original was atomic; the refactor isn't. **Fix:** Either keep the combined method, or document that callers must serialize, or take a coarser lock at the caller. Lesson: separating Query from Modifier can break atomicity invariants.

Bug 4 — Introduce Parameter Object reorders construction (Python)¶

Original:

def overlaps(start_a, end_a, start_b, end_b): ...
overlaps(jan1, jan31, feb1, feb28)

"Refactored":

@dataclass
class DateRange:
    start: date
    end: date

def overlaps(a: DateRange, b: DateRange): ...

overlaps(DateRange(jan1, feb1), DateRange(jan31, feb28))   # ❌ wrong dates

Bug

Caller mistakenly grouped `(start_a, start_b)` together rather than `(start_a, end_a)` together. **Fix:** Group by *meaning* (range A's bounds together).

overlaps(DateRange(jan1, jan31), DateRange(feb1, feb28))

Lesson: the parameter object's meaning must align with the caller's mental model.

Bug 5 — Replace Constructor with Factory Method but caller serialization (Java)¶

Original:

public class Order {
    public Order(String id, ...) { ... }
}
// Jackson reads JSON → calls constructor.

"Refactored":

public class Order {
    private Order(String id, ...) { ... }   // private
    public static Order of(String id, ...) { ... }
}

Jackson can't deserialize — no public constructor.

Bug

Many serialization libraries require a public no-arg or annotated constructor. **Fix:**

public class Order {
    @JsonCreator
    Order(@JsonProperty("id") String id, ...) { ... }   // package-private
    public static Order of(String id, ...) { ... }
}

Or use Jackson's `@JsonCreator` on the factory method:

@JsonCreator
public static Order of(...) { ... }

Bug 6 — Replace Error Code with Exception in Go (Go)¶

// Original:
func Withdraw(amount float64) int {
    if amount > balance { return -1 }
    balance -= amount
    return 0
}

// "Refactored":
func Withdraw(amount float64) {
    if amount > balance { panic("insufficient") }   // ❌
    balance -= amount
}

Bug

Go convention is to return errors, not panic. `panic` is for unrecoverable programming errors. **Fix:**

func Withdraw(amount float64) error {
    if amount > balance { return ErrInsufficientFunds }
    balance -= amount
    return nil
}

Lesson: each language has its own idiom for failure reporting; don't blindly translate.

Bug 7 — Hide Method but tests still need it (Java)¶

Original:

public class Calculator {
    public double internalRound(double v) { ... }
}

Tests:

@Test void testRound() { assertEquals(2.0, calc.internalRound(2.4)); }

"Refactored": make internalRound private. Tests fail to compile.

Bug

Hiding a method that has tests breaks tests. **Fix options:** 1. Test through public interface only (preferred — tests aren't tied to internals). 2. Use package-private + same-package tests (Maven/Gradle conventions). 3. Use `@VisibleForTesting` annotation on a package-private method. Lesson: hide aggressively, but consider test seams.

Bug 8 — Preserve Whole Object causes circular dependency (Java)¶

Original:

class Plan {
    boolean withinRange(double low, double high) { ... }
}

"Refactored":

class Plan {
    boolean withinRange(TempRange tr) { return tr.getLow() ... }   // Plan now depends on TempRange
}

If TempRange already depended on Plan (e.g., TempRange.fitsPlan(Plan p)), you've created a cycle.

Bug

Mutual dependency between Plan and TempRange. **Fix:** Either break the original direction, or pass primitives still. Lesson: Preserve Whole Object adds a dependency edge — check the graph before making the change.

Bug 9 — Replace Parameter with Method Call calls expensive method (Java)¶

Original:

double tax(double base, double rate) { return base * rate; }
double total(Order o) {
    double base = o.subtotal();
    double rate = lookupRateExpensive(o.country());
    return base + tax(base, rate);
}

"Refactored":

double tax(double base) { return base * lookupRateExpensive(country()); }
double total(Order o) {
    double base = o.subtotal();
    return base + tax(base);
}

Bug

`lookupRateExpensive` is now hidden inside `tax()`. If `total()` calls `tax()` more than once, the expensive lookup happens repeatedly. **Fix:** Cache or memoize. Or keep the parameter explicit:

double tax(double base, double rate) { return base * rate; }
double total(Order o) {
    double rate = lookupRateExpensive(o.country());   // cached
    return o.subtotal() + tax(o.subtotal(), rate);
}

Bug 10 — Encapsulate Downcast hides a real type mismatch (Java)¶

Original:

Object o = readings.last();
if (o instanceof Reading) {
    Reading r = (Reading) o;
    process(r);
} else {
    log.warn("unexpected type: " + o.getClass());
}

"Refactored":

public Reading lastReading() { return (Reading) readings.last(); }   // ❌ throws ClassCastException

Bug

The original handled the non-Reading case. The refactor throws ClassCastException for the same input. **Fix:** Either ensure the cast is always safe, or return Optional:

public Optional<Reading> lastReading() {
    Object o = readings.last();
    return o instanceof Reading r ? Optional.of(r) : Optional.empty();
}

Bug 11 — Builder allows invalid state (Java)¶

public class HttpRequest {
    public static class Builder {
        private String url;
        private String method;
        public Builder url(String u) { this.url = u; return this; }
        public Builder method(String m) { this.method = m; return this; }
        public HttpRequest build() { return new HttpRequest(url, method); }
    }
}

new HttpRequest.Builder().build();   // ❌ url is null, method is null

Bug

Builder doesn't validate; you can construct an invalid request. **Fix:** Validate in `build()`:

public HttpRequest build() {
    Objects.requireNonNull(url, "url required");
    if (method == null) method = "GET";   // sensible default
    return new HttpRequest(url, method);
}

Or use a typed builder progression:

new HttpRequestBuilder().url(...).method(...).build();   // each step returns next-state builder

Bug 12 — Replace Exception with Test introduces TOCTOU (Java)¶

Original:

try {
    return cache.get(key);
} catch (NotFoundException e) {
    return loadAndCache(key);
}

"Refactored":

if (!cache.contains(key)) {
    return loadAndCache(key);
}
return cache.get(key);   // ❌ TOCTOU race

Bug

Time-of-check to time-of-use: between `contains` and `get`, another thread could evict the entry. `get` then throws. **Fix:** Atomic operation:

return cache.get(key, k -> loadAndCache(k));   // computeIfAbsent pattern

Lesson: Replace Exception with Test must preserve atomicity. Use atomic APIs (computeIfAbsent, ConcurrentMap).

Patterns¶

Bug	Root cause
Reflection-based caller missed	IDE rename limited
Subclass override broken	No @Override annotation
Atomic operation split	Lost synchronization
Param object grouping wrong	Misaligned with mental model
Constructor private breaks deser	Need @JsonCreator
Wrong-language idiom	panic vs. error
Hidden method fails test	No test seam
Circular dep	Preserve Whole adds edge
Hidden expensive call	Replace Parameter calls more often
Encapsulated cast removed type guard	Lost null/type check
Invalid state in builder	No validation in build()
TOCTOU race	Atomicity lost in test-then-act

Next¶

optimize.md, tasks.md, interview.md