Over-Engineering Anti-Patterns — Refactoring Practice¶

Category: Development Anti-Patterns → Over-Engineering — effort spent solving problems you don't have. Covers (collectively): Premature Optimization · Speculative Generality · Gold Plating · Yo-yo Problem · Lasagna Code · Accidental Complexity · Soft Coding · Bikeshedding

These are not "spot the smell" puzzles — find-bug.md does that. Here the code works, but it does too much: an interface with one implementation, a five-layer call chain, a rules engine standing in for an if, a loop hand-tuned without a profile. Your job is the opposite of every other topic's "optimize": here "optimize" almost always means simplify and remove, not speed up. You take over-engineered code and collapse it to the simplest thing that still passes the same tests.

The twist matters. In a few exercises you'll also see that the simpler code is as fast or faster — a benchmark sketch makes the point that abstraction isn't free: every interface dispatch, every layer hop, every config lookup is a cost the simple version never pays.

The discipline is identical to Bad Structure:

1. Pin behavior first. Write a characterization test — a test that records what the code does now. You are freezing observable behavior, not blessing the design.
2. Take small, reversible steps. One named refactoring at a time (Inline Function, Inline Class, Collapse Hierarchy, Remove Middle Man, …), tests green after each, commit.
3. Separate structural from behavioral commits. Simplification must preserve behavior; if a test goes red, the last step is the culprit — revert it. A public API removal needs its own commit and a deprecation note.

Each worked solution names the canonical refactoring moves (Fowler's Refactoring, 2nd ed.) so you build the vocabulary, not just the instinct.

How to use this file: read the "Before" code, decide yourself what to remove and what to keep, write the move sequence down, then expand the solution. The hardest judgment call — exercises 3 and 5 — is what NOT to remove. A justified seam is not over-engineering; deleting it is under-engineering. Knowing the difference is the whole skill.

Table of Contents¶

Exercise 1 — Revert a premature optimization¶

Anti-pattern: Premature Optimization. Goal: replace a cryptic hand-optimized routine with the clear idiomatic version, and prove with a benchmark that you lost no speed (here you actually gain some). Constraints: identical output for every input; no API change.

// Before — manual loop-unrolling + bit tricks on a routine that was never profiled.
func SumPositive(nums []int) int {
    s, i, n := 0, 0, len(nums)
    for ; i < n-3; i += 4 { // hand-unrolled by 4
        if nums[i] > 0 {
            s += nums[i]
        }
        if nums[i+1] > 0 {
            s += nums[i+1]
        }
        if nums[i+2] > 0 {
            s += nums[i+2]
        }
        if nums[i+3] > 0 {
            s += nums[i+3]
        }
    }
    for ; i < n; i++ { // tail
        if nums[i] > 0 {
            s += nums[i]
        }
    }
    return s
}

func BenchmarkSumPositive(b *testing.B) {
    nums := makeData(1_000_000) // representative size
    b.ResetTimer()
    for i := 0; i < b.N; i++ {
        _ = SumPositive(nums)
    }
}
// $ go test -bench=SumPositive -benchmem -count=10 > new.txt
// compare old.txt new.txt with benchstat

// After — clear, idiomatic, no manual unrolling.
func SumPositive(nums []int) int {
    s := 0
    for _, n := range nums {
        if n > 0 {
            s += n
        }
    }
    return s
}

name           old time/op    new time/op    delta
SumPositive-8    412µs ± 2%     398µs ± 1%   -3.4%  (p=0.000 n=10+10)

Exercise 2 — Inline the speculative factory¶

Anti-pattern: Speculative Generality. Goal: collapse an interface + factory + strategy that has exactly one implementation, no test-double need, and no published-contract role. Constraints: preserve the greeting output; callers may be updated.

// Before — interface, factory, strategy, engine... to say "Hello, X".
interface GreetingStrategy {
    String greet(String name);
}

class DefaultGreetingStrategy implements GreetingStrategy {
    public String greet(String name) { return "Hello, " + name; }
}

class GreetingStrategyFactory {
    static GreetingStrategy create() { return new DefaultGreetingStrategy(); }
}

class GreetingEngine {
    private final GreetingStrategy strategy;
    GreetingEngine(GreetingStrategy s) { this.strategy = s; }
    String run(String name) { return strategy.greet(name); }
}

// Only call site anywhere in the codebase:
String msg = new GreetingEngine(GreetingStrategyFactory.create()).run("Sam");

grep -rn "implements GreetingStrategy" --include="*.java" .   # one hit (the default)
grep -rn "GreetingStrategy" src/test                          # zero — never mocked

// After — the one behavior, stated once.
class Greeter {
    static String greet(String name) { return "Hello, " + name; }
}

// call site:
String msg = Greeter.greet("Sam");

Exercise 3 — Keep the seam that's justified (counter-case)¶

Anti-pattern: Speculative Generality — the counter-case. Goal: look at an interface-with-one-implementation that looks exactly like Exercise 2, and correctly decide to keep it. The skill here is restraint: not every abstraction is over-engineering, and inlining a real seam is under-engineering.

// Before — also one production implementation. Inline it too?
interface PaymentGateway {
    Receipt charge(Money amount, Card card);
}

class StripeGateway implements PaymentGateway {
    public Receipt charge(Money amount, Card card) {
        return Stripe.charge(apiKey, card, amount.cents()); // real network call
    }
}

class CheckoutService {
    private final PaymentGateway gateway;
    CheckoutService(PaymentGateway gateway) { this.gateway = gateway; }
    Receipt checkout(Cart cart) {
        return gateway.charge(cart.total(), cart.card());
    }
}

// After — unchanged code, plus the rationale that protects it from a wrong inline.
/**
 * Seam is intentional, not speculative:
 *  - injected as a fake in CheckoutServiceTest (cannot charge real cards in unit tests)
 *  - isolates the volatile Stripe SDK behind our own boundary
 * Do NOT inline. See over-engineering/optimize.md Ex.3.
 */
interface PaymentGateway {
    Receipt charge(Money amount, Card card);
}

Exercise 4 — Trim the gold-plated API¶

Anti-pattern: Gold Plating. Goal: reduce a public function whose surface ballooned with options nobody asked for, down to its real, used surface — with a deprecation path for the public parameters you remove. Constraints: the two parameters that are actually used at call sites must keep working unchanged.

# Before — task was "save the report to a file". This is what shipped.
def save_report(report, path, *, fmt="txt", compress=False, encrypt=False,
                email_to=None, watermark=None, theme="dark", retries=3,
                upload_to_s3=False, callback=None):
    text = report.render(theme=theme)
    if watermark:
        text = _apply_watermark(text, watermark)      # never exercised
    data = text.encode()
    if compress:
        data = gzip.compress(data)                     # never exercised
    if encrypt:
        data = _encrypt(data)                          # never exercised
    _write_with_retries(path, data, retries)
    if email_to:
        _email(email_to, data)                         # never exercised
    if upload_to_s3:
        _s3_put(path, data)                            # never exercised
    if callback:
        callback(path)                                 # never exercised

grep -rn "save_report(" --include="*.py" . | grep -oE "fmt=|compress=|encrypt=|email_to=|watermark=|theme=|retries=|upload_to_s3=|callback="
# result: only `theme=` and `retries=` ever appear.

# After — Step 4a: real surface kept; extras deprecated, not yet deleted.
import warnings

def save_report(report, path, *, theme="dark", retries=3, **deprecated):
    if deprecated:
        warnings.warn(
            f"save_report() options {list(deprecated)} are deprecated and ignored; "
            "removal in v3.0. See CHANGELOG.",
            DeprecationWarning, stacklevel=2,
        )
    data = report.render(theme=theme).encode()
    _write_with_retries(path, data, retries)

# After — Step 4b (v3.0): the surface the task actually required.
def save_report(report, path, *, theme="dark", retries=3):
    data = report.render(theme=theme).encode()
    _write_with_retries(path, data, retries)

Exercise 5 — Reduce accidental complexity (framework → direct)¶

Anti-pattern: Accidental Complexity. Goal: the problem is "uppercase a list of names." The code is a generic step-pipeline "framework." Replace the framework with the direct expression. Constraints: same output list; same order.

# Before — a reusable "transformation pipeline" engine for a one-line map().
from abc import ABC, abstractmethod

class Step(ABC):
    @abstractmethod
    def apply(self, data): ...

class UpperStep(Step):
    def apply(self, data): return [x.upper() for x in data]

class StripStep(Step):
    def apply(self, data): return [x.strip() for x in data]

class Pipeline:
    def __init__(self, steps): self.steps = steps
    def run(self, data):
        for step in self.steps:
            data = step.apply(data)
        return data

# the only call site:
clean = Pipeline([StripStep(), UpperStep()]).run(raw_names)

# After — the essential problem, directly.
def clean_names(raw_names):
    return [name.strip().upper() for name in raw_names]

clean = clean_names(raw_names)

pipeline version:   2.10 µs per call
direct comprehension: 0.74 µs per call   (~2.8× faster, fewer allocations)

Exercise 6 — Collapse the Lasagna layers¶

Anti-pattern: Lasagna Code. Goal: collapse a chain of pass-through layers that each only forward the call, keeping the one layer that has a real responsibility. Constraints: preserve OrderApi.getOrder's behavior; callers unchanged.

// Before — five layers; only one does real work.
class OrderApi {
    private final OrderService service;
    OrderApi(OrderService s) { this.service = s; }
    Order getOrder(int id) { return service.getOrder(id); }       // forwards
}
class OrderService {
    private final OrderManager manager;
    OrderService(OrderManager m) { this.manager = m; }
    Order getOrder(int id) { return manager.fetch(id); }          // forwards (renames)
}
class OrderManager {
    private final OrderHandler handler;
    OrderManager(OrderHandler h) { this.handler = h; }
    Order fetch(int id) { return handler.handle(id); }            // forwards (renames)
}
class OrderHandler {
    private final OrderRepository repo;
    OrderHandler(OrderRepository r) { this.repo = r; }
    Order handle(int id) {
        Order o = repo.load(id);
        if (o == null) throw new NotFoundException(id);           // REAL job: not-found policy
        return o;
    }
}
class OrderRepository {
    Order load(int id) { return db.queryOne("SELECT ... WHERE id=?", id); } // REAL job: the query
}

// After — two layers, each with a real responsibility.
class OrderRepository {
    Order findOrThrow(int id) {
        Order o = db.queryOne("SELECT ... WHERE id=?", id);   // query
        if (o == null) throw new NotFoundException(id);       // not-found policy
        return o;
    }
}

class OrderApi {                  // public entry point preserved
    private final OrderRepository repo;
    OrderApi(OrderRepository repo) { this.repo = repo; }
    Order getOrder(int id) { return repo.findOrThrow(id); }
}

Exercise 7 — Flatten Yo-yo inheritance to composition¶

Anti-pattern: Yo-yo Problem. Goal: to learn what notify() does you must climb a four-level inheritance chain with super calls weaving up and down. Replace the inheritance tower with composition so behavior lives in one place. Constraints: preserve the message each notifier produces.

// Before — behavior smeared across four levels; super calls yo-yo up and down.
abstract class Notifier {
    String notify(String msg) { return decorate(msg); }
    protected String decorate(String msg) { return msg; }
}
class TimestampedNotifier extends Notifier {
    protected String decorate(String msg) { return "[" + now() + "] " + super.decorate(msg); }
}
class TaggedNotifier extends TimestampedNotifier {
    protected String decorate(String msg) { return super.decorate("#alert " + msg); }
}
class UpperNotifier extends TaggedNotifier {
    protected String decorate(String msg) { return super.decorate(msg).toUpperCase(); }
}
// To answer "what does new UpperNotifier().notify('hi') return?" you climb 4 files.

// After — each behavior is one named transform; composition lists them in order.
@FunctionalInterface
interface Transform { String apply(String s); }

class Notifier {
    private final List<Transform> steps;
    Notifier(Transform... steps) { this.steps = List.of(steps); }
    String notify(String msg) {
        String out = msg;
        for (Transform t : steps) out = t.apply(out);
        return out;
    }
}

// Named, reusable transforms — behavior lives in one obvious place:
Transform tag       = s -> "#alert " + s;
Transform timestamp = s -> "[" + now() + "] " + s;
Transform upper     = String::toUpperCase;

// The old UpperNotifier, expressed as the exact pipeline it really was:
Notifier upperNotifier = new Notifier(tag, timestamp, upper);

Exercise 8 — Migrate the soft-coded rule engine back to code¶

Anti-pattern: Soft Coding. Goal: business logic has been encoded as a JSON "rule engine" interpreted at runtime — untyped, untestable, undebuggable. Migrate the rules back into tested code. Constraints: identical eligibility decision for every input; the migration must be provably behavior-preserving.

# Before — discount logic lives in JSON, run by a hand-rolled interpreter.
RULES = [
    {"if": {"field": "tier", "op": "==", "val": "vip"},
     "and": {"field": "total", "op": ">=", "val": 100},
     "then": {"discount": 0.15}},
    {"if": {"field": "region", "op": "in", "val": ["EU", "UK"]},
     "and": {"field": "total", "op": ">=", "val": 50},
     "then": {"discount": 0.10}},
    {"else": {"discount": 0.0}},
]

def discount(order):
    for rule in RULES:
        if "else" in rule:
            return rule["else"]["discount"]
        if _match(order, rule["if"]) and _match(order, rule["and"]):
            return rule["then"]["discount"]
    return 0.0

def _match(order, cond):                       # a tiny, untyped expression interpreter
    actual = getattr(order, cond["field"])
    op = cond["op"]
    if op == "==":  return actual == cond["val"]
    if op == ">=":  return actual >= cond["val"]
    if op == "in":  return actual in cond["val"]
    raise ValueError(f"unknown op {op}")

cases = [Order(tier=t, region=r, total=tot)
         for t in ("vip", "regular")
         for r in ("EU", "US", "UK")
         for tot in (49, 50, 99, 100, 200)]
golden = {c: discount(c) for c in cases}   # frozen expected outputs

# After — the rules as tested, typed, debuggable code; order preserved.
def discount(order) -> float:
    if order.tier == "vip" and order.total >= 100:
        return 0.15
    if order.region in ("EU", "UK") and order.total >= 50:
        return 0.10
    return 0.0

Exercise 9 — Pre-decide the bikeshed¶

Anti-pattern: Bikeshedding. Goal: a project hand-rolled a configurable "style" layer so every formatting choice can be argued and toggled. Replace the whole apparatus with a tool's default, so the trivial choice is pre-decided and undebatable. Constraints: output formatting may change to the tool's canonical form (that's the point); logic unchanged.

// Before — a homegrown formatter with knobs for every style preference,
// each of which spawned a 40-comment PR thread.
type FormatOptions struct {
    IndentWithTabs bool
    IndentWidth    int
    BraceOnNewLine bool
    MaxLineLength  int
    QuoteStyle     string // "single" | "double"
    TrailingComma  bool
}

func Format(src string, opts FormatOptions) string {
    // 200 lines re-implementing, configurably, what gofmt already does
    // ...
}

// After — no knobs to argue about; one canonical format, enforced by tooling.
import "go/format"

func Format(src []byte) ([]byte, error) {
    return format.Source(src) // gofmt's canonical style; zero options
}

# ci.yml — formatting is now a machine's job, not a review topic.
- name: check formatting
  run: test -z "$(gofmt -l .)"   # fails the build on any unformatted file

Exercise 10 — Remove the dead flexibility (config that never varies)¶

Anti-pattern: Speculative Generality + Soft Coding. Goal: a function is drowning in "configurable" parameters that every caller passes with the same value. Inline the constants and remove the dead flexibility. Constraints: preserve output for the values actually in use; no caller passes anything different.

// Before — "flexible" retry helper; every call site passes identical knobs.
func FetchWithPolicy(
    url string,
    maxRetries int,
    backoffBase time.Duration,
    backoffFactor float64,
    jitter bool,
    timeout time.Duration,
    retryOn func(int) bool,
) ([]byte, error) {
    // honors every parameter...
}

// All 12 call sites, verbatim:
FetchWithPolicy(u, 3, 100*time.Millisecond, 2.0, true, 5*time.Second,
    func(code int) bool { return code >= 500 })

grep -rn "FetchWithPolicy(" --include="*.go" .
# all 12 calls pass: 3, 100ms, 2.0, true, 5s, and the same >=500 predicate.

// After — the policy that actually exists, named once. Knobs return only when a real need does.
const (
    maxRetries  = 3
    backoffBase = 100 * time.Millisecond
    timeout     = 5 * time.Second
)

func Fetch(url string) ([]byte, error) {
    // exponential backoff (base 100ms, ×2) + jitter, retry on 5xx, 5s timeout — inlined
    // ...
}

// call site:
Fetch(u)

Exercise 11 — Revert a cache nobody measured¶

Anti-pattern: Premature Optimization + Accidental Complexity. Goal: a memoization cache was bolted onto a cheap pure function "for performance," adding mutexes and a map but no measured benefit. Remove it and benchmark to confirm the simple version is as fast (often faster, since the cache had locking overhead). Constraints: identical results; thread-safe (the function is called concurrently).

// Before — a concurrency-safe cache wrapped around a trivial computation.
type SquareCache struct {
    mu    sync.Mutex
    cache map[int]int
}

func NewSquareCache() *SquareCache {
    return &SquareCache{cache: make(map[int]int)}
}

func (c *SquareCache) Square(n int) int {
    c.mu.Lock()
    defer c.mu.Unlock()
    if v, ok := c.cache[n]; ok {
        return v
    }
    v := n * n
    c.cache[n] = v
    return v
}

func BenchmarkSquareCacheParallel(b *testing.B) {
    c := NewSquareCache()
    b.RunParallel(func(pb *testing.PB) {
        for pb.Next() { _ = c.Square(7) }
    })
}
func BenchmarkSquareParallel(b *testing.B) {
    b.RunParallel(func(pb *testing.PB) {
        for pb.Next() { _ = Square(7) }
    })
}

// After — pure, allocation-free, trivially thread-safe (no shared state).
func Square(n int) int { return n * n }

name                  cache time/op   pure time/op   delta
Square-8 (parallel)      48.2ns ± 6%    0.31ns ± 1%   -99.4%  (mutex contention removed)

Exercise 12 — Collapse the generic-repository tower¶

Anti-pattern: Speculative Generality + Lasagna. Goal: a generic Repository<T> abstraction with an abstract base, a generic interface, and a per-entity subclass exists to serve one entity with one query. Collapse the tower to the concrete code that's actually used. Constraints: preserve findUser's behavior; the only consumer may be updated.

// Before — a four-type generic-repository framework serving exactly one entity.
interface Repository<T, ID> {
    Optional<T> findById(ID id);
    List<T> findAll();
    T save(T entity);
    void deleteById(ID id);
}

abstract class AbstractRepository<T, ID> implements Repository<T, ID> {
    protected abstract String table();
    protected abstract T map(ResultSet rs) throws SQLException;
    public Optional<T> findById(ID id) { /* generic reflective query */ }
    public List<T> findAll()           { /* generic */ }              // never called
    public T save(T entity)            { /* generic reflective insert */ } // never called
    public void deleteById(ID id)      { /* generic */ }              // never called
}

class UserRepository extends AbstractRepository<User, Long> {
    protected String table() { return "users"; }
    protected User map(ResultSet rs) throws SQLException {
        return new User(rs.getLong("id"), rs.getString("email"));
    }
}

// The only call anywhere:
Optional<User> u = userRepository.findById(42L);

// After — the one query that exists, written plainly.
class UserRepository {
    private final DataSource db;
    UserRepository(DataSource db) { this.db = db; }

    Optional<User> findById(long id) {
        User u = db.queryOne(
            "SELECT id, email FROM users WHERE id = ?", id,
            rs -> new User(rs.getLong("id"), rs.getString("email")));
        return Optional.ofNullable(u);
    }
}

Exercise 13 — The full combo simplification¶

Anti-pattern: all eight at once. Goal: one module exhibits a premature optimization, a speculative interface, a gold-plated option, a soft-coded rule, accidental framework complexity, a Lasagna hop, and a bikeshed-able knob. Simplify it in a safe order. Constraints: preserve the computed total; tackle the patterns from safest/most-isolated to most-entangled.

# Before — a "configurable, extensible, optimized" order-total calculator.
from abc import ABC, abstractmethod

# soft-coded tax rule (operators in data) + gold-plated knobs
CONFIG = {
    "rounding": "half_up",          # only "half_up" ever used
    "currency_symbol": "$",         # bikeshed magnet; logic doesn't need it
    "tax_rules": [{"if": {"field": "region", "op": "==", "val": "US"}, "then": 0.08},
                  {"else": 0.0}],
}

class TotalStep(ABC):               # speculative framework for two steps
    @abstractmethod
    def apply(self, ctx): ...

class SubtotalStep(TotalStep):
    def apply(self, ctx):
        # premature optimization: manual unroll of a sum
        items, s, i, n = ctx["items"], 0.0, 0, len(ctx["items"])
        while i < n - 1:
            s += items[i].price * items[i].qty
            s += items[i + 1].price * items[i + 1].qty
            i += 2
        while i < n:
            s += items[i].price * items[i].qty
            i += 1
        ctx["subtotal"] = s
        return ctx

class TaxStep(TotalStep):           # interprets the soft-coded rules
    def apply(self, ctx):
        rate = 0.0
        for rule in CONFIG["tax_rules"]:
            if "else" in rule: rate = rule["else"]; break
            if getattr(ctx["order"], rule["if"]["field"]) == rule["if"]["val"]:
                rate = rule["then"]; break
        ctx["tax"] = ctx["subtotal"] * rate
        return ctx

class Pipeline:                     # Lasagna: a runner that just loops
    def __init__(self, steps): self.steps = steps
    def run(self, ctx):
        for s in self.steps: ctx = s.apply(ctx)
        return ctx

def order_total(order):             # speculative seam: one impl, never mocked
    ctx = {"order": order, "items": order.items}
    ctx = Pipeline([SubtotalStep(), TaxStep()]).run(ctx)
    return round(ctx["subtotal"] + ctx["tax"], 2)

# After — the essential calculation, stated directly.
def subtotal(order) -> float:
    return sum(item.price * item.qty for item in order.items)

def tax(order, subtotal_amount: float) -> float:
    rate = 0.08 if order.region == "US" else 0.0
    return subtotal_amount * rate

def order_total(order) -> float:
    sub = subtotal(order)
    return round(sub + tax(order, sub), 2)

Simplification discipline — the recap¶

Every exercise above runs the same loop — note that "the change" here is almost always a removal:

green  →  one named removal/inline  →  green  →  commit  →  repeat
          (a public-API removal gets its OWN commit + deprecation note)

• Characterize before you simplify. A test that pins current observable behavior is the seatbelt; for a logic migration (Ex. 8, 13) a parallel run comparing old output to new is the characterization test. Without it, "simplifying" is just deleting and hoping.
• Small, reversible steps. Inline one class, run tests, commit. If green turns red, your last removal is the suspect — revert it. Git is the undo button, so deletion is cheap and recoverable the day a real need appears (YAGNI in action).
• Separate structural from behavioral commits. Simplification must preserve behavior; the canonical-formatter switch (Ex. 9) and any rounding change (Ex. 13) are deliberate behavioral deltas — give them their own reviewed commits, never smuggled into a structural one.
• Know what NOT to remove. A test-double seam, a volatile-dependency boundary, or a published contract is justified — inlining it is under-engineering (Ex. 3, and the "keep the seam" notes in Ex. 12). The shape (interface + one impl) never decides; the justification does.
• Public removals need a deprecation path. Trimming a gold-plated public API (Ex. 4) ships as deprecate-then-delete across releases with a DeprecationWarning and a CHANGELOG entry — never a silent breaking delete.
• Benchmark before claiming "faster." When you assert the simpler version costs nothing (Ex. 1, 5, 11), prove it with benchstat/timeit numbers. The recurring finding: abstraction isn't free — unrolling the compiler already does, caches that only add lock contention, and frameworks that add allocations all make the simple version as fast or faster.

Related Topics¶

• tasks.md — guided exercises building these simplification moves from scratch.
• find-bug.md — the spotting counterpart: identify the over-engineering, don't fix it.
• junior.md · middle.md · senior.md — recognize → calibrate the right altitude → simplify at scale.
• Bad Structure → optimize — the same refactoring discipline applied to under-structured code (the over-correction to avoid when collapsing layers is Spaghetti).
• Bad Shortcuts → middle — the Rule of Three and the wrong-abstraction trap; Soft Coding as the over-correction of Hard Coding.
• Refactoring → Techniques — the mechanical catalog for Inline Class, Inline Function, Collapse Hierarchy, Replace Subclass with Delegate, Remove Middle Man.
• Refactoring → Code Smells — Speculative Generality, Middle Man, and Lazy Element at the smell level.
• Clean Code → Classes — composition over inheritance; cohesion, the target state for Exercise 7.