Refactoring and Code Smells — The 12 You Will Actually Meet

Fowler’s Refactoring (2nd ed, 2018 — rewritten in JavaScript, but the ideas transfer directly to Java) is the definitive catalog. This file distills the smells you will meet in real enterprise Java, the refactoring moves that fix them, and the study-worthy way to talk about them. If your day job includes a 15-year-old Spring monolith, this is your survival guide.

The Twelve Smells

#	Smell	One-line reading	Refactoring move
1	God Class	One class doing everything	Extract Class, Extract Method
2	Feature Envy	Method uses another class’s data more than its own	Move Method
3	Primitive Obsession	String and int for domain concepts	Replace Primitive with Value Object
4	Long Parameter List	5+ params, easy to swap	Introduce Parameter Object
5	Shotgun Surgery	One change touches many classes	Move Method / Move Field / Inline Class
6	Data Clumps	Same 3+ params appear together everywhere	Extract Class
7	Dead Code	Unreachable or unused	Delete it — Git remembers
8	Comment as Deodorant	Comment explains bad code	Extract Method with a good name
9	Speculative Generality	Framework for imagined future needs	Inline Class / Collapse Hierarchy
10	Message Chains	a.getB().getC().getD().doIt()	Hide Delegate
11	Middle Man	Class only delegates, adds nothing	Remove Middle Man
12	Refused Bequest	Subclass ignores parent’s methods	Replace Inheritance with Delegation

Each smell is a diagnostic tool, not a moral failing. You will write some yourself under deadline. The goal is to notice, name, and fix.

---

1. God Class

One class does everything: parses input, calls the database, formats output, sends emails. You see it when scrolling past line 500 in the same file. Also called Blob or Large Class.

Symptoms: file > 500 lines, > 20 methods, mixed abstraction levels (SQL string literals next to business rules next to HTML formatting).

Fix: Extract Class along cohesion boundaries — group methods that share fields. Move each group to its own class. What remains is a coordinator, ideally under 100 lines.

// Before: 800-line OrderService doing everything
class OrderService {
    void placeOrder(...) { validateInputs(...); calculatePricing(...);
                            saveToDb(...); sendConfirmationEmail(...);
                            updateInventory(...); notifyWarehouse(...); }
    // 30 more methods
}

// After: coordinator + collaborators
class OrderService {
    private final OrderValidator validator;
    private final PricingEngine pricing;
    private final OrderRepository repo;
    private final NotificationService notifications;
    private final InventoryClient inventory;

    void placeOrder(OrderRequest req) {
        validator.validate(req);
        var priced = pricing.price(req);
        var saved  = repo.save(priced);
        inventory.reserve(saved);
        notifications.confirm(saved);
    }
}

2. Feature Envy

A method in class A reaches into class B for data more than it uses A’s own data:

// Feature envy — method belongs on Order, not on ReportGenerator
class ReportGenerator {
    String format(Order order) {
        return order.getCustomer().getName() + " " +
               order.getCustomer().getEmail() + " " +
               order.getTotal().toString();
    }
}

Fix: Move Method. Put format() on Order (or on a dedicated formatter that takes a small view of Order, not the whole customer graph).

3. Primitive Obsession

You pass around String userId, String email, String currency, int amountInCents. Nothing prevents you from swapping them or passing invalid values.

// Bad: what stops someone passing email as userId?
void transfer(String fromUserId, String toUserId, int amountInCents, String currency);

// Better: value objects that carry meaning and validation
void transfer(UserId from, UserId to, Money amount);

record UserId(String value) {
    public UserId { if (!value.matches("U-\\d{8}")) throw new IllegalArgumentException(); }
}
record Money(BigDecimal amount, Currency currency) { /* ... */ }

Fix: Replace Primitive with Value Object. Records make this cheap in modern Java.

4. Long Parameter List

User createUser(String firstName, String lastName, String email,
                String phone, LocalDate dob, String country,
                String state, String city, String zipCode);

Fix: Introduce Parameter Object. Group related fields:

User createUser(PersonName name, ContactInfo contact, Address address);

Rule of thumb: if 3+ parameters always travel together (a Data Clump), they are a class waiting to be born.

5. Shotgun Surgery

Adding a currency requires editing 27 files across the codebase. This is the mirror image of God Class — responsibility is smeared thin instead of piled up.

Fix: Move Method / Move Field to consolidate the scattered logic into one class or module. Sometimes Inline Class merges two anemic types.

How to detect: after your last three commits, how many files did you touch per logical change? If it is always >10, you have Shotgun Surgery.

6. Data Clumps

// This trio appears in 40 methods
void ship(String street, String city, String zip) { ... }
void bill(String street, String city, String zip) { ... }
void validate(String street, String city, String zip) { ... }

Fix: Extract Class into Address. Once the class exists, methods migrate to it (Feature Envy fix).

7. Dead Code

An if (false) branch. A private method with no callers. A field never read. A whole class only referenced from tests that themselves test nothing.

Fix: Delete. Git remembers. Do not comment out — that just creates archaeological noise.

Modern IDEs (IntelliJ inspections, mvn dependency-check, unimported) find most of it automatically. Run them monthly.

8. Comment as Deodorant

// Calculate the discount if the customer is premium and has been
// a member for over a year and the order is over $500
if (customer.isPremium() && customer.membershipYears() > 1 && order.total().compareTo(new BigDecimal("500")) > 0) {
    discount = order.total().multiply(new BigDecimal("0.10"));
}

The comment exists because the code is unreadable. Fix: Extract Method with a name that says what the comment said:

if (qualifiesForLoyaltyDiscount(customer, order)) {
    discount = order.total().multiply(new BigDecimal("0.10"));
}

Exceptions: comments that explain why (business context, historical decisions, non-obvious constraints) are gold. Comments that explain what are usually smells.

9. Speculative Generality

// One implementation. Ever. But we built the abstraction "just in case".
interface UserRepositoryFactory {
    UserRepository create(UserRepositoryConfig config);
}
interface UserRepositoryConfig { /* one method */ }
class DefaultUserRepositoryFactory implements UserRepositoryFactory { ... }

Built for imaginary future needs that never came. Fix: Inline Class, Collapse Hierarchy. Delete the interface with one implementation. If a second implementation appears, extract the interface then.

The honest rule: extract abstraction on the second concrete case, not the first.

10. Message Chains

order.getCustomer().getAddress().getCity().getCountry().getName();

Every . is a coupling. If Address restructures, this chain breaks. Also called “train wreck.”

Fix: Hide Delegate. Ask the object for what you want, not for its collaborators:

order.customerCountryName();  // Order asks Customer asks Address… internally

The Law of Demeter says talk only to your immediate friends. Streams and Optional often make chains legitimate (stream().filter().map().collect()); domain object chains are the bad kind.

11. Middle Man

class Manager {
    private final Employee employee;
    public String getName()      { return employee.getName(); }
    public String getEmail()     { return employee.getEmail(); }
    public String getPhone()     { return employee.getPhone(); }
    // 20 more pure delegations
}

Manager adds nothing. Fix: Remove Middle Man — callers talk to Employee directly. Or find the missing behavior that would justify Manager and add it.

12. Refused Bequest

A subclass inherits methods it does not want and overrides them to throw UnsupportedOperationException or empty out the body. Classic in JDK: Collections.unmodifiableList().add() throws — the unmodifiable list “refuses” the mutation methods of List.

Fix: Replace Inheritance with Delegation. If Stack “is not really a” Vector (its API subset), stop extending. Wrap and expose only the intended surface.

This is closely tied to LSP violations — refused bequest is where inheritance lied about substitutability.

---

⚠️ What Most People Get Wrong

They attempt heroic multi-day refactors. Then production breaks, the diff review is unreviewable, and management bans refactoring for the next quarter. The correct approach is the Boy Scout Rule (leave the code cleaner than you found it) applied in tiny commits: rename one variable, extract one method, add one test. Each commit is independently shippable. Fowler calls this refactoring on the margins of feature work — you refactor to enable the feature you are already writing, not as a separate project.

The second common mistake: refactoring without tests. If you cannot verify behavior stayed the same, you are rewriting, not refactoring. Before you touch smelly code, pin its current behavior with characterization tests — tests that document “whatever it does now, this is it.” Michael Feathers’ Working Effectively with Legacy Code is the definitive text on this.

Fowler’s Core Moves (Cheatsheet)

Move	When to use
Extract Method	Method too long, or a comment names a sub-step
Inline Method	Method body is clearer than its name
Extract Class	Two responsibilities in one class
Inline Class	Class does too little
Move Method	Method uses another class’s data more than its own
Rename Variable/Method	Name lies or is unclear
Introduce Parameter Object	Data clump in signatures
Replace Conditional with Polymorphism	switch on type across many methods
Replace Inheritance with Delegation	Subclass refuses bequest
Encapsulate Field	Direct field access from outside

IntelliJ automates every one of these safely. Learn the shortcuts — they pay back within a week.

What to Practice This Week

1. Pick one class > 300 lines in your codebase. List its responsibilities. Extract the smallest coherent one into a new class in one commit. Ship it.

2. Find a message chain of length ≥ 3 in production code. Hide the delegate. Watch the test suite.

3. Find a comment that explains what code does. Extract Method with a name matching the comment; delete the comment.

4. Read Fowler Refactoring 2nd ed chapter 3 (Bad Smells) — the taxonomy is exhaustive. This file is just the greatest hits.

---

Return to README.md · Previous: 04_records_sealed_pattern_matching.md · Next: projects.md