Simple Design — Find the Design Smell¶

Category: Craftsmanship Disciplines — Kent Beck's four rules for writing code that is no more complicated than it needs to be, in strict priority order.

12 snippets that violate simple design. Unlike a normal "find-bug," most of these compile and pass their tests — the defect is in the design, not the behavior. Spot the smell, then expand the fix and the lesson. Each names the rule it violates.

Table of Contents¶

1. Smell 1: One-Implementation Interface
2. Smell 2: Hidden (Knowledge) Duplication
3. Smell 3: Intention-Obscuring Names
4. Smell 4: DRYing Coincidental Similarity
5. Smell 5: Speculative Configuration
6. Smell 6: The Wrong Abstraction (Flag Soup)
7. Smell 7: Pass-Through / Forwarding Layer
8. Smell 8: Premature Extraction at Two Occurrences
9. Smell 9: Simplistic (Drops a Required Case)
10. Smell 10: Element-Count Minimalism Hurting Clarity
11. Smell 11: YAGNI Violation at a Reversible Decision
12. Smell 12: Under-Engineering at a One-Way Door
13. Practice Tips

Smell 1: One-Implementation Interface¶

type UserFormatter interface{ Format(u User) string }

type DefaultUserFormatter struct{}
func (DefaultUserFormatter) Format(u User) string { return u.First + " " + u.Last }

func label(f UserFormatter, u User) string { return f.Format(u) }
// DefaultUserFormatter is the ONLY implementation; label is called once.

Rule violated: 4 (fewest elements) + YAGNI.

Fix¶

func label(u User) string { return u.First + " " + u.Last }

Lesson¶

No interface without a second real implementation (or a present test-seam requirement). Go's structural typing lets you add the interface later without touching the concrete type — so deferring costs nothing.

Smell 2: Hidden (Knowledge) Duplication¶

def gross_invoice(net):  return net + net * 0.20    # VAT rule, copy 1
def display_total(net):  return net + net * 0.20    # VAT rule, copy 2
def audit_amount(net):   return net + net * 0.20    # VAT rule, copy 3

Rule violated: 3 (no duplication).

Fix¶

VAT_RATE = 0.20
def with_vat(net): return net + net * VAT_RATE      # one home for the rule

def gross_invoice(net): return with_vat(net)
def display_total(net): return with_vat(net)
def audit_amount(net):  return with_vat(net)

Lesson¶

Duplicated knowledge (a single business rule in multiple places) is the dangerous kind — it can drift apart. Give the rule one home. Verify it's the same rule first (here it is: all three compute VAT).

Smell 3: Intention-Obscuring Names¶

List<X> proc(List<X> l) {
    List<X> r = new ArrayList<>();
    for (X x : l) if (x.s() == 1) r.add(x);
    return r;
}

Rule violated: 2 (reveals intention).

Fix¶

List<Order> activeOrders(List<Order> orders) {
    return orders.stream()
                 .filter(o -> o.status() == Status.ACTIVE)
                 .toList();
}

Lesson¶

Names are design. status() == 1 → status() == Status.ACTIVE also kills connascence-of-meaning (a magic value two sides must agree on). Reveals-intention is the second rule — above DRY — for exactly this reason: clarity is the higher value.

Smell 4: DRYing Coincidental Similarity¶

def discount_rate(): return 0.15
def commission_rate(): return 0.15

# A teammate "DRYs" them:
RATE = 0.15
def discount_rate():   return RATE
def commission_rate(): return RATE     # now coupled!

Rule violated: 3 — misapplied (manufactured coupling).

Fix¶

DISCOUNT_RATE = 0.15
COMMISSION_RATE = 0.15     # same value now, free to diverge later

def discount_rate():   return DISCOUNT_RATE
def commission_rate(): return COMMISSION_RATE

Lesson¶

DRY targets duplicated knowledge, not duplicated values. Test: would a change to one force the same change to the other? No → coincidence → keep separate. Over-zealous DRY creates the very coupling it's supposed to prevent.

Smell 5: Speculative Configuration¶

public class ReportGenerator {
    private final int maxRows;
    private final boolean useCache;
    private final String dateFormat;
    private final Charset charset;
    private final int retryCount;
    // All five read from config; in production every one has a single fixed value
    // and no requirement to vary. Five knobs nobody turns.
}

Rule violated: 4 (fewest elements) + YAGNI.

Fix¶

public class ReportGenerator {
    private static final int MAX_ROWS = 10_000;
    private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ISO_DATE;
    // Hard-code the single real value. Make a knob configurable the day
    // someone actually needs to configure it.
}

Lesson¶

Configurability is a cost: more surface, more code paths, more to test. Add a config knob when a present requirement needs the value to vary — not "in case."

Smell 6: The Wrong Abstraction (Flag Soup)¶

def build_message(user, kind, urgent=False, html=False, short=False, lang="en"):
    if kind == "welcome":   txt = greet(user, lang)
    elif kind == "reset":   txt = reset_link(user)
    elif kind == "alert":   txt = ("!! " if urgent else "") + alert_text(user)
    if short: txt = txt[:140]
    return wrap_html(txt) if html else txt
# Six callers, each using a different subset of the flags; almost no shared logic.

Rule violated: 2 + 3 (a "DRY" abstraction that obscures and couples).

Fix¶

def welcome_message(user, lang="en"): return greet(user, lang)
def reset_message(user):              return reset_link(user)
def alert_message(user, urgent=False):
    return ("!! " if urgent else "") + alert_text(user)
# Each caller calls the specific one. wrap_html()/truncation move to the
# one or two callers that actually need them.

Lesson¶

When an abstraction grows flags to serve divergent callers, it's the wrong abstraction. Inline it back (re-introduce duplication), simplify each caller, and re-extract only genuinely shared knowledge. Duplication is cheaper than the wrong abstraction.

Smell 7: Pass-Through / Forwarding Layer¶

class OrderService {
    private final OrderRepository repo;
    Order find(long id)      { return repo.find(id); }      // just forwards
    void save(Order o)       { repo.save(o); }              // just forwards
    void delete(long id)     { repo.delete(id); }           // just forwards
    List<Order> all()        { return repo.all(); }         // just forwards
}
// OrderService adds NO behavior — every method is a one-line delegation.

Rule violated: 4 (fewest elements).

Fix¶

// Delete OrderService; callers use OrderRepository directly.
// Reintroduce a service ONLY when it has real behavior to add
// (transactions, validation, orchestration across repositories).

Lesson¶

A layer earns its place by adding behavior. Pure pass-through layers are needless elements — often added because "a service layer is best practice." Best practice is the behavior, not the empty class.

Smell 8: Premature Extraction at Two Occurrences¶

# Only TWO call sites exist, but someone extracted a "flexible" helper:
def format_line(value, prefix="", suffix="", pad=0, sep=":", upper=False):
    s = f"{prefix}{value}{sep}{suffix}".ljust(pad)
    return s.upper() if upper else s

a = format_line(total_a, prefix="Cart A ", suffix="$")
b = format_line(total_b, prefix="Cart B ", suffix="$")

Rule violated: 3 — misapplied (extracted too early, wrong shape).

Fix¶

# At two occurrences, keep it inline and concrete:
a = f"Cart A: ${total_a}"
b = f"Cart B: ${total_b}"
# Extract a small, well-shaped helper when a THIRD occurrence reveals
# what's truly invariant.

Lesson¶

The rule of three exists precisely to prevent this. Two points can't define the abstraction's real shape; extracting early over-parameterizes for imagined variation. Tolerate the duplication until the third case.

Smell 9: Simplistic (Drops a Required Case)¶

def average(numbers):
    return sum(numbers) / len(numbers)     # "simple!" ... but:

Rule violated: 1 (passes all the tests) — simplistic ≠ simple.

Fix¶

def average(numbers):
    if not numbers:
        raise ValueError("average of an empty sequence is undefined")
    return sum(numbers) / len(numbers)

Lesson¶

Simple removes the needless, never the necessary. Dropping a required case to shorten code fails Rule 1, the highest rule — so it can't be simple. Simplicity is "smallest thing that's still correct," not "shortest thing."

Smell 10: Element-Count Minimalism Hurting Clarity¶

def p(o):
    return sum(i.q*i.up for i in o.it)*(1+(0.2 if o.tx else 0))-(o.d or 0)

Rule violated: 2 (reveals intention) — minimalism taken too far.

Fix¶

def order_total(order):
    subtotal = sum(item.qty * item.unit_price for item in order.items)
    taxed = subtotal * (1 + TAX_RATE) if order.taxable else subtotal
    return taxed - (order.discount or 0)

Lesson¶

"Fewest elements" means fewest concepts to understand, not fewest characters. A clear five-line function is simpler than a cryptic one-liner. Rule 4 is a tiebreaker bounded above by Rule 2 — clarity always wins.

Smell 11: YAGNI Violation at a Reversible Decision¶

// Building a generic plugin system for what is, today, ONE report type.
interface ReportPlugin { Report generate(Params p); }
class PluginRegistry { /* register, discover, load, version plugins... */ }
class ReportEngine { /* runs plugins via the registry... */ }
// There is exactly one report. No second report is a requirement.

Rule violated: 4 + YAGNI (speculative architecture).

Fix¶

class SalesReport {
    Report generate(Params p) { /* the one report, directly */ }
}
// No plugin system. If a 2nd report type becomes real, extract a small
// interface THEN — shaped by two concrete reports.

Lesson¶

This is internal and reversible, so YAGNI governs: defer the abstraction. (Contrast Smell 12 — the opposite mistake on an irreversible decision.)

Smell 12: Under-Engineering at a One-Way Door¶

// "Keep it simple": persistence details inlined throughout the domain.
func (s *OrderService) Place(o Order) error {
    _, err := s.db.Exec("INSERT INTO orders (id, total) VALUES ($1, $2)",
        o.ID, o.Total)                       // raw SQL in the domain layer
    return err
}
// The same raw-SQL pattern is repeated across ~200 domain files.

Rule violated: under-engineering — misapplying "fewest elements" to a one-way door.

Fix¶

// A repository seam — a present, justified element because the storage
// boundary is irreversible and a migration is foreseeable.
type OrderRepository interface{ Save(o Order) error }

func (s *OrderService) Place(o Order) error { return s.repo.Save(o) }

Lesson¶

"Fewest elements" is bounded by reversibility. YAGNI applies to reversible decisions; for one-way doors (schema, storage, public API, protocol) a seam is cheap insurance. Crushing necessary seams to win on element count is under-engineering — the symmetric opposite of gold-plating.

Practice Tips¶

1. Count implementations of each interface — one implementation, no test-seam need → delete the interface.
2. Search for repeated business rules/constants (a rate, a formula) — that's knowledge duplication; give it one home.
3. Before merging look-alikes, ask would a change to one force the same change to the other? If no, it's coincidence — don't merge.
4. Flag every abstraction without a present requirement — "might need it later" is a YAGNI violation.
5. Spot flag soup — a function with many boolean/mode parameters serving divergent callers is the wrong abstraction; inline and re-split.
6. Find pass-through layers — classes/methods that only forward calls add no behavior; remove them.
7. Watch for "simplistic" — short code that drops a required case fails Rule 1; it isn't simple.
8. Watch for one-liners that hurt clarity — Rule 4 never overrides Rule 2.
9. Check reversibility — missing seams at one-way doors are under-engineering, not simplicity.

← Tasks · Craftsmanship Disciplines · Roadmap · Next: Optimize