Refactoring and Code Smells

Writing Maintainable Software

Neil Ernst

2025-11-06

What is Refactoring?

Martin Fowler’s Definition

“Refactoring is a controlled technique for improving the design of an existing code base. Its essence is applying a series of small behavior-preserving transformations.”

Key Principles:

• Small, incremental changes
• Tests verify behavior unchanged
• Improve structure, not functionality

Code Smells

What are they?

Surface indications that usually correspond to deeper problems in the system

Think: Warning signs, not bugs

Important Context

• Not always a problem
• Context matters
• Indicate where to look
• Not what’s wrong

Code Smell #1: Long Method

Methods that do too much

Warning Signs

• Hard to understand
• Hard to reuse
• Hard to test
• Rule of thumb: If you need to scroll, it’s probably too long

// Smell: 150-line method doing calculation, validation,
// database access, and formatting
public void processUserRequest() {
    // ... 150 lines of code ...
}

Code Smell #2: Large Class

Class trying to do too much

• Violates Single Responsibility Principle
• Too many instance variables
• Too many methods
• Often a “God Class”

// Smell: UserManagerDatabaseValidatorEmailerLogger
public class UserManager {
    // 50 instance variables
    // 100 methods
}

Code Smell #3: Duplicate Code

Same code structure in multiple places

// In UserController.java
if (email == null || !email.contains("@")) {
    throw new ValidationException("Invalid email");
}

// In RegistrationService.java
if (email == null || !email.contains("@")) {
    throw new ValidationException("Invalid email");
}

// In ProfileUpdater.java
if (email == null || !email.contains("@")) {
    throw new ValidationException("Invalid email");
}

Warning

Changes must be made in multiple locations - error prone!

Code Smell #4: Long Parameter List

Methods with many parameters

// Smell: 7 parameters!
public void createUser(String name,
                       String email,
                       String phone,
                       String address,
                       String city,
                       String zip,
                       String country) {
    // ...
}

• Hard to understand
• Hard to call correctly
• Often indicates missing abstraction

Code Smell #5: Primitive Obsession

Using primitives instead of small objects

Smell

String phoneNumber = "555-1234";
String email = "invalid-email";
double money = 19.99;
String date = "2024-13-45";

// No type safety!
// No validation!
// No domain meaning!

Better

PhoneNumber phone =
    new PhoneNumber("555-1234");

Email email =
    new Email("user@example.com");

Money price =
    Money.dollars(19.99);

LocalDate date =
    LocalDate.parse("2024-03-15");

Code Smell #6: Feature Envy

Method more interested in another class

// In Order class - but mostly manipulates Customer data
public double calculateDiscount() {
    if (customer.getAge() > 65) {
        return customer.getBaseDiscount() *
               customer.getLoyaltyMultiplier() *
               customer.getSeniorBonus();
    }
    return customer.getBaseDiscount() *
           customer.getLoyaltyMultiplier();
}

Tip

This method envies the Customer class - maybe it belongs there?

Beyond Code: Architectural Smells

Code smells are local problems - but what about system-level issues?

Architectural Smells

Structural problems in how modules/components relate to each other

• Harder to detect (need system view)
• More expensive to fix
• Greater impact on maintainability

Dependency Structure Matrix (DSM)

Concept (Carliss Baldwin)

A matrix showing dependencies between system modules

• Rows/Columns = Modules
• X = Module A depends on Module B
• Reveals hidden complexity

Example DSM

	UI	DB	Auth	Utils
UI	-	X	X	X
DB		-		X
Auth	X	X	-	X
Utils	X	X	X	-

Warning

Problem: Everyone depends on everyone! (Cyclic dependencies)

DSM Analysis: Identifying Smells

What to Look For

1. Entries above diagonal = Cyclic dependencies (feedback loops) {.fragment}
2. Dense rows = Module depends on too many others (high coupling) {.fragment}
3. Dense columns = Module depended on by many (ripple effects) {.fragment}
4. Scattered X’s = Poor modularity {.fragment}

. . .

Goal: Triangular Matrix

	Utils	DB	Auth	UI
Utils	-
DB	X	-
Auth	X	X	-
UI	X	X	X	-

. . .

Lower-level modules at top, high-level at bottom

Architectural Smell #1: Cyclic Dependencies

When modules form dependency cycles

// Module A
import B;
class A {
    B helper = new B();
}

// Module B
import C;
class B {
    C processor = new C();
}

// Module C
import A;  // ← Creates cycle!
class C {
    A coordinator = new A();
}

Problems

• Can’t compile/deploy independently
• Changes ripple through entire cycle
• Hard to understand flow

Architectural Smell #2: God Module

One module that everyone depends on

DSM View

	Utils	Core	UI	DB
Utils	-
Core	X	-
UI	X	X	-
DB	X	X		-

Core is a bottleneck

Problems

• Changes to Core break everything
• Core becomes complex
• Can’t parallelize development
• Testing nightmare

Similar to: God Class, but at architecture level

Architectural Smell #3: Unstable Dependencies

Stable modules depending on unstable ones

Stability Metrics (Robert Martin)

Instability = Efferent / (Afferent + Efferent)

• Efferent = Outgoing dependencies (what I depend on)
• Afferent = Incoming dependencies (who depends on me)
• Ranges from 0 (stable) to 1 (unstable)

The Smell

Stable module (low instability) → Depends on → Unstable module (high instability)

Example: Core business logic depending on experimental UI framework

Architectural Smell #4: Shotgun Surgery

Single change requires modifications across many modules

Example

Adding a new field “customerType”:

1. Database schema
2. Data access layer
3. Business logic
4. UI validation
5. Reporting module
6. Export module
7. API layer
8. …15 files total

Root Cause

Feature scattered across architecture

DSM Indicator: - Dense columns - No clear boundaries

Tip

Solution: Refactor to Feature-based or Layer-based modules

DSM Example: Before vs After

Before (Tangled)

	A	B	C	D
A	-	X	X	X
B	X	-	X	X
C	X		-	X
D		X		-

Cycles, high coupling

After (Layered)

	D	C	B	A
D	-
C	X	-
B	X	X	-
A	X	X	X	-

Clean hierarchy

How?

1. Identify strongly connected components
2. Break cycles (Dependency Inversion)
3. Reorder modules by level

Breaking Architectural Smells

Techniques

1. Dependency Inversion - Introduce interfaces/abstractions {.fragment}
2. Extract Module - Pull out cohesive functionality {.fragment}
3. Move Functionality - Relocate to reduce coupling {.fragment}
4. Introduce Mediator - Centralize complex interactions {.fragment}
5. Split Module - Separate concerns {.fragment}

. . .

Key Insight from Baldwin

Modularity has economic value

• Good architecture → Easier parallel development
• Clean dependencies → Lower change cost
• Visible structure → Better decisions

Tools for Architectural Analysis

DSM Tools

• Lattix - Commercial DSM tool
• Structure101 - Architecture visualization
• JDepend / NDepend - Dependency metrics

. . .

Manual Analysis

# Find circular dependencies
jdeps --dot-output . myapp.jar

# Calculate coupling metrics
java -jar metricsgarden.jar --analyze .

. . .

Tip

Practice: Draw DSM for your current project - what do you see?

Code vs Architecture Smells

	Code Smells	Architectural Smells
Scope	Methods, classes	Modules, components
Impact	Local readability	System-wide maintainability
Detection	Code review, IDE	DSM, metrics, tools
Fix Cost	Hours to days	Days to weeks
Fix Tools	Extract method, rename	Restructure modules, add layers

Note

Both are important!

• Fix code smells continuously
• Monitor and fix architectural smells regularly

Refactoring Techniques

Golden Rule

Always have tests before refactoring!

Refactoring #1: Extract Method

When: Code fragment that can be grouped together

Before

void printOwing() {
    printBanner();

    // print details
    System.out.println("name: " + name);
    System.out.println("amount: " +
        getOutstanding());
}

After

void printOwing() {
    printBanner();
    printDetails(getOutstanding());
}

void printDetails(double outstanding) {
    System.out.println("name: " + name);
    System.out.println("amount: " +
        outstanding);
}

Benefits: Readability, reuse, testability

Refactoring #2: Rename Variable/Method

When: Name doesn’t reveal its purpose

Before

int d; // elapsed time in days

double calc(double x, double y) {
    return x * y * 0.1;
}

After

int daysSinceLastUpdate;

double calculateDiscount(
    double price,
    double quantity) {
    return price * quantity *
        DISCOUNT_RATE;
}

Note

Code is read 10x more than it’s written

Refactoring #3: Extract Variable

When: Complex expression is hard to understand

Before

if ((platform.toUpperCase().indexOf("MAC") > -1) &&
    (browser.toUpperCase().indexOf("IE") > -1) &&
    wasInitialized() && resize > 0) {
    // do something
}

. . .

After

final boolean isMacOs = platform.toUpperCase().indexOf("MAC") > -1;
final boolean isIEBrowser = browser.toUpperCase().indexOf("IE") > -1;
final boolean wasResized = resize > 0;

if (isMacOs && isIEBrowser && wasInitialized() && wasResized) {
    // do something
}

Refactoring #4: Replace Magic Number

When: Literal numbers with unclear meaning

Before

double potentialEnergy(
    double mass,
    double height) {

    return mass * 9.81 * height;
}

// What is 9.81?

After

static final double
    GRAVITATIONAL_CONSTANT = 9.81;

double potentialEnergy(
    double mass,
    double height) {

    return mass *
        GRAVITATIONAL_CONSTANT *
        height;
}

Refactoring #5: Introduce Parameter Object

When: Parameters naturally go together

Before

double calculateDistance(double x1, double y1,
                        double x2, double y2) {
    return Math.sqrt(Math.pow(x2 - x1, 2) +
                     Math.pow(y2 - y1, 2));
}

. . .

After

class Point {
    final double x, y;
    Point(double x, double y) { this.x = x; this.y = y; }
}

double calculateDistance(Point start, Point end) {
    return Math.sqrt(Math.pow(end.x - start.x, 2) +
                     Math.pow(end.y - start.y, 2));
}

Refactoring #6: Replace Conditional with Polymorphism

When: Conditional behavior based on object type

// Before: Type checking everywhere
if (employee.type == ENGINEER) {
    return engineer.getSalary() * 1.2;
} else if (employee.type == MANAGER) {
    return manager.getSalary() * 1.5;
}

// After: Use inheritance/interfaces
interface Employee {
    double calculateBonus();
}
// Each type implements its own logic

When to Refactor?

The Rule of Three (Beck’s Rule)

1. First time: Just do it
2. Second time: Wince at duplication, but do it anyway
3. Third time: REFACTOR

Good Times to Refactor

• Before adding a feature - Clean up to make it easier
• While reviewing code - Improve understanding
• When fixing bugs - Often indicates unclear code
• During code review - Pair refactoring

When NOT to Refactor

• Code needs complete rewrite from scratch {.fragment}
• Close to a deadline (schedule dedicated time instead) {.fragment}
• Tests don’t exist (write tests first!) {.fragment}

Benefits of Refactoring

1. Improves design over time {.fragment}
2. Makes code easier to understand {.fragment}
3. Helps find bugs {.fragment}
4. Helps program faster (cleaner code = faster development) {.fragment}

Exercise Time!

Task: Identify code smells and suggest refactorings

Time: 5 minutes individual work, then discussion

Exercise Code

public class OrderProcessor {
    public void processOrder(String custName, String custEmail,
                            String custPhone, String item1,
                            double price1, int qty1, String item2,
                            double price2, int qty2) {

        // Calculate total
        double t = price1 * qty1 + price2 * qty2;

        // Apply discount
        double d = 0;
        if (t > 100) {
            d = t * 0.1;
        } else if (t > 50) {
            d = t * 0.05;
        }
        t = t - d;

        // Add tax
        t = t * 1.13;

Exercise Code (continued)

        // Print receipt
        System.out.println("=============================");
        System.out.println("Customer: " + custName);
        System.out.println("Email: " + custEmail);
        System.out.println("Phone: " + custPhone);
        System.out.println("-----------------------------");
        System.out.println(item1 + " x " + qty1 + " = $" +
            (price1 * qty1));
        System.out.println(item2 + " x " + qty2 + " = $" +
            (price2 * qty2));
        System.out.println("Discount: $" + d);
        System.out.println("Tax: $" + (t - (t / 1.13)));
        System.out.println("-----------------------------");
        System.out.println("TOTAL: $" + t);
        System.out.println("=============================");

        // Send email
        System.out.println("Sending email to " + custEmail);
    }
}

Exercise Questions

1. What code smells can you identify? {.fragment}
2. What refactorings would you apply? {.fragment}
3. How would you break this into smaller methods? {.fragment}

Hint: Look for at least 6 smells!

Code Smells Identified

1. Long Method - Does calculation, validation, formatting, I/O {.fragment}
2. Long Parameter List - 9 parameters! {.fragment}
3. Primitive Obsession - Strings for customer, items {.fragment}
4. Magic Numbers - 100, 50, 0.1, 0.05, 1.13 {.fragment}
5. Duplicate Code - Item calculation repeated {.fragment}
6. Poor Naming - t, d single-letter variables {.fragment}
7. Feature Envy - Method doing everything about multiple concepts {.fragment}

Refactoring Strategy

Extract Classes

class Customer { String name, email, phone; }
class OrderItem { String name; double price; int quantity; }
class Order { Customer customer; List<OrderItem> items; }

. . .

Extract Constants

static final double TAX_RATE = 0.13;
static final double HIGH_DISCOUNT = 0.1;
static final double LOW_DISCOUNT = 0.05;
static final double HIGH_THRESHOLD = 100.0;
static final double LOW_THRESHOLD = 50.0;

Refactored Solution

Extract Methods

public void processOrder(Order order) {
    double subtotal = calculateSubtotal(order);
    double discount = calculateDiscount(subtotal);
    double total = calculateTotal(subtotal, discount);

    Receipt receipt = new Receipt(order, subtotal, discount, total);
    printReceipt(receipt);
    sendConfirmationEmail(order.getCustomer(), receipt);
}

private double calculateSubtotal(Order order) {
    return order.getItems().stream()
        .mapToDouble(item -> item.getPrice() * item.getQuantity())
        .sum();
}

Key Takeaways

1. Code smells are indicators - They point to potential problems {.fragment}
2. Refactor continuously - Don’t let technical debt accumulate {.fragment}
3. Small steps - Many small refactorings are safer than big rewrites {.fragment}
4. Test first - Never refactor without tests {.fragment}
5. Name things well - Good names prevent many smells {.fragment}

Tools That Help

IDE Features

• IntelliJ IDEA
• Eclipse
• VS Code
• Automated refactorings

Static Analysis

• SonarQube
• PMD
• Checkstyle
• ESLint / Pylint

Practices

• Code review
• Pair programming
• Continuous integration

Further Learning

Essential Resources

Book: Refactoring: Improving the Design of Existing Code (2nd Edition) by Martin Fowler

Website: refactoring.com - Catalog of refactorings

In Visual Studio

Summary

• Refactoring is about behavior preserving changes that improve the internal design of the code.
• Code smells and architecture smells are indicators of possible refactoring opportunities
• Refactoring requires passing test suites - keep the tests green.