Over-Engineering Anti-Patterns — Find the Bug¶

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

This file is critical-reading practice. Each snippet below is a plausible chunk of real-world code in Go, Java, or Python. Read it the way a sharp reviewer does and answer three questions:

What over-engineering anti-pattern(s) are present? What's the cost — or what bug does the over-engineering cause? How would you simplify or fix it?

Over-engineering is mostly "spot what's unnecessary" — flexibility, layers, and tuning the problem never asked for. But it is not harmless aesthetics. Over-engineering hides and causes real functional bugs, and several snippets here contain one: the hand-tuned "fast" loop is subtly wrong; the speculative abstraction's unused branch is broken; the soft-coded rule engine silently does the wrong thing; a fragile base class breaks a subclass; a Lasagna layer drops data on a hop. The clever version is often the buggy version, precisely because nobody can read it.

One snippet is a deliberate trap: code that looks over-engineered but whose abstraction is genuinely justified. Reflexively deleting abstraction is its own anti-pattern. The skill is judgment, not allergy.

How to use this file: read each snippet and write your own answer before expanding the collapsible. The skill you're training is noticing the shape — and resisting the easy "delete it all" reflex — not recalling the name.

Table of Contents¶

1. The fast even-sum that lost a number
2. A notifier ready for everything
3. The discount rules that moved to JSON
4. The price calculator chain
5. The report exporter that grew options
6. The account hierarchy and one small base change
7. The clock you can inject
8. A cached total that drifts
9. The generic repository with a flexible filter
10. Turning a map into a framework
11. The rounding that got clever
12. The plugin system for one plugin
13. The validation rule engine
14. The PR that polished the wrong thing

Snippet 1 — The fast even-sum that lost a number¶

// Go — someone "optimized" a hot loop by unrolling it four-wide.
// A benchmark exists; it's 6% faster. Ship it?
func sumEven(nums []int) int {
    s, i, n := 0, 0, len(nums)
    for ; i < n-3; i += 4 { // process 4 per iteration
        if nums[i]&1 == 0 {
            s += nums[i]
        }
        if nums[i+1]&1 == 0 {
            s += nums[i+1]
        }
        if nums[i+2]&1 == 0 {
            s += nums[i+2]
        }
        if nums[i+3]&1 == 0 {
            s += nums[i+3]
        }
    }
    // remainder loop
    for ; i <= n; i++ { // <-- handle the leftover elements
        if nums[i]&1 == 0 {
            s += nums[i]
        }
    }
    return s
}

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

func sumEven(nums []int) int {
    s := 0
    for _, n := range nums {
        if n%2 == 0 {
            s += n
        }
    }
    return s
}

Snippet 2 — A notifier ready for everything¶

// Java — the requirement was: "email the user when their order ships."
public interface NotificationChannel {
    void send(User u, Message m);
}
public interface MessageFormatter {
    Message format(String template, Map<String, Object> vars);
}
public interface ChannelSelector {
    NotificationChannel choose(User u, NotificationType type);
}

public class NotificationEngine {
    private final Map<NotificationType, ChannelSelector> selectors;
    private final Map<String, MessageFormatter> formatters;
    private final RetryPolicy retry;
    private final RateLimiter limiter;

    public void notify(User u, NotificationType type, String tmpl,
                       Map<String, Object> vars, String formatterKey) {
        NotificationChannel ch = selectors.get(type).choose(u, type);
        Message m = formatters.get(formatterKey).format(tmpl, vars);
        limiter.acquire(u.id());
        retry.run(() -> ch.send(u, m));
    }
}
// Only one call site in the whole codebase:
engine.notify(user, NotificationType.ORDER_SHIPPED,
              "Your order {{id}} shipped", vars, "default");

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

public class ShipmentNotifier {
    private final EmailClient email;
    ShipmentNotifier(EmailClient email) { this.email = email; }

    public void orderShipped(User u, Order o) {
        email.send(u.email(), "Your order " + o.id() + " shipped");
    }
}

Snippet 3 — The discount rules that moved to JSON¶

# Python — "so the business team can edit discounts without a deploy,"
# pricing logic was moved into a JSON rule engine loaded at startup.
RULES = json.load(open("discount_rules.json"))
# [
#   {"if": {"field": "tier", "op": "eq",  "val": "gold"},  "discount": 0.10},
#   {"if": {"field": "total","op": "gte", "val": 100},      "discount": 0.05},
#   {"if": {"field": "coupon","op": "eq", "val": "SAVE20"}, "discount": 0.20}
# ]

OPS = {
    "eq":  lambda a, b: a == b,
    "gte": lambda a, b: a >= b,
    "lte": lambda a, b: a <= b,
}

def discount_for(order):
    best = 0.0
    for rule in RULES:
        cond = rule["if"]
        op = OPS[cond["op"]]
        if op(order.get(cond["field"]), cond["val"]):
            best = max(best, rule["discount"])
    return best

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

GOLD_RATE, BIG_ORDER_RATE, COUPON_RATE = 0.10, 0.05, 0.20  # tunable constants

def discount_for(order):
    candidates = [0.0]
    if order.tier == "gold":           candidates.append(GOLD_RATE)
    if order.total >= 100:             candidates.append(BIG_ORDER_RATE)
    if order.coupon == "SAVE20":       candidates.append(COUPON_RATE)
    return max(candidates)             # "best discount" — explicit, testable

Snippet 4 — The price calculator chain¶

# Python — "clean layered architecture" for computing a line-item price.
class PriceController:
    def __init__(self, service): self.service = service
    def get_price(self, item_id, qty):
        return self.service.get_price(item_id, qty)

class PriceService:
    def __init__(self, manager): self.manager = manager
    def get_price(self, item_id, qty):
        return self.manager.compute(item_id, qty)

class PriceManager:
    def __init__(self, calc): self.calc = calc
    def compute(self, item_id, quantity):       # note: renamed qty -> quantity
        return self.calc.calculate(item_id, quantity)

class PriceCalculator:
    def __init__(self, repo): self.repo = repo
    def calculate(self, item_id, qty):
        unit = self.repo.unit_price(item_id)
        return unit * qty                       # the one line that does work

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

class PriceCalculator:
    def __init__(self, repo): self.repo = repo
    def price(self, item_id, qty):
        return self.repo.unit_price(item_id) * qty

Snippet 5 — The report exporter that grew options¶

# Ticket: "Export the user's transaction list to CSV."
# What landed in the PR:
def export_transactions(txns, path, *, fmt="csv", delimiter=",",
                        compress=False, encrypt=False, password=None,
                        email_to=None, watermark=None, columns=None,
                        locale="en_US", currency_symbol="$",
                        include_pending=True, sort_by="date", desc=False):
    rows = txns
    if not include_pending:
        rows = [t for t in rows if not t.pending]
    rows = sorted(rows, key=lambda t: getattr(t, sort_by), reverse=desc)
    if fmt == "csv":
        data = to_csv(rows, delimiter, columns, locale, currency_symbol)
    elif fmt == "xlsx":
        data = to_xlsx(rows, watermark)            # nobody asked for xlsx
    elif fmt == "pdf":
        data = to_pdf(rows, watermark)             # nobody asked for pdf
    if compress:
        data = gzip.compress(data)
    if encrypt:
        data = aes_encrypt(data, password)         # password may be None
    if email_to:
        send_email(email_to, data)
    else:
        Path(path).write_bytes(data)

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

def export_transactions_csv(txns, path):
    Path(path).write_text(to_csv(txns))

Snippet 6 — The account hierarchy and one small base change¶

// Java — a deep account hierarchy. A "small" base-class tweak was just merged.
abstract class Account {
    protected BigDecimal balance = BigDecimal.ZERO;
    void deposit(BigDecimal amt) { balance = balance.add(applyFee(amt)); }
    // NEW: base now skims a 1% processing fee on every deposit
    protected BigDecimal applyFee(BigDecimal amt) {
        return amt.multiply(new BigDecimal("0.99"));
    }
}
class SavingsAccount extends Account {
    void deposit(BigDecimal amt) { super.deposit(amt); accrueInterest(); }
    void accrueInterest() { balance = balance.multiply(rate()); /* ... */ }
}
class PromoSavingsAccount extends SavingsAccount {
    // promo accounts were always advertised as "0% fees, ever"
    @Override
    protected BigDecimal applyFee(BigDecimal amt) { return amt; } // override fee
    void deposit(BigDecimal amt) {
        balance = balance.add(amt);   // <-- bypasses super.deposit, adds raw amt
        accrueInterest();
    }
}

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

interface FeePolicy { BigDecimal net(BigDecimal amt); }
final class StandardFee implements FeePolicy {
    public BigDecimal net(BigDecimal a) { return a.multiply(new BigDecimal("0.99")); }
}
final class NoFee implements FeePolicy {
    public BigDecimal net(BigDecimal a) { return a; }
}

final class Account {
    private BigDecimal balance = BigDecimal.ZERO;
    private final FeePolicy fee;
    Account(FeePolicy fee) { this.fee = fee; }
    void deposit(BigDecimal amt) { balance = balance.add(fee.net(amt)); }
}
// Promo account = new Account(new NoFee()); the fee rule is data, not depth.

Snippet 7 — The clock you can inject¶

// Go — a token-expiry check. A reviewer flagged "this Clock interface is
// over-engineering — just call time.Now()." Are they right?
type Clock interface {
    Now() time.Time
}
type realClock struct{}
func (realClock) Now() time.Time { return time.Now() }

type TokenValidator struct {
    clock Clock
}
func (v TokenValidator) Valid(t Token) bool {
    return v.clock.Now().Before(t.ExpiresAt)
}

// production wiring:
validator := TokenValidator{clock: realClock{}}

// in tests:
func TestExpiredToken(t *testing.T) {
    fixed := fakeClock{at: time.Date(2030, 1, 1, 0, 0, 0, 0, time.UTC)}
    v := TokenValidator{clock: fixed}
    tok := Token{ExpiresAt: time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)}
    if v.Valid(tok) {
        t.Fatal("expected expired token to be invalid")
    }
}

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

Snippet 8 — A cached total that drifts¶

// Java — someone added a running-total cache "for performance" on a cart
// that holds at most ~20 items. No profile was taken.
public class Cart {
    private final List<Item> items = new ArrayList<>();
    private BigDecimal cachedTotal = BigDecimal.ZERO;   // perf optimization

    public void add(Item i) {
        items.add(i);
        cachedTotal = cachedTotal.add(i.price().multiply(qty(i)));
    }
    public void remove(Item i) {
        items.remove(i);
        cachedTotal = cachedTotal.subtract(i.price().multiply(qty(i)));
    }
    public void applyDiscount(BigDecimal pct) {
        for (Item i : items) {
            i.setPrice(i.price().multiply(BigDecimal.ONE.subtract(pct)));
        }
        // (cachedTotal not updated)
    }
    public BigDecimal total() { return cachedTotal; }
    private BigDecimal qty(Item i) { return new BigDecimal(i.quantity()); }
}

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

public class Cart {
    private final List<Item> items = new ArrayList<>();
    public void add(Item i)    { items.add(i); }
    public void remove(Item i) { items.remove(i); }
    public void applyDiscount(BigDecimal pct) {
        for (Item i : items)
            i.setPrice(i.price().multiply(BigDecimal.ONE.subtract(pct)));
    }
    public BigDecimal total() {           // one source of truth, can't drift
        return items.stream()
            .map(i -> i.price().multiply(new BigDecimal(i.quantity())))
            .reduce(BigDecimal.ZERO, BigDecimal::add);
    }
}

Snippet 9 — The generic repository with a flexible filter¶

// Java — a "future-proof" data layer so any query can be expressed generically.
public class GenericRepository<T> {
    public List<T> find(Class<T> type, Map<String, Object> criteria,
                        String orderBy, Integer limit, Boolean distinct,
                        List<String> joins, String having) {
        StringBuilder sql = new StringBuilder("SELECT ");
        sql.append(distinct != null && distinct ? "DISTINCT " : "");
        sql.append("* FROM ").append(table(type));
        for (String j : joins) sql.append(" JOIN ").append(j);
        if (!criteria.isEmpty()) {
            sql.append(" WHERE ");
            List<String> clauses = new ArrayList<>();
            for (Map.Entry<String, Object> e : criteria.entrySet()) {
                clauses.add(e.getKey() + " = '" + e.getValue() + "'"); // build WHERE
            }
            sql.append(String.join(" AND ", clauses));
        }
        if (having != null) sql.append(" HAVING ").append(having);
        if (orderBy != null) sql.append(" ORDER BY ").append(orderBy);
        if (limit != null)   sql.append(" LIMIT ").append(limit);
        return jdbc.query(sql.toString(), rowMapper(type));
    }
}
// The entire app uses it exactly two ways:
repo.find(User.class, Map.of("email", email), null, null, null, List.of(), null);
repo.find(Order.class, Map.of("userId", id), "createdAt", 50, null, List.of(), null);

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

public class UserRepository {
    public Optional<User> findByEmail(String email) {
        return jdbc.query("SELECT * FROM users WHERE email = ?",  // bound param
                          rowMapper(User.class), email).stream().findFirst();
    }
}
public class OrderRepository {
    public List<Order> recentForUser(long userId, int limit) {
        return jdbc.query(
            "SELECT * FROM orders WHERE user_id = ? ORDER BY created_at LIMIT ?",
            rowMapper(Order.class), userId, limit);
    }
}

Snippet 10 — Turning a map into a framework¶

# Python — the task: "uppercase each tag and drop blanks." One comprehension.
# What was committed: a configurable, extensible transformation pipeline.
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 DropBlankStep(Step):
    def apply(self, data): return [x for x in data if x.strip()]

class Pipeline:
    def __init__(self, steps=None):
        self.steps = steps or []
    def register(self, step): self.steps.append(step); return self
    def run(self, data):
        for s in self.steps:
            data = s.apply(data)
        return data

def normalize_tags(tags):
    return (Pipeline()
            .register(UpperStep())
            .register(DropBlankStep())
            .run(tags))

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

def normalize_tags(tags):
    return [t.upper() for t in tags if t and t.strip()]

Snippet 11 — The rounding that got clever¶

# Python — "fast" currency rounding to 2 decimals, hand-written to avoid
# the "slow" Decimal library. No benchmark was run.
def round_money(amount):
    # multiply, add 0.5 for rounding, truncate, divide — classic trick
    return int(amount * 100 + 0.5) / 100

# used everywhere money is displayed or stored:
total = round_money(price * quantity * (1 - discount))

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

from decimal import Decimal, ROUND_HALF_UP

def round_money(amount: Decimal) -> Decimal:
    return amount.quantize(Decimal("0.01"), rounding=ROUND_HALF_UP)

Snippet 12 — The plugin system for one plugin¶

// Go — a "plugin architecture" for image processing. There is one processor.
type Processor interface {
    Name() string
    Process(img Image, opts map[string]interface{}) (Image, error)
}

var registry = map[string]Processor{}

func Register(p Processor) { registry[p.Name()] = p }

func Run(name string, img Image, opts map[string]interface{}) (Image, error) {
    p := registry[name]
    return p.Process(img, opts) // look up by name, then process
}

// the only processor ever registered:
type Resizer struct{}
func (Resizer) Name() string { return "resize" }
func (Resizer) Process(img Image, opts map[string]interface{}) (Image, error) {
    w := opts["width"].(int)   // type assertion on an untyped map
    h := opts["height"].(int)
    return resize(img, w, h), nil
}

func init() { Register(Resizer{}) }

// caller:
out, _ := Run("resize", img, map[string]interface{}{"width": 800, "height": 600})

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

func Resize(img Image, width, height int) Image {
    return resize(img, width, height)
}
// caller:
out := Resize(img, 800, 600)   // compiler checks the name and the types

Snippet 13 — The validation rule engine¶

// Java — form validation, "made data-driven so rules live in config."
public class RuleEngine {
    // rules loaded from YAML: field, type, min, max, required, pattern
    private final List<Rule> rules;

    public List<String> validate(Map<String, String> form) {
        List<String> errors = new ArrayList<>();
        for (Rule r : rules) {
            String v = form.get(r.field);
            if (r.required && v == null) {
                errors.add(r.field + " is required");
                continue;
            }
            if (v == null) continue;
            switch (r.type) {
                case "int":
                    int n = Integer.parseInt(v);          // throws on bad input
                    if (n < r.min || n > r.max) errors.add(r.field + " out of range");
                    break;
                case "string":
                    if (!v.matches(r.pattern)) errors.add(r.field + " invalid");
                    break;
                // no default case
            }
        }
        return errors;
    }
}

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

public List<String> validate(SignupForm f) {
    List<String> errors = new ArrayList<>();
    if (f.email() == null || !EMAIL.matcher(f.email()).matches())
        errors.add("email invalid");
    if (f.age() < 13 || f.age() > 120)        // age is a typed int, parsed once at the edge
        errors.add("age out of range");
    return errors;
}

Snippet 14 — The PR that polished the wrong thing¶

# A PR titled "Improve user search." The diff:
#  +180 lines: a configurable, weighted, fuzzy-match scoring engine with
#              tunable weights per field, Levenshtein distance, and a
#              pluggable tokenizer — for a search box over ~500 users.
#  PR thread: 38 comments debating the default weight for the "email" field
#             (0.3 vs 0.35?), whether to call it `tokenizer` or `lexer`,
#             and 2-space vs 4-space indent in the new module.
#  Also in the diff (0 comments): the search query interpolates user input:
def search(term):
    q = f"SELECT * FROM users WHERE name LIKE '%{term}%'"   # <-- injection
    return db.execute(q)

What over-engineering anti-pattern(s) are here? What's the cost / what bug does it cause? How would you simplify/fix it?

def search(term):
    return db.execute(
        "SELECT * FROM users WHERE name LIKE ?",   # parameterized — no injection
        (f"%{term}%",),
    )

Summary — how to spot it without over-correcting¶

You don't spot over-engineering by recognizing one bad line — you spot it by comparing the code to the problem, and by asking what each abstraction, layer, or optimization actually earns. The repeatable moves from these fourteen snippets:

• Compare the code to a plain-English description of the problem. "Uppercase the tags and drop blanks" should not need an abstract Step class and a pipeline builder (Snippets 10, 4). The gap between the sentence and the machinery is Accidental Complexity — and it's removable.
• Demand a measurement before any optimization. Loop unrolling, hand-rolled rounding, and a running-total cache all appeared with no profile — and all three were not just pointless but wrong (out-of-bounds panic, off-by-a-cent money errors, a total that drifts after a discount). The "fast" version is the buggy version precisely because nobody can read it (Snippets 1, 8, 11).
• Count the real consumers of every abstraction — including tests. One plugin behind a registry, one notifier behind an engine, one query shape behind a generic repository: Speculative Generality whose unused branches are untested crash paths (Snippets 2, 9, 12). But one production impl behind an interface that a test injects is a justified seam — not over-engineering (Snippet 7). The diagnostic is purpose, not shape.
• Watch business logic migrating into data. Discount rules and validation rules pushed into JSON/YAML lose types, tests, and a debugger — and a config typo becomes a crash or a silent fail-open (Snippets 3, 13). Keep logic in code; configure only what genuinely varies per environment.
• Trace data and control across hops and inheritance levels. Lasagna layers that rename arguments drop or swap a parameter on a hop (Snippet 4); a deep hierarchy lets a base-class change silently break a subclass — the fragile base class / Yo-yo failure (Snippet 6).
• Match review attention to risk. When a PR is buried in comments about naming and default weights while a SQL injection goes unread, that's Bikeshedding stealing the attention the real bug needed (Snippet 14). Automate the trivia; aim human scrutiny at blast radius.

The meta-lesson cuts both ways. Over-engineering causes bugs, not just bloat — a plugin registry panics on a typo, a generic query builder opens an injection hole, a clever round drops cents. But the cure is judgment, not allergy: the same instinct that deletes a speculative interface will, applied blindly, delete the Clock seam your tests depend on. Ask "what real need does this serve today, and how expensive is it to add later?" — then keep what earns its place and cut what only insures an imagined future.

Related Topics¶

• junior.md — what each of the eight patterns looks like and why it's bad (YAGNI, KISS).
• middle.md — when over-engineering creeps in, the Rule of Three, and justified seams vs speculation.
• tasks.md — exercises that build the same muscles from the writing side.
• optimize.md — over-engineered implementations to simplify end-to-end.
• interview.md — Q&A across all levels.
• Bad Structure → find-bug and Bad Shortcuts → find-bug — the sibling find-bug files (Lasagna is the inverse of Spaghetti; Soft Coding the over-correction of Hard Coding).
• Clean Code → Classes — composition over inheritance, the cure for Yo-yo.
• Refactoring → Code Smells — Speculative Generality and Middle Man (Lasagna) at the smell level.
• Refactoring → Techniques — Remove Speculative Generality, Collapse Hierarchy, Inline Class, Replace Subclass with Delegate.