31 · 32 · 6 min
Breaking up the class that does everything
Most real codebases contain one. A class of two thousand lines that grew a feature at a time, that everything depends on, and that nobody wants to touch.
Recognising it
public class OrderManager {
public void createOrder(...) // creation
public void calculateTax(...) // tax rules
public void applyDiscounts(...) // pricing
public void chargeCard(...) // payments
public void sendConfirmation(...) // email
public void updateInventory(...) // stock
public void generateInvoicePdf(...) // documents
public void exportToAccounting(...) // integration
}The tells: the name contains Manager, Helper, Util or Processor; it changes for unrelated reasons; a test requires half the system; and describing it needs the word "and" several times.
That last one is the single responsibility principle stated usefully. A class should have one reason to change, and this has eight.
Do not rewrite it
The instinct is to start again. It is also the approach that fails, because the class encodes years of accumulated cases that nobody remembers and no document records. A rewrite loses them one at a time, in production.
The safe sequence
1. Get tests around it first. Characterisation tests that record what it does now, correct or not. You are not fixing behaviour; you are pinning it down so a refactor cannot change it silently.
2. Find the clusters. Group methods by which fields they touch. Fields used by only three of twenty methods, together, are a class trying to get out.
3. Extract one at a time.
public class TaxCalculator {
public Money taxFor(Order order, Region region) { ... }
}
public class OrderManager {
private final TaxCalculator tax; // delegate, do not delete yet
public Money calculateTax(Order o) { return tax.taxFor(o, o.region()); }
}The old method stays and delegates, so every caller keeps working. Nothing outside changes in this step, which is what makes it safe to ship.
4. Move the callers over, gradually.
5. Delete the delegating method once nothing calls it.
Five small deployments, each individually harmless and reversible. This is the expand and contract pattern from the software engineering course, applied to code instead of a schema.
What good looks like afterwards
OrderService orchestrates, holds no rules
TaxCalculator tax rules, pure, trivially testable
PricingEngine discounts
PaymentGateway an interface, with a real and a fake implementation
NotificationSender an interface
InventoryService stockEach is testable alone, each has one reason to change, and each can be understood without the others.
When to leave it
If it is stable, rarely changed and works, the refactor may not pay for itself. The case for doing it is that it changes often and every change is expensive and risky. Refactoring is an investment, and it needs a return.