Skip to main content
Software Maintenance
Software Maintenance
IS IA
2h

Clean Code

Any fool can write code that a computer can understand. Good programmers write code that humans can understand. — Martin Fowler

Clean code is not a luxury — in a maintenance context, code that humans cannot understand becomes progressively more expensive to modify. This lecture covers the practices and tools that keep code readable, maintainable, and trustworthy over time.


What is Quality Code?

Quality code is characterised by four properties:

  • Correct: matches its technical specification, does what it claims to do
  • Robust: handles edge cases, unexpected inputs, and failures gracefully
  • Readable: easy to understand without prior context; the next developer can comprehend it quickly
  • Efficient: uses resources appropriately — not over-engineered, not wasteful

Software entropy: an evolving system increases its complexity unless work is done to reduce it. — Meir Lehman

Shipping first-time code is like going into debt. A little debt speeds development so long as it is paid back promptly with refactoring. The danger occurs when the debt is not repaid. Every minute spent on code that is not quite right counts as interest on that debt. — Ward Cunningham

Short-term shortcuts accumulate over time. Unrepaid technical debt slows all future development — every new feature must be built on top of an increasingly unstable foundation.

Deliberate vs. Accidental Debt

The metaphor is worth taking seriously, because not all technical debt is a mistake. A useful split:

  • Deliberate debt: a conscious tradeoff, made with eyes open — “we’re hardcoding this config to hit the demo deadline; ticket filed to fix it after launch.” Like a real loan, this can be a perfectly reasonable business decision, as long as someone intends to pay it back and everyone involved knows the debt exists.
  • Accidental (or “reckless”) debt: shortcuts nobody chose deliberately — code that grew messy through neglect, a design that was never revisited as requirements changed, or corners cut by a team that didn’t know better. This is the kind the Code Smells section below helps you recognize.

The failure mode isn’t taking on debt — it’s taking it on without anyone noticing, so it never gets tracked, never gets prioritized against new features, and quietly compounds. A team that deliberately logs its shortcuts (a // TODO with a linked ticket, a line in a known-issues doc) is managing debt. A team that doesn’t even know how much of it exists is the one Lehman’s law of increasing entropy describes: complexity rising with no corresponding effort to reduce it.


Naming Conventions

Names are the primary communication channel between the author and future readers. A well-named variable, method, or class eliminates the need for a comment.

  • Names should be auto-descriptive and pronounceable — avoid abbreviations and acronyms
  • Variables and fields: nouns or noun phrases describing what they hold
  • Methods: verbs or verb phrases — getUser(), validateInput(), computeTotal()
  • Booleans: questions that answer true/false — isValid(), hasPermission(), areEqual()
  • Constants: SCREAMING_SNAKE_CASE for named constants; never embed magic numbers or strings in logic
// Bad: what is 'd'? what does '4' mean? what is 'u.s'?
int d = 86400;
if (u.s == 4) { ... }

// Good: self-explanatory, no comment needed
final int SECONDS_PER_DAY = 86400;
if (user.status == UserStatus.SUSPENDED) { ... }

Comments

Comments are always a failure. — Robert C. Martin (“Uncle Bob”), Clean Code

Comments lie — they go out of sync with the code they describe. They age badly: code is refactored but comments are forgotten. They are not refactorable: renaming a method does not rename comments that mention it.

A comment often signals that the code failed to:

  • Choose a good name for a variable or method
  • Extract logic into a well-named helper
  • Create the right abstraction

A stale comment is worse than no comment at all — it actively misleads, because a reader trusts it by default:

// Retry up to 3 times before giving up
for (int attempt = 0; attempt < 5; attempt++) {
    if (tryConnect()) break;
}

The comment says 3, the loop says 5 — someone changed the retry count and never touched the comment above it, and there’s no way for a reader to know which one is actually true without checking the git history. Rename the loop variable and drop the comment entirely, and this problem becomes structurally impossible:

final int MAX_CONNECTION_ATTEMPTS = 5;
for (int attempt = 0; attempt < MAX_CONNECTION_ATTEMPTS; attempt++) {
    if (tryConnect()) break;
}
When comments ARE acceptable
  • Javadoc (/** ... */) for public APIs — documents intent and contracts, not implementation
  • Algorithm citations — when using a non-obvious algorithm, cite the paper or source (future maintainers need to understand the why)
  • Legal and license headers — required by many organizations

Code Layout

Good layout makes a file easier to navigate before a single line of logic is read:

  • File size: aim for ~200 lines; 500 is a hard warning sign
  • Line length: 80–120 characters per line — beyond that, the logic is probably too complex
  • Indentation and spacing: consistent style, enforced by tooling (not by convention)
  • Reading order: code should flow top-to-bottom like a newspaper — high-level concept first, details lower
The Newspaper Metaphor

A well-organized class reads like a news article: the headline (class name and purpose) at the top, the most important concepts next, and implementation details at the bottom. A reader should be able to stop reading at any point and have understood the most important parts.


Principles

KISS — Keep It Simple, Stupid

If you can’t explain it simply, you don’t understand it well enough. — Albert Einstein

  • Use the simplest logical approach that works
  • Avoid layers of abstraction for their own sake
  • Simpler code is easier to read, test, debug, and maintain
  • Complexity is a debt paid by every future reader

DRY — Don’t Repeat Yourself

Also known as DIE (Duplication Is Evil).

  • Every piece of knowledge should have a single, authoritative representation in the codebase
  • Code duplication means multiple places to fix the same bug
  • Duplication is the root cause of many maintenance problems: a fix in one copy is forgotten in the other

Factorize: extract common logic into methods, constants, or classes. Tools like PMD’s CPD (Copy/Paste Detector) can find duplication automatically.

YAGNI — You Aren’t Gonna Need It

From Extreme Programming (XP): prefer the simplest thing that could possibly work.

  • Do not build features you think you might need in the future
  • Unused code is dead weight: it must be read, tested, and maintained, but serves no user
  • Speculative generality is one of the most common code smells in maintenance-heavy codebases

SOLID

The five SOLID principles provide a framework for designing classes and modules that are easy to change and extend:

PrincipleNameMeaning
SSingle ResponsibilityA class should have exactly one reason to change
OOpen/ClosedOpen for extension, closed for modification — add behaviour by adding code, not by changing existing code
LLiskov SubstitutionA subclass must be usable wherever its parent class is used without surprising the caller
IInterface SegregationMany small, specific interfaces are better than one large general-purpose interface
DDependency InversionDepend on abstractions (interfaces), not concrete implementations

Violating SOLID typically leads to tightly coupled, hard-to-test classes that break whenever a related component changes.

Law of Demeter

“Don’t talk to strangers.” A method should only call:

  • Methods on itself (this)
  • Methods on objects it created
  • Methods on objects passed to it as parameters
  • Methods on its direct fields

Violating the Law of Demeter produces train wrecks: chains like order.getCustomer().getAddress().getCity() couple the caller to the entire object graph and make refactoring painful. The fix is to ask the object for what you need, not to dig into its structure.

// Violation — caller knows too much about internal structure
String city = order.getCustomer().getAddress().getCity();

// Better — delegation through the chain
String city = order.getCustomerCity();

Code Smells

Code smells are patterns that signal a likely design problem. They don’t always indicate a bug, but they indicate code that will become harder to maintain over time:

SmellDescription
Long MethodMethods longer than 20-30 lines usually have multiple responsibilities
Large ClassClasses that do too much; violates Single Responsibility
Duplicate CodeThe same logic appears in more than one place
Long Parameter ListMore than 2-3 parameters is a sign of missing abstraction
Feature EnvyA method uses data from another class more than its own
Dead CodeCode that is never called — dead weight, misleads readers
Magic Numbers/StringsLiteral values with no explanation embedded in logic
Deeply Nested ConditionalsMore than 2-3 nesting levels; use early returns or extracted methods

Two of these are worth seeing in code, since they’re easy to miss when reading quickly. Feature Envy — a method that reaches into another object’s data more than it uses its own is a sign the logic belongs on that other class instead:

// Feature Envy: Invoice barely touches its own state — it's really operating on Customer
class Invoice {
    double total(Customer customer) {
        double discount = customer.getLoyaltyTier() == Tier.GOLD ? 0.10
                         : customer.getLoyaltyTier() == Tier.SILVER ? 0.05
                         : 0.0;
        return amount * (1 - discount);
    }
    private double amount;
}

// Fixed: the discount logic moves to where the data actually lives
class Customer {
    double discountRate() {
        return switch (loyaltyTier) {
            case GOLD -> 0.10;
            case SILVER -> 0.05;
            default -> 0.0;
        };
    }
    private Tier loyaltyTier;
}

class Invoice {
    double total(Customer customer) {
        return amount * (1 - customer.discountRate());
    }
    private double amount;
}

Deeply Nested Conditionals, fixed with early returns — the logic doesn’t change, only how far right it drifts:

// Deeply nested
String classify(int age, boolean hasLicense) {
    if (age >= 18) {
        if (hasLicense) {
            return "can drive";
        } else {
            return "needs a license";
        }
    } else {
        return "too young";
    }
}

// Flattened with early returns — same logic, no nesting
String classify(int age, boolean hasLicense) {
    if (age < 18) return "too young";
    if (!hasLicense) return "needs a license";
    return "can drive";
}
Code smells compound over time

A single code smell is a minor concern. Multiple smells in the same class signal a class that has lost cohesion and will become exponentially more expensive to modify. Address smells incrementally during normal development — do not wait for a dedicated cleanup sprint.


Refactoring Catalog

“Refactor this” is easy to say and vague to act on. Refactoring specifically means changing a program’s internal structure without changing its observable behavior — which is exactly why the Testing chapter’s TDD cycle puts a “Refactor” step after Green: you only rearrange code you can already prove still works. Below is a small catalog of named, repeatable moves — each one a concrete answer to “how do I actually fix this smell?”

Extract Method — pull a chunk of a long method out into its own, well-named method. This is the direct fix for Long Method, and often for Deeply Nested Conditionals too, since each extracted branch gets to stand on its own:

// Before
void printInvoice(Invoice invoice) {
    double subtotal = invoice.items().stream().mapToDouble(Item::price).sum();
    double tax = subtotal * 0.20;
    double total = subtotal + tax;
    System.out.println("Subtotal: " + subtotal);
    System.out.println("Tax: " + tax);
    System.out.println("Total: " + total);
}

// After — the "what" (printInvoice) is now separated from the "how" (calculateTotal)
void printInvoice(Invoice invoice) {
    InvoiceTotal totals = calculateTotal(invoice);
    System.out.println("Subtotal: " + totals.subtotal());
    System.out.println("Tax: " + totals.tax());
    System.out.println("Total: " + totals.total());
}

InvoiceTotal calculateTotal(Invoice invoice) {
    double subtotal = invoice.items().stream().mapToDouble(Item::price).sum();
    double tax = subtotal * 0.20;
    return new InvoiceTotal(subtotal, tax, subtotal + tax);
}

Extract Class — when a class has grown two or more unrelated responsibilities (Large Class), split it along that seam rather than trying to trim it in place:

// Before: Employee is doing payroll math AND contact-info formatting
class Employee {
    double calculatePay() { /* ... */ }
    String formatMailingAddress() { /* ... */ }
}

// After: each responsibility gets its own class
class Employee {
    private final PayCalculator payCalculator;
    private final Address address;
    double calculatePay() { return payCalculator.calculate(this); }
    String formatMailingAddress() { return address.format(); }
}
class PayCalculator { double calculate(Employee e) { /* ... */ return 0; } }
class Address { String format() { /* ... */ return ""; } }

Replace Magic Number/String with a Named Constant — the smallest, cheapest refactor on this list, and the direct fix for the Magic Numbers/Strings smell:

// Before
if (order.total() > 10000) { flagForReview(order); }

// After
static final double FRAUD_REVIEW_THRESHOLD = 10_000;
if (order.total() > FRAUD_REVIEW_THRESHOLD) { flagForReview(order); }

Replace Conditional with Polymorphism — when a method branches on an object’s type or category to decide what to do, that branching logic is often better expressed as an overridden method on each type instead. This directly fixes a common source of Duplicate Code, since the same type-check tends to be copy-pasted at every call site:

// Before: every caller that cares about shape area needs this same switch
double area(Shape shape) {
    return switch (shape.kind()) {
        case CIRCLE -> Math.PI * shape.radius() * shape.radius();
        case SQUARE -> shape.side() * shape.side();
    };
}

// After: each shape knows how to compute its own area; the switch disappears entirely
abstract class Shape {
    abstract double area();
}
class Circle extends Shape {
    double area() { return Math.PI * radius * radius; }
    private double radius;
}
class Square extends Shape {
    double area() { return side * side; }
    private double side;
}

Rename and Inline round out the everyday toolkit: Rename (variable, method, or class) exists because a name that was clear when written can stop being clear as a system evolves — renaming costs nothing at compile time and pays back every time someone reads the code afterward. Inline Method/Variable is Extract Method’s inverse: when a method or variable adds a layer of indirection without adding real clarity (a one-line method called from exactly one place, a variable used once right after its declaration), removing it can make the code easier to follow, not harder.

Refactor in small, reversible steps

Every move above should be small enough to run your tests immediately afterward and confirm nothing broke. Refactoring five things at once and then running the tests defeats the purpose — if something fails, you won’t know which of the five changes caused it. This is also exactly why the characterization tests from the Testing chapter matter here: you cannot safely refactor code you have no way to check.

Java Tooling

Good practices need tool support. The Java ecosystem provides a mature set of tools that integrate into build systems (Maven, Gradle) and CI pipelines.

Checkstyle

Enforces coding style rules: naming conventions, import ordering, Javadoc completeness, maximum line length, indentation.

  • Configurable rule sets (Google Java Style, Sun Coding Conventions, or custom)
  • Integrates with Maven, Gradle, IntelliJ IDEA, Eclipse, VS Code
  • Fast: runs on source code before compilation
<!-- Maven: add to pom.xml -->
<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-checkstyle-plugin</artifactId>
    <version>3.4.0</version>
    <configuration>
        <configLocation>google_checks.xml</configLocation>
        <failsOnError>true</failsOnError>
        <consoleOutput>true</consoleOutput>
    </configuration>
    <executions>
        <execution>
            <id>validate</id>
            <phase>validate</phase>
            <goals><goal>check</goal></goals>
        </execution>
    </executions>
</plugin>

Run: mvn checkstyle:check

PMD

Static source code analyzer that finds potential bugs, unused code, overly complex methods, empty catch blocks, and naming violations. Its CPD (Copy/Paste Detector) sub-tool finds duplicated code.

# Analyze source code
pmd check -d src/main/java -R rulesets/java/quickstart.xml -f text

# Find duplicate code (CPD)
pmd cpd --minimum-tokens 100 --dir src/main/java

SpotBugs

Analyzes compiled bytecode (.class files) rather than source code. This allows it to detect a different class of bugs: null pointer dereferences, resource leaks, incorrect equals/hashCode implementations, infinite loops, and security vulnerabilities.

SpotBugs is the maintained successor to FindBugs.

<plugin>
    <groupId>com.github.spotbugs</groupId>
    <artifactId>spotbugs-maven-plugin</artifactId>
    <version>4.8.6.4</version>
    <configuration>
        <effort>Max</effort>
        <threshold>Low</threshold>
    </configuration>
</plugin>

Run: mvn spotbugs:check

JaCoCo — Code Coverage

Measures which lines, branches, and methods are actually executed by your test suite. Generates HTML and XML reports.

<plugin>
    <groupId>org.jacoco</groupId>
    <artifactId>jacoco-maven-plugin</artifactId>
    <version>0.8.11</version>
    <executions>
        <execution>
            <goals><goal>prepare-agent</goal></goals>
        </execution>
        <execution>
            <id>report</id>
            <phase>test</phase>
            <goals><goal>report</goal></goals>
        </execution>
    </executions>
</plugin>

Run: mvn test (coverage runs automatically) → HTML report in target/site/jacoco/index.html

Coverage is not Quality

100% line coverage does not mean bug-free code. A test that executes a line without asserting anything about the result contributes to coverage without validating behaviour. Coverage is a necessary but not sufficient condition for quality.

SonarQube

A unified quality management platform that aggregates results from static analysis, coverage, duplication detection, and security scanning into a single dashboard.

  • Combines Checkstyle, PMD, SpotBugs, and JaCoCo data
  • Tracks metrics over time: technical debt trend, code smell growth, coverage evolution
  • Quality Gate: a configurable threshold (e.g., “coverage > 80%, no new critical bugs”) that can fail a CI pipeline
  • Available as a cloud service (SonarCloud) or self-hosted

Build Automation Pipeline (GitLab CI)

Putting it all together — an example pipeline that enforces quality automatically on every push:

stages:
    - lint
    - build
    - test
    - quality

job:checkstyle:
    stage: lint
    script: mvn checkstyle:check

job:build:
    stage: build
    script: mvn compile # note: -DskipTests would be a no-op here — the "compile"
                         # phase runs before "test" in Maven's lifecycle, so tests
                         # never execute at this stage regardless of that flag

job:test:
    stage: test
    script: mvn test

job:quality:
    stage: quality
    script:
        - mvn spotbugs:check
        # JaCoCo report is generated automatically during 'mvn test' (bound to test phase)
        - mvn sonar:sonar -Dsonar.projectKey=my-project
    when: on_success
    dependencies:
        - job:test
Fail fast, fix fast

Putting Checkstyle in the lint stage means style violations are caught before compilation even starts — the fastest possible feedback. SpotBugs and SonarQube run after tests so they have coverage data available. Order your pipeline stages to surface the cheapest checks first.