Skip to content

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

java
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.

java
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

code
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    stock

Each 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.