Bad Structure Anti-Patterns — Senior Level¶
Category: Development Anti-Patterns → Bad Structure — code that has grown into a shape that resists change. Covers (collectively): God Object · Spaghetti Code · Lava Flow · Boat Anchor · Arrow Anti-Pattern
Table of Contents¶
- Introduction
- Prerequisites
- How Did the Codebase Get Here? — Root-Cause Forces
- The Senior Refactoring Toolkit
- Dismantling a God Object at Scale
- Untangling Spaghetti: State Machines and Explicit Pipelines
- Lava Flow: Archaeology and Safe-Delete Campaigns
- Boat Anchor: Governing Speculative Abstraction
- Arrow: Replacing Conditional Towers Structurally
- When NOT to Refactor
- Preventing Structural Decay Organizationally
- Common Mistakes
- Test Yourself
- Cheat Sheet
- Summary
- Further Reading
- Related Topics
Introduction¶
Focus: How did the codebase get here? and How do I refactor safely at scale?
At the junior level you learned to recognize these five shapes; at the middle level you learned to reverse them with small, test-backed moves before they took root. This file is about the situation you actually inherit as a senior: the God Object is already 8,000 lines, it is on the critical path of revenue, three teams touch it weekly, and "stop the world and rewrite it" is not on the table.
Two questions define senior-level work here:
-
How did it get this way? Bad structure at scale is almost never an individual failing — it is the deterministic output of organizational forces: missing boundaries, Conway's law, deadline ratchets, and ownership gaps. If you refactor the code without addressing the force, the structure regrows. You are fighting entropy with a pump; if you don't fix the leak, the water comes back.
-
How do I change it without an outage? The answer is a set of disciplined techniques — seams, characterization tests, the Strangler Fig, branch-by-abstraction, parallel-change — that let you transform a load-bearing structure while it stays in production, in reversible steps, each one shippable on its own.
The senior mindset shift: the junior asks "is this code good?"; the senior asks "what is the cost of changing it, who pays that cost, and what is the smallest reversible step that lowers it?" You are no longer optimizing a file — you are managing risk and migrating a system that cannot stop.
Prerequisites¶
- Required: Fluency with
junior.mdandmiddle.md— you can recognize all five anti-patterns and apply Extract Class / guard clauses with a characterization test. - Required: You have shipped to production, owned an incident, and maintained code you did not write.
- Helpful: Working knowledge of feature flags / config-driven rollout, and a CI pipeline you can extend.
- Helpful: Familiarity with SOLID (especially SRP and Dependency Inversion) and Refactoring techniques.
- Helpful: Exposure to architecture tests / fitness functions (ArchUnit,
go-arch-lint, import-linter) — covered inprofessional.mdfor the runtime angle.
How Did the Codebase Get Here? — Root-Cause Forces¶
Every God Object has a biography. Before you touch a line, understand the forces that produced the shape, because the same forces will undo your refactor if they persist.
Missing boundaries¶
The single largest cause. When there is no agreed answer to "where does X belong?", X lands wherever it was first needed — and the first place is usually whatever class the author already had open. Email logic ends up in OrderService not because anyone decided it should, but because nobody decided it shouldn't. A God Object is the negative space left by absent module boundaries. Spaghetti is the negative space left by absent flow boundaries.
Conway's law¶
"Organizations design systems that mirror their own communication structure." — Melvin Conway, 1968
A single team that owns "everything orders-related" will produce a single orders module — and as the team's scope grows, so does the module, into a God Object. Conversely, a feature split across three teams with no shared owner produces Spaghetti at the seams, where each team bolts its path onto the others'. Structure follows org chart. This is why an inverse Conway maneuver — reshaping teams to match the architecture you want — is sometimes the real fix, and why a senior's refactoring plan often includes an ownership proposal.
The deadline ratchet¶
Each deadline justifies one local shortcut: add a flag instead of splitting the function, branch instead of extracting, leave the old path running "until we're sure." Individually rational, each shortcut is a click of a ratchet that only turns one way — toward more entanglement — because the cleanup is never itself a deadline. Spaghetti and Lava Flow are the ratchet's sediment.
Ownership gaps¶
Code with no clear owner gets added to but never shaped. Nobody has the standing — or the incentive — to say "no, that doesn't belong here." Orphaned modules become dumping grounds (God Object) and graveyards (Lava Flow) simultaneously.
Broken windows¶
"One broken window, left unrepaired, instills a sense of abandonment. So another window gets broken... and soon the building is damaged beyond the owner's desire to fix it." — Hunt & Thomas, The Pragmatic Programmer
Structure has a powerful social dimension. A 6-level-deep Arrow in a file signals "depth is acceptable here," and the next edit adds level 7. A 5,000-line class signals "size is fine here." Bad structure is contagious because it lowers the local standard. The corollary is the senior's lever: repairing the window — even partially — raises the standard and slows the spread.
The practical takeaway: a senior refactoring plan names the force, not just the smell. "Refactor OrderService" is a wish. "Establish a payments boundary, give the payments team ownership of it, strangle the payment methods out of OrderService, and add an architecture test that forbids re-importing payments into orders" is a plan that stays fixed.
The Senior Refactoring Toolkit¶
These five techniques are the load-bearing tools for changing structure that cannot stop. They compose: a real God Object migration uses all of them.
1. Seams (Feathers)¶
A seam is a place where you can alter behavior without editing in place — by substituting a collaborator. You create seams to get tests around legacy code before changing it. The most common seam is dependency injection at a constructor or parameter boundary; in languages without easy DI, a subclass-and-override seam or a link/build seam works.
2. Characterization tests¶
Before changing untested legacy code, write tests that capture what it does today — not what it should do. You are pinning current behavior so that your refactor can prove it preserved it. Bugs frozen by characterization tests get fixed later, in a separate, clearly-labeled commit. (Covered mechanically in middle.md; at scale, you often generate them by capturing real production inputs/outputs and replaying them — "golden master" testing.)
3. The Strangler Fig¶
Named after the fig that grows around a host tree until the host rots away and the fig stands on its own. You route a slice of traffic/calls through the new implementation while the old one still handles the rest, grow the new one feature by feature, and remove the old one only when nothing calls it. The system is never down; the old and new coexist behind a routing layer.
4. Branch-by-abstraction¶
For changing a component that the whole codebase depends on (you can't fork the call sites). You: (a) introduce an abstraction (interface) over the current implementation, (b) point all callers at the abstraction, (c) build the new implementation behind the same abstraction, (d) flip callers over, (e) delete the old implementation and, if it has served its purpose, the abstraction. Unlike a long-lived feature branch, the work happens on trunk and stays integrated the whole time.
5. Parallel-change (expand / migrate / contract)¶
The technique for evolving an interface without a flag-day break:
- Expand — add the new method/field/signature alongside the old one.
- Migrate — move callers over one at a time; both work during the transition.
- Contract — once no caller uses the old form, delete it.
This is how you change a method signature, a database column, or a public API across a large codebase (or across services) with zero coordinated downtime.
Dismantling a God Object at Scale¶
You inherit OrderService: 8,000 lines, 60 public methods, imported by 140 files, on the revenue path. A weekend rewrite would be a multi-month bug factory. Here is the disciplined sequence.
Step 0 — Map and triage, don't touch¶
Before any edit, build a picture:
- Responsibility map. Cluster the methods by the fields and external systems they touch (the cohesion analysis from
middle.md, now at scale). Payments methods cluster around the gateway client; notification methods cluster around the mailer; pricing clusters around the catalog. Each cluster is a candidate module. - Call-site and churn analysis. Which methods are called most, and which change most often (
git logchurn)? High-churn, high-fan-in clusters are where extraction pays off first; cold, stable clusters can wait. - Blast-radius assessment. Which extraction can be done with the fewest call-site changes? Start there to build momentum and trust.
Step 1 — Get a seam and characterization tests around the target cluster¶
Pick one cluster (say, notifications). Introduce a seam so it can be tested and later swapped.
# BEFORE — notifications welded into the God Object, untestable without SMTP.
class OrderService:
def place_order(self, order):
self._validate(order)
self._charge(order)
# ... 40 lines of SMTP, templating, retry inline ...
smtp = smtplib.SMTP(self.smtp_host)
smtp.sendmail(self.from_addr, order.email, self._render(order))
self._persist(order)
# STEP 1 — introduce a seam: a Notifier the God Object depends on, defaulted to
# the existing behavior. No external behavior changes yet; now it is injectable
# and therefore testable.
class Notifier(Protocol):
def order_placed(self, order: Order) -> None: ...
class SmtpNotifier: # the extracted-but-unchanged logic
def order_placed(self, order): ... # exactly what was inline before
class OrderService:
def __init__(self, notifier: Notifier | None = None):
self._notifier = notifier or SmtpNotifier() # default preserves behavior
def place_order(self, order):
self._validate(order)
self._charge(order)
self._notifier.order_placed(order) # delegate through the seam
self._persist(order)
Now write characterization tests against OrderService with a fake Notifier that records calls. You have pinned behavior and created an injection point — and you did it without changing what the system does in production. Ship this commit on its own.
Step 2 — Strangle one capability out via parallel-change¶
Move the ownership of the capability out of the God Object, one caller at a time. Suppose pricing should become its own Pricing service. Use branch-by-abstraction:
// EXPAND: introduce the abstraction over current behavior.
interface PriceCalculator {
Money priceOf(Order order);
}
// The legacy impl just calls back into the God Object — nothing moves yet.
class LegacyPriceCalculator implements PriceCalculator {
private final OrderService legacy;
public Money priceOf(Order o) { return legacy.calculatePriceInternal(o); }
}
// MIGRATE: callers depend on PriceCalculator, not OrderService.priceOf().
// Behind a flag, swap to the real extraction:
class PricingService implements PriceCalculator { // clean, owned by pricing team
public Money priceOf(Order o) { /* the real, extracted logic */ }
}
// CONTRACT: when traffic on PricingService is proven, delete
// LegacyPriceCalculator AND OrderService.calculatePriceInternal().
Each phase is a small, reversible, shippable PR. If PricingService misbehaves, flip the flag back to LegacyPriceCalculator — the rollback is a config change, not a deploy.
Step 3 — Feature-flag the cutover¶
Route a percentage of traffic through the new path. This is the Strangler Fig at runtime: the new service handles 1% → 10% → 50% → 100% as confidence grows, with the old path as instant fallback.
// Go — strangler routing behind a flag, with a kill switch.
func (h *OrderHandler) Price(o Order) (Money, error) {
if h.flags.Enabled("pricing.use_new_service", o.CustomerID) {
m, err := h.newPricing.PriceOf(o)
if err != nil {
h.metrics.Inc("pricing.new.error")
return h.legacyPricing.PriceOf(o) // fall back, alert, but stay up
}
return m, nil
}
return h.legacyPricing.PriceOf(o)
}
A powerful safety upgrade is dark launching / parallel run: call both implementations, return the old result, and log any divergence. You verify the new code on real production traffic before it serves a single real response.
func (h *OrderHandler) PriceWithShadow(o Order) (Money, error) {
legacy, err := h.legacyPricing.PriceOf(o)
go func() { // shadow call, result discarded, divergence logged
if shadow, e := h.newPricing.PriceOf(o); e == nil && shadow != legacy {
h.metrics.Inc("pricing.shadow.divergence")
h.log.Warn("pricing divergence", "order", o.ID, "legacy", legacy, "new", shadow)
}
}()
return legacy, err
}
Step 4 — Repeat, then delete the husk¶
Strangle clusters out one at a time — notifications, pricing, inventory, audit — each with its own seam, tests, flag, and cutover. When the last capability has moved and divergence counters read zero, OrderService is an empty coordinator. Delete it. The fig now stands without the tree.
The cardinal rule of God Object surgery: never let a refactoring branch live longer than a few days. Everything happens on trunk, behind abstractions and flags, integrated continuously. The big-bang rewrite branch is itself an anti-pattern — it diverges, conflicts, and is abandoned. (See
middle.mdCommon Mistakes.)
Untangling Spaghetti: State Machines and Explicit Pipelines¶
Spaghetti at scale is hidden temporal coupling: code that only works if A runs before B, communicated through shared mutable state rather than data flow. The senior fix makes the implicit sequence explicit and enforceable.
Make hidden temporal coupling impossible¶
The middle-level move was "pass data in, return data out." At scale, the structural fix is to model the state explicitly so illegal sequences cannot be expressed. Two tools dominate: explicit pipelines and finite state machines.
# BEFORE — temporal coupling: validate() must run before charge(), enforced by
# nothing but tribal knowledge. Calling charge() first silently corrupts state.
class Checkout:
def validate(self): self.ok = self._check(self.cart)
def charge(self): self._gateway.charge(self.cart) # forgot to check self.ok!
def confirm(self): self._email() # works only if charged
# AFTER — a pipeline: the order of steps is the structure of the code, not a
# rule in someone's head. Each stage consumes the previous stage's output type.
def checkout(cart: Cart) -> Receipt:
validated = validate(cart) # -> ValidatedCart (can't skip; type-gated)
charged = charge(validated) # -> ChargedCart (needs a ValidatedCart)
return confirm(charged) # -> Receipt (needs a ChargedCart)
By giving each stage a distinct return type that the next stage requires, you make "charge before validate" a compile-time (or at least obvious runtime) error. The temporal coupling has become a data dependency the type system enforces. This is the type-driven cousin of "make illegal states unrepresentable."
When flow is genuinely stateful: a finite state machine¶
Some spaghetti is irreducibly stateful — an order moves through Cart → Pending → Paid → Shipped → Delivered, with refunds, cancellations, and retries crisscrossing. Scattering this across if (order.paid && !order.shipped && order.refundRequested) checks is the spaghetti. Replace it with an explicit FSM where transitions are the only way state changes.
// Go — an explicit state machine. The allowed transitions are data, not
// scattered conditionals; an illegal transition is a single, central error.
type State string
const (
Cart State = "cart"
Pending State = "pending"
Paid State = "paid"
Shipped State = "shipped"
Cancelled State = "cancelled"
)
var transitions = map[State]map[Event]State{
Cart: {Submit: Pending},
Pending: {Pay: Paid, Cancel: Cancelled},
Paid: {Ship: Shipped, Refund: Cancelled},
Shipped: {}, // terminal
}
func (o *Order) Apply(e Event) error {
next, ok := transitions[o.State][e]
if !ok {
return fmt.Errorf("illegal transition: %s on %s", e, o.State) // one place
}
o.State = next
return nil
}
Every "can this happen now?" question is answered by one table instead of by reading the whole codebase. Adding a state is a row, not an archaeology dig. This is the single highest-leverage move against entrenched workflow spaghetti.
Refactoring spaghetti safely: wrap the existing tangled flow in a characterization test (capture real inputs → outputs), introduce the pipeline/FSM behind the same entry point, and use a shadow/parallel run to confirm the new flow produces identical outputs before cutting over. Same discipline as the God Object.
Lava Flow: Archaeology and Safe-Delete Campaigns¶
At scale, Lava Flow is measured in modules, and the deletion risk is real: some "dead" code is secretly load-bearing (a side effect, an import that registers a handler, a cron that nobody remembers). The senior approach is evidence-driven deletion under telemetry.
Archaeology: reconstruct the why¶
# What is this, who added it, and does the reason still exist?
git log --follow -p -- path/to/suspect.go # full history of the file
git log -S 'recalcLegacy' --oneline # commits that added/removed the symbol
git blame -w -C path/to/suspect.go # ignore whitespace + moved lines
The goal is to find the originating ticket. Nine times out of ten it is closed, its feature flag is long since true, and the "we kept it just in case" reason has expired.
Telemetry-driven dead-code removal¶
Static dead-code tools (deadcode/staticcheck in Go, vulture in Python, IDE "unused" inspections + ArchUnit in Java) find the statically unreachable. The dangerous Lava Flow is dynamically dead — reachable in theory, never hit in practice. Prove it with production telemetry before you delete.
// Instrument the suspect path. If this counter stays at zero across a full
// business cycle (a month, a quarter — long enough to cover rare paths),
// the code is dead with evidence, not hope.
func recalcLegacy(x int) int {
metrics.Inc("deadcode.recalcLegacy.hit") // canary
return oldFormula(x)
}
The safe-delete campaign¶
A senior runs deletion as a campaign, not a one-off:
- Instrument all suspects with hit-counters or structured logs; ship.
- Wait a full business cycle (rare paths fire at quarter-end, during incidents, for one big customer).
- Delete the proven-cold code in small, isolated commits — one logical unit per commit, message citing the evidence (
removed: 0 hits in 90d, flag X=true since 2021, ticket PROJ-451 closed). - Watch error rates and the canary dashboards after each deploy; the small commits make any "oops, it was load-bearing" trivially revertible.
The load-bearing trap. The scariest Lava Flow isn't called for its return value — it's called for a side effect: it warms a cache, registers a route, mutates a global, or its mere
importruns aninit(). Hit-counters on the function miss these. Defend by also asserting on outcomes (does removing it change any test's behavior? any metric?) and by deleting behind a flag/dark-launch when the side effect is plausible. Never delete a whole module in one commit; the bisect surface is too large.
Boat Anchor: Governing Speculative Abstraction¶
At the senior level, Boat Anchors are not just unused classes — they are unused capabilities you are contractually obligated to maintain: a public API method nobody calls, a config flag with no consumer, a "pluggable" interface with one implementation invented for a future that never arrived. The senior job is governance: stopping new anchors from being cast, and safely cutting away existing ones.
Stop the casting: a "needs a ticket" gate¶
The cheapest control is social and lives in review (and in your [definition of done]). When a PR adds capability "for the future" — a third export format, a strategy interface with one strategy, a generic plugin system — the reviewer asks: "What ticket needs this now?" No ticket → defer. This is YAGNI as an enforced norm, not a slogan. The cost of adding the abstraction the day a real second case appears is almost always lower than the cost of carrying it unused (and you'll design it better against a real requirement).
Deprecating unused public API¶
Internal Boat Anchors you just delete. Public ones — anything outside your blast radius can depend on — need a deprecation process, because deletion is a breaking change for someone you can't see.
// 1. Mark it, point to the replacement, and (crucially) instrument it so you
// learn whether anyone actually calls it.
@Deprecated(since = "4.2", forRemoval = true)
public Report exportXml(Report r) {
deprecationMetrics.increment("api.exportXml"); // who still uses this?
return legacyXmlExport(r);
}
The deprecation lifecycle: announce (changelog, deprecation header, compiler warning) → instrument (count real callers) → wait a published window (a major version, a quarter) → remove only after the counter is zero or remaining callers are migrated. This is parallel-change applied to a public contract: expand the replacement, migrate callers, contract the old form — but the "migrate" step now includes external consumers and therefore a communication plan, not just a code change. See API versioning / deprecation.
The discipline: a Boat Anchor is unused code that invites dependence. Every day it survives, the odds rise that someone wires into it — converting a cheap deletion into an expensive migration. Govern the inflow (no speculative abstraction without a ticket) and run a periodic outflow (deprecate-and-remove unused public surface).
Arrow: Replacing Conditional Towers Structurally¶
Guard clauses (junior) and polymorphism/handler-maps (middle) handle most Arrows. The senior addition is how to make the structural replacement safely in a hot, branchy core — and a guiding principle for the order of operations.
"Make the change easy, then make the easy change"¶
"For each desired change, make the change easy (warning: this may be hard), then make the easy change." — Kent Beck
You almost never flatten an Arrow and add the new feature in one step. You first refactor the conditional tower into a dispatch structure (the hard, behavior-preserving part), confirm green, then the new case is a one-line addition (the easy part). Conflating the two is how "add a feature" becomes a regression.
Conditional tower → table-driven dispatch¶
The most entrenched Arrows are a switch/if-else ladder on a type or code, often duplicated across several functions (price and tax and shipping all switch on product.kind). The structural fix is to make the dispatch data.
# BEFORE — the same arrow ladder, copy-pasted into three functions.
def price(p):
if p.kind == "book": return base(p)
elif p.kind == "ebook": return base(p) * 0.9
elif p.kind == "bundle": return sum(price(c) for c in p.children)
# ... and a parallel ladder in tax() and in ship() ...
# AFTER — table-driven: each kind's rules live in one row; dispatch is a lookup.
# Adding a "subscription" kind is ONE new row, not edits to three ladders.
@dataclass
class KindRules:
price: Callable[[Product], Money]
taxable: bool
ships_physically: bool
RULES: dict[str, KindRules] = {
"book": KindRules(price=lambda p: base(p), taxable=True, ships_physically=True),
"ebook": KindRules(price=lambda p: base(p) * 0.9, taxable=False, ships_physically=False),
"bundle": KindRules(price=lambda p: sum(price(c) for c in p.children),
taxable=True, ships_physically=True),
}
def price(p): return RULES[p.kind].price(p)
def taxable(p): return RULES[p.kind].taxable
When the rules carry behavior and data and the set is open-ended, promote each row to a polymorphic type (Replace Conditional with Polymorphism, Strategy / State patterns). The key senior judgment: a table for closed, data-like variation; polymorphism for open, behavior-rich variation; guard clauses for validation nesting. Don't reach for a class hierarchy where a map literal would do — that's how you turn an Arrow into Lasagna.
When NOT to Refactor¶
The senior skill that juniors lack entirely: knowing when bad structure is the right structure to leave alone. Refactoring is an investment; it pays off only when the code will be read and changed again. Decline to refactor when:
- The code is frozen / sunset. A module slated for decommission next quarter, or a feature behind a flag that's being turned off, is throwing-good-effort-after-bad. Let it die ugly.
- It's stable and cold. Code that hasn't changed in two years and has no upcoming work has already amortized its structure cost. An ugly function called once at startup, never touched, costs almost nothing. Reserve effort for high-churn, high-fan-in hot spots — the Pareto frontier of
git logchurn × blast radius. - You lack a safety net you can't build. If there are no tests, no way to get a seam, and no telemetry, a structural change to revenue-critical code is a coin flip. Sometimes the honest answer is "freeze it, build observability first, refactor next quarter."
- The risk/cost exceeds the benefit right now. During an incident, a launch freeze, or a quarter where the team has zero slack, the correct move is to record the debt (a tracked ticket, a
// TODO(DEBT-123)with a link) and schedule it — not to start surgery on the critical path under pressure. - The "refactor" is really a rewrite in disguise. If your plan can't be sliced into reversible, shippable steps on trunk, it's a rewrite, and rewrites of load-bearing systems fail far more often than they're sold. Find the strangler slices, or don't start.
The frame: refactoring is not virtue, it's risk-adjusted investment in future change-cost. No future change → no return. A senior can articulate the cost of inaction — "every payments feature pays a 3-day tax to navigate this God Object, and we ship ~20 a quarter" — which is what turns "the code is ugly" into a funded, prioritized initiative.
Preventing Structural Decay Organizationally¶
Refactoring removes today's bad structure; prevention stops it regrowing. Since the root causes are organizational (Conway, ownership, deadline ratchet, broken windows), the durable fixes are organizational and automated — they outlast the engineer who cares.
Fitness functions and architecture tests¶
Encode your boundaries as executable tests in CI, so a violation fails the build instead of relying on a reviewer noticing. This is the antidote to "the boundary I refactored in eroded six months later."
// Java — ArchUnit: the payments boundary you fought to establish is now
// enforced by the build. Re-importing payments into orders fails CI.
@ArchTest
static final ArchRule payments_isolation =
noClasses().that().resideInAPackage("..orders..")
.should().dependOnClassesThat().resideInAPackage("..payments.internal..");
@ArchTest
static final ArchRule no_god_objects =
classes().should(haveFewerThanXMethods(40)) // a smoke alarm, not a law
.as("classes should not accrete unbounded responsibilities");
// Go — a layering rule via go-arch-lint / depguard, checked in CI.
// deps:
// orders: [catalog, shared] # may NOT import payments.internal
// payments: [shared]
Other useful fitness functions: max cyclomatic complexity / nesting depth as a ratcheting gate (new code must not exceed N; the threshold only tightens), a dead-code check that fails on new statically-unreachable code, and a "no new files in god_package" rule. The ratchet metaphor cuts both ways — make quality the thing that only ratchets up.
Ownership: CODEOWNERS¶
Every directory must have an owner. CODEOWNERS makes it mechanical — the right team is auto-requested on every PR, closing the ownership gap root cause. Orphaned code is where God Objects and Lava Flows breed.
# .github/CODEOWNERS — no module is an orphan.
/payments/ @org/payments-team
/orders/ @org/orders-team
/shared/ @org/platform-team
* @org/eng-leads # catch-all so nothing is unowned
Review norms and the broken-windows defense¶
- Small, scoped PRs. A 40-line PR rarely hides a God Object; a 2,000-line one routinely does. Cap PR size by norm.
- "Leave it better" boy-scout rule, bounded. Each PR may repair one window in code it touches — but separated into its own commit, never mixed with the feature (the
middle.mdrule). This slowly reverses broken-windows decay without unbounded scope creep. - Refactoring on the roadmap. The deadline ratchet only turns one way unless cleanup is itself scheduled. Seniors get structural work funded by quantifying its change-cost tax, then put it on the sprint board with the same weight as features.
- Architecture Decision Records. When you establish a boundary ("payments must not depend on orders"), write down why in an ADR. Six months later, the engineer tempted to cross it reads the rationale instead of re-learning it the hard way.
The senior's real product is not the refactor — it's the system that keeps the refactor from un-happening: an enforced boundary, an owner, a fitness function, and a team norm. Code rots back to its org chart; change the forces, and the structure holds.
Common Mistakes¶
Mistakes seniors make when refactoring bad structure at scale:
- The long-lived rewrite branch. "We'll rebuild it cleanly on a branch and merge in three months." It diverges, conflicts daily, never merges, and is abandoned. Always strangle on trunk behind abstractions and flags.
- Refactoring the symptom, not the force. You split the God Object beautifully; six months later it has regrown because the missing boundary and ownership gap were never addressed. Name the force; add a fitness function and an owner.
- Deleting Lava Flow without telemetry. Static "unreachable" is not the same as "never executes." Side-effect-only code (
init(), route registration, cache warming) looks dead and is load-bearing. Instrument, wait a full business cycle, delete in small reversible commits. - Mixing structural and behavioral change in one commit. When the deploy breaks, you can't tell whether the extraction or the bug-fix did it, and you can't cleanly revert. Separate, always — including the bug a characterization test froze.
- Refactoring frozen or cold code. Spending a sprint cleaning a module that ships next quarter, or a startup function touched once a year. Spend the budget on high-churn, high-fan-in hot spots; leave cold/dying code alone.
- Treating fitness-function thresholds as truth. A hard "max 40 methods" gate produces a 41st class that's a pass-through (Lasagna), gaming the metric. Metrics are smoke alarms that route attention; humans judge.
- Refactoring without a rollback path. Cutting over a strangler with no flag and no fallback turns a bad day into an outage. Every cutover needs an instant kill switch (flag flip), ideally preceded by a shadow/parallel run.
- Selling the refactor as virtue instead of risk-adjusted investment. "The code is ugly" doesn't get funded. Quantify the change-cost tax and the cost of inaction; that's what buys you the time.
Test Yourself¶
- You inherit a 9,000-line
BillingManageron the revenue path, imported by 200 files, with no tests. Outline the first three concrete steps you take, and explain why a clean-rewrite branch is the wrong move. - Explain branch-by-abstraction and how it differs from a feature branch. At which step do you delete the old implementation?
- A workflow has spaghetti of the form
if (order.paid && !order.shipped && !order.refunded && order.inventoryReserved)scattered across twelve functions. What structural replacement do you reach for, and what does it make impossible? - You want to delete a suspicious 300-line module. A static analyzer says it's unreachable, but you hesitate. What is the specific danger the static analyzer can miss, and how do you defend against it before deleting?
- Give two situations where the correct senior decision is not to refactor a clearly badly-structured module.
- You've split a God Object successfully. What two organizational mechanisms do you put in place so it doesn't regrow, and which root cause does each address?
- Apply Beck's "make the change easy, then make the easy change" to flattening a duplicated
switchladder before adding a new product type. What are the two distinct commits?
Answers
1. **(a)** Map responsibilities by field/system cohesion + analyze churn and fan-in to pick the highest-leverage, lowest-blast-radius cluster first. **(b)** Introduce a *seam* (inject a collaborator, defaulted to current behavior) around that cluster — no behavior change — and write *characterization tests* through it. **(c)** Strangle that one capability out via branch-by-abstraction behind a feature flag, with a shadow/parallel run, then cut over a small traffic %. A clean-rewrite branch is wrong because it lives for months, diverges and conflicts, can't be shipped incrementally, has no rollback granularity, and historically gets abandoned — all change must stay integrated on trunk. 2. Branch-by-abstraction: introduce an interface over the current implementation, point all callers at it, build the new implementation behind the same interface, flip callers, then delete the old impl (and possibly the abstraction). Unlike a feature branch, the work happens **on trunk**, staying continuously integrated. You delete the old implementation in the **contract** step, only after callers are flipped and the new path is proven. 3. A **finite state machine** with an explicit transition table (and/or a typed pipeline where each stage returns a distinct type the next requires). It makes *illegal transitions impossible to express* — the scattered boolean checks collapse into one central transition lookup; "can this happen now?" is answered by a table, and adding a state is a row, not an archaeology dig. 4. The danger: code that is *statically* reachable but *dynamically* dead, OR code called only for a **side effect** (an `init()`, route/handler registration, cache warming, a global mutation) — removing it changes behavior even though its return value is unused, and a static "unreachable" check or a return-value hit-counter both miss it. Defense: instrument with a hit-counter *and* watch outcome metrics, wait a full business cycle, delete behind a flag/dark-launch when a side effect is plausible, and never delete a whole module in one commit (keep the bisect/rollback surface small). 5. Any two of: the module is **frozen/sunset** (decommissioned soon — wasted effort); it's **stable and cold** (no upcoming change, so no return on the investment); you **lack and can't build a safety net** on critical code (the change is a coin flip — build observability first); you're **under a freeze/incident/zero-slack** window (record the debt and schedule it instead); or the "refactor" can't be sliced into reversible trunk steps and is really a **rewrite in disguise**. 6. Any two, e.g.: **a fitness/architecture test in CI** (e.g., ArchUnit/`go-arch-lint` rule forbidding the re-import you just untangled) — addresses *missing boundaries* by making them executable; **`CODEOWNERS` assigning the new module an owner** — addresses *ownership gaps* so it's shaped, not just added to; **scheduling refactoring on the roadmap** — addresses the *deadline ratchet*; **the bounded boy-scout rule in review** — addresses *broken windows*. 7. **Commit 1 (make the change easy — the hard part):** behavior-preservingly refactor the duplicated `switch` ladders into one table-driven dispatch (a `RULES` map / polymorphic types), with characterization tests proving identical behavior. **Commit 2 (make the easy change):** add the new product type as a single new row/class. The two are separate so that if the new type misbehaves, it's unambiguous which change caused it, and the refactor can be verified independently.Cheat Sheet¶
| Anti-pattern at scale | Root-cause force | Senior refactoring move | Safety mechanism |
|---|---|---|---|
| God Object | Missing boundary + Conway + ownership gap | Seam → characterize → strangle clusters via branch-by-abstraction | Feature flag + shadow run; trunk-only, small PRs |
| Spaghetti | Deadline ratchet + shared mutable state | Explicit typed pipeline; finite state machine with transition table | Characterization/golden-master + parallel run before cutover |
| Lava Flow | Ownership gap + fear (uncertainty) | Archaeology (git log -S, blame) + telemetry-driven safe-delete campaign | Hit-counters + full business cycle + small reversible commits |
| Boat Anchor | Speculative over-prep ("for the future") | YAGNI gate in review; deprecate-and-remove unused public API | Deprecation window + caller telemetry + parallel-change |
| Arrow | Broken windows + per-case if accretion | Table-driven dispatch / polymorphism; "make the change easy, then easy change" | Refactor and feature-add in separate commits |
Three golden rules: - Name the force, not just the smell — fix the org cause or the structure regrows. - Change load-bearing structure on trunk, in reversible steps, behind flags and shadow runs — never on a long-lived rewrite branch. - Refactoring is risk-adjusted investment in future change-cost; no future change (frozen/cold/sunset) → don't.
Summary¶
- How it got here: bad structure at scale is the deterministic output of organizational forces — missing boundaries, Conway's law, the deadline ratchet, ownership gaps, and broken windows. A refactor that ignores the force is a pump on an un-fixed leak.
- The toolkit: seams + characterization tests to make legacy code changeable; the Strangler Fig, branch-by-abstraction, parallel-change, and feature flags to transform load-bearing structure while it stays in production, in small reversible steps on trunk.
- God Object: map → seam → characterize → strangle one cluster at a time via branch-by-abstraction behind a flag, with shadow runs; delete the husk last. Never a long-lived rewrite branch.
- Spaghetti: kill hidden temporal coupling with typed pipelines (illegal order won't type-check) and finite state machines (illegal transitions can't be expressed); cut over behind a parallel run.
- Lava Flow: archaeology + telemetry-driven safe-delete campaigns, waiting a full business cycle and watching for load-bearing side effects; delete in small, evidence-citing commits.
- Boat Anchor: govern the inflow with a YAGNI "needs-a-ticket" review gate; cut the outflow with a real deprecation process for unused public API.
- Arrow: table-driven dispatch for closed variation, polymorphism for open behavior; make the change easy, then make the easy change — in separate commits.
- When not to: frozen/sunset, stable/cold, no buildable safety net, freeze windows, or a disguised rewrite. Refactoring is risk-adjusted investment; no future change → no return.
- Prevention is organizational: fitness functions / architecture tests in CI, CODEOWNERS, ratcheting quality gates, ADRs, small PRs, and a bounded boy-scout rule. The senior's real deliverable is the system that keeps the structure from un-happening.
- Next:
professional.md— the performance, GC, runtime, and toolchain implications of these shapes and their fixes.
Further Reading¶
- Working Effectively with Legacy Code — Michael Feathers (2004) — seams, characterization tests, breaking dependencies in tangled, test-less code. The senior's primary text here.
- Refactoring — Martin Fowler (2nd ed., 2018) — Branch by Abstraction, Parallel Change, Replace Conditional with Polymorphism, Replace Type Code with Subclasses/Strategy.
- Building Evolutionary Architectures — Ford, Parsons, Kua (2nd ed., 2022) — fitness functions and architecture tests as executable governance.
- Monolith to Microservices — Sam Newman (2019) — the Strangler Fig and parallel-run patterns applied to decomposing large systems.
- Refactoring at Scale — Maude Lemaire (2020) — running multi-month structural migrations on a live, team-shared codebase.
- The Pragmatic Programmer — Hunt & Thomas (20th anniv. ed., 2019) — broken windows, reversibility, orthogonality.
- martinfowler.com — "StranglerFigApplication," "BranchByAbstraction," "ParallelChange" articles (canonical short references).
Related Topics¶
- Clean Code → Classes — SRP and cohesion, the target state for a dismantled God Object.
- Refactoring → Refactoring Techniques — the mechanical moves: Extract Class, Replace Conditional with Polymorphism, Branch by Abstraction.
- Refactoring → Code Smells — Large Class, Long Method, Shotgun Surgery at the smell level.
- Design Patterns → Strategy / State — the positive counterparts to type-switch Arrows and stateful spaghetti.
- Over-Engineering — the failure mode of over-applying these fixes: Lasagna, Speculative Generality, YAGNI.
- Bad Shortcuts — the sibling category; making illegal states unrepresentable connects the two.
- Architecture → SOLID & System Design — boundaries, Dependency Inversion, and the Conway's-law / inverse-Conway angle.
In this topic
- junior
- middle
- senior
- professional