ch15-junit-internals

Chapter 15: JUnit Internals

clean-code junit case-study refactoring

Status: Notes complete
Difficulty: Medium
Time to complete: ~40 min read

---

Overview

This chapter is a live code review. Martin takes ComparisonCompactor — a real, production class from the JUnit testing framework — and walks through finding its smells and cleaning them up. The class is already “pretty good” by most standards: it is short, well-organized, and comes from a respected open-source project. The point of the chapter is exactly that: even good code can be made better, and the act of making it better is a professional obligation (the Boy Scout Rule).

Before: The class uses encoded member variable names (fExpected), mixes compacting logic with formatting logic in one method, and contains a hidden temporal coupling that can silently produce wrong output if methods are called out of order.

After: Each method does exactly one thing. Variable names are plain and honest. The temporal coupling is made explicit. The public method is renamed to describe what it produces, not what it does internally.

The lessons generalize far beyond JUnit: every piece of code, no matter how clean, rewards a patient second look.

---

The System Under Study

ComparisonCompactor lives in JUnit’s assertion machinery. When an assertEquals fails, JUnit calls this class to produce a human-readable failure message:

expected:<[ABCDE]> but was:<[ABCFE]>

For long strings, it compacts the output by trimming the parts that match and showing only the region that differs:

expected:<...[BD[E]F]...> but was:<...[BD[F]E]...>

The interface is a single public method:

public String compact(String message)

compact() takes a JUnit message prefix (e.g., "values differ") and returns the full formatted failure string with the compacted expected/actual values embedded.

The class holds three fields set at construction time:

• int contextLength — how many characters to show on each side of the difference
• String expected — the expected string
• String actual — the actual string

---

The Original ComparisonCompactor

Below is a faithful reconstruction of the original class as reviewed by Martin in the chapter.

// ORIGINAL — before refactoring
public class ComparisonCompactor {
 
    private static final String ELLIPSIS = "...";
    private static final String DELTA_END = "]";
    private static final String DELTA_START = "[";
 
    private int fContextLength;
    private String fExpected;
    private String fActual;
    private int fPrefix;
    private int fSuffix;
 
    public ComparisonCompactor(int contextLength,
                               String expected,
                               String actual) {
        fContextLength = contextLength;
        fExpected = expected;
        fActual = actual;
    }
 
    public String compact(String message) {
        if (fExpected == null || fActual == null || areStringsEqual())
            return Assert.format(message, fExpected, fActual);
 
        findCommonPrefix();
        findCommonSuffix();
        String expected = compactString(fExpected);
        String actual = compactString(fActual);
        return Assert.format(message, expected, actual);
    }
 
    private String compactString(String source) {
        String result = DELTA_START +
            source.substring(fPrefix, source.length() - fSuffix + 1) +
            DELTA_END;
        if (fPrefix > 0)
            result = computeCommonPrefix() + result;
        if (fSuffix > 0)
            result = result + computeCommonSuffix();
        return result;
    }
 
    private void findCommonPrefix() {
        fPrefix = 0;
        int end = Math.min(fExpected.length(), fActual.length());
        for (; fPrefix < end; fPrefix++) {
            if (fExpected.charAt(fPrefix) != fActual.charAt(fPrefix))
                break;
        }
    }
 
    private void findCommonSuffix() {
        int expectedSuffix = fExpected.length() - 1;
        int actualSuffix = fActual.length() - 1;
        for (; actualSuffix >= fPrefix && expectedSuffix >= fPrefix;
             actualSuffix--, expectedSuffix--) {
            if (fExpected.charAt(expectedSuffix) != fActual.charAt(actualSuffix))
                break;
        }
        fSuffix = fExpected.length() - expectedSuffix;
    }
 
    private String computeCommonPrefix() {
        return (fPrefix > fContextLength ? ELLIPSIS : "") +
            fExpected.substring(Math.max(0, fPrefix - fContextLength), fPrefix);
    }
 
    private String computeCommonSuffix() {
        int end = Math.min(fExpected.length() - fSuffix + 1 + fContextLength,
                           fExpected.length());
        return fExpected.substring(fExpected.length() - fSuffix + 1, end) +
            (fExpected.length() - fSuffix + 1 < fExpected.length() - fContextLength
                ? ELLIPSIS : "");
    }
 
    private boolean areStringsEqual() {
        return fExpected.equals(fActual);
    }
}

---

Problems Found (Code Smells)

Smell 1 — Encoding in Names (f prefix)

Category: Names (N6 — Avoid Encodings)
Where: fContextLength, fExpected, fActual, fPrefix, fSuffix
Why it’s a problem: The f prefix was a convention from early Java and C++ to signal “field” (member variable). Modern IDEs highlight fields with color; the encoding is redundant noise. It also creates a minor confusion hazard: you can shadow fExpected with a local expected in the same method body (which the original compact() method actually does!), leading to two variables with almost the same name referring to almost the same thing.
The fix: Drop the prefix. Fields are named contextLength, expected, actual, prefix, suffix.

---

Smell 2 — Negative Conditional (G28 — Encapsulate Conditionals)

Category: General
Where: The guard in compact(): if (fExpected == null || fActual == null || areStringsEqual())
Why it’s a problem: The condition is implicitly asking “should we NOT compact?” but the method is called compact(). The reader must flip the logic mentally. Martin introduces a helper shouldNotCompact() — or better, inverts it to canBeCompacted() — so the intent is readable without mental gymnastics.
The fix: Extract and invert to a positive named predicate.

// BAD — requires mental negation
if (fExpected == null || fActual == null || areStringsEqual())
    return Assert.format(message, fExpected, fActual);
 
// GOOD — intent is clear
if (shouldNotCompact())
    return Assert.format(message, expected, actual);

---

Smell 3 — Method Name Doesn’t Describe Output (N1 — Choose Descriptive Names)

Category: Names
Where: The public method compact(String message)
Why it’s a problem: compact describes an action (compacting strings), but the method returns a fully formatted comparison string. The real output is a formatted comparison message, not just a “compacted” string. A reader calling this method from a test runner wants to know what they’re getting, not what internal transformation occurred.
The fix: Rename to formatCompactedComparison(String message).

---

Smell 4 — Functions Doing More Than One Thing (F1 — Small! / Do One Thing)

Category: Functions
Where: The compact() (now formatCompactedComparison()) method
Why it’s a problem: The method (a) decides whether to compact, (b) computes the compacted expected string, (c) computes the compacted actual string, and (d) formats and returns the assertion failure message. That is at minimum three distinct responsibilities in one method.
The fix: Extract compactExpected and compactActual as computed fields (or returned values from private methods). The public method only assembles the final result.

---

Smell 5 — Hidden Temporal Coupling (G31 — Hidden Temporal Coupling)

Category: General
Where: findCommonPrefix() and findCommonSuffix() are two separate void methods that write to fields
Why it’s a problem: findCommonSuffix() uses fPrefix internally — it relies on findCommonPrefix() having been called first. Nothing in the method signature or calling code communicates this dependency. If a future maintainer reorders the calls, or calls only findCommonSuffix() after construction, the result is silently wrong. This is one of the most insidious structural bugs in the original class.
The fix: Make findCommonSuffix(int prefixLength) accept prefixLength as a parameter. The coupling is now explicit — you cannot call it without providing the prefix length you computed. The compiler enforces the ordering.

// BAD — hidden dependency
findCommonPrefix();   // sets fPrefix
findCommonSuffix();   // secretly uses fPrefix
 
// GOOD — dependency made explicit
int prefixLength = findCommonPrefix();
int suffixLength = findCommonSuffix(prefixLength);

---

Smell 6 — Misleading Variable Name (suffixIndex vs. suffixLength)

Category: Names (N4 — Unambiguous Names)
Where: fSuffix (called suffixIndex in some intermediate versions)
Why it’s a problem: The variable stores the count of matching suffix characters, not the index of the first matching character. The name suffixIndex implies an array index; the name suffixLength accurately describes what is stored. Using the wrong name forces readers to cross-reference with how the variable is used to understand what it contains.
The fix: Rename to suffixLength. This also makes off-by-one errors more obvious because expressions like source.length() - suffixLength read naturally as “trim N characters from the end.”

---

Smell 7 — Variable Name Shadowing

Category: Names / General
Where: In the original compact(), local variables String expected and String actual shadow the fields fExpected and fActual
Why it’s a problem: Even with the f prefix, having expected as a local and fExpected as a field in the same method body creates a cognitive load. Without the f prefix, the two names are identical. Shadowing is a source of subtle bugs.
The fix: After removing the f prefix from fields, rename the local variables to compactExpected and compactActual to be explicit about what they represent.

---

Refactoring Steps

Step 1 — Remove f prefix from all member variables

Before (Java):

private int fContextLength;
private String fExpected;
private String fActual;
private int fPrefix;
private int fSuffix;

After (Java):

private int contextLength;
private String expected;
private String actual;
private int prefixLength;
private int suffixLength;

Note that fSuffix is simultaneously renamed to suffixLength (Step 6 is folded in).

---

Step 2 — Rename compact() to formatCompactedComparison()

The method returns a formatted assertion message, so name it accordingly.

Before (Java):

public String compact(String message) { ... }

After (Java):

public String formatCompactedComparison(String message) { ... }

---

Step 3 — Extract shouldNotCompact() and invert the conditional

Before (Java):

public String compact(String message) {
    if (fExpected == null || fActual == null || areStringsEqual())
        return Assert.format(message, fExpected, fActual);
    ...
}

After (Java):

public String formatCompactedComparison(String message) {
    if (canBeCompacted()) {
        compactExpectedAndActual();
        return Assert.format(message, compactExpected, compactActual);
    } else {
        return Assert.format(message, expected, actual);
    }
}
 
private boolean canBeCompacted() {
    return expected != null && actual != null && !areStringsEqual();
}

The positive conditional (canBeCompacted()) reads as a natural English sentence. No mental negation required.

---

Step 4 — Extract compacting logic into separate methods

The compact method was doing four things. After extraction:

After (Java):

private String compactExpected;
private String compactActual;
 
private void compactExpectedAndActual() {
    prefixLength = findCommonPrefix();
    suffixLength = findCommonSuffix(prefixLength);
    compactExpected = compactString(expected);
    compactActual = compactString(actual);
}

Each private helper now does exactly one thing.

---

Step 5 — Fix the hidden temporal coupling by passing prefixLength

Before (Java) — findCommonSuffix() reads from the field fPrefix implicitly:

private void findCommonSuffix() {
    int expectedSuffix = fExpected.length() - 1;
    int actualSuffix = fActual.length() - 1;
    for (; actualSuffix >= fPrefix && expectedSuffix >= fPrefix; ...) {
        ...
    }
    fSuffix = fExpected.length() - expectedSuffix;
}

After (Java) — coupling is explicit:

private int findCommonSuffix(int prefixLength) {
    int expectedSuffix = expected.length() - 1;
    int actualSuffix = actual.length() - 1;
    for (; actualSuffix >= prefixLength && expectedSuffix >= prefixLength;
         actualSuffix--, expectedSuffix--) {
        if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix))
            break;
    }
    return expected.length() - expectedSuffix;
}

The caller is now forced to compute and pass prefixLength, making the dependency visible in the call site.

---

Step 6 — Rename suffixIndex to suffixLength

Already folded into Step 1. The rename also fixes the off-by-one arithmetic: the expressions source.length() - suffixLength now read accurately as “all but the last N characters”.

---

The Final ComparisonCompactor

After all refactorings, the class is significantly cleaner. Each method is short and has a single purpose.

// GOOD — after all refactorings
public class ComparisonCompactor {
 
    private static final String ELLIPSIS = "...";
    private static final String DELTA_END = "]";
    private static final String DELTA_START = "[";
 
    private final int contextLength;
    private final String expected;
    private final String actual;
    private int prefixLength;
    private int suffixLength;
 
    public ComparisonCompactor(int contextLength,
                               String expected,
                               String actual) {
        this.contextLength = contextLength;
        this.expected = expected;
        this.actual = actual;
    }
 
    public String formatCompactedComparison(String message) {
        String compactExpected = expected;
        String compactActual = actual;
        if (canBeCompacted()) {
            prefixLength = findCommonPrefix();
            suffixLength = findCommonSuffix(prefixLength);
            compactExpected = compact(expected);
            compactActual = compact(actual);
        }
        return Assert.format(message, compactExpected, compactActual);
    }
 
    private boolean canBeCompacted() {
        return expected != null && actual != null && !areStringsEqual();
    }
 
    private boolean areStringsEqual() {
        return expected.equals(actual);
    }
 
    private int findCommonPrefix() {
        int prefix = 0;
        int end = Math.min(expected.length(), actual.length());
        for (; prefix < end; prefix++) {
            if (expected.charAt(prefix) != actual.charAt(prefix))
                break;
        }
        return prefix;
    }
 
    private int findCommonSuffix(int prefixLength) {
        int expectedSuffix = expected.length() - 1;
        int actualSuffix = actual.length() - 1;
        for (; actualSuffix >= prefixLength && expectedSuffix >= prefixLength;
             actualSuffix--, expectedSuffix--) {
            if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix))
                break;
        }
        return expected.length() - expectedSuffix;
    }
 
    private String compact(String source) {
        return computeCommonPrefix()
            + DELTA_START
            + source.substring(prefixLength, source.length() - suffixLength + 1)
            + DELTA_END
            + computeCommonSuffix();
    }
 
    private String computeCommonPrefix() {
        return (prefixLength > contextLength ? ELLIPSIS : "")
            + expected.substring(Math.max(0, prefixLength - contextLength), prefixLength);
    }
 
    private String computeCommonSuffix() {
        int end = Math.min(expected.length() - suffixLength + 1 + contextLength,
                           expected.length());
        return expected.substring(expected.length() - suffixLength + 1, end)
            + (expected.length() - suffixLength + 1 < expected.length() - contextLength
               ? ELLIPSIS : "");
    }
}

What changed:

• All f-prefixed fields are gone
• compact() is now formatCompactedComparison() — the name matches the output
• findCommonSuffix takes prefixLength explicitly — temporal coupling is surfaced
• canBeCompacted() replaces the inline negative conditional
• compactString() is renamed to compact() — the parameter makes it unambiguous

---

Lessons Learned

1. Good code from good projects still has smells.
JUnit is a professional, widely-reviewed open-source library. The ComparisonCompactor was already compact and functional. Yet six distinct smells were found in under 50 lines. Smells do not require negligence — they accumulate through small, individually defensible decisions.

2. Renaming is the highest-ROI refactoring.
Three of the six smells were resolved entirely by renaming. Renaming costs seconds and produces lasting dividends every time the file is read. It is the easiest refactoring and the one most often skipped.

3. Temporal coupling is invisible until it breaks.
The findCommonPrefix() / findCommonSuffix() ordering dependency could not be seen from reading either method in isolation. Only by tracing the data flow do you find it. The fix — passing prefixLength as a parameter — makes the dependency undeniable. Prefer function signatures that tell you what a method requires over functions that read hidden state.

4. Positive conditionals read faster than negative ones.
if (canBeCompacted()) scans in one pass. if (!(expected == null || actual == null || areStringsEqual())) forces you to build and then invert a mental model. Negative conditionals are not wrong, but they are slower to read. When a conditional guards a non-trivial path, name it positively.

5. Method names should describe outputs, not internal actions.
compact() describes what the method does internally. formatCompactedComparison() describes what the method returns. Callers care about what they receive, not how the method produces it. Verbs like compute, find, build, format, create are better than opaque nouns or single-word actions when the output is a meaningful artifact.

6. Variable shadowing is an avoidable hazard.
The original code had local variables shadowing fields with near-identical names. This is legal in Java but harmful. When removing encodings like f, simultaneously rename the locals to avoid shadowing. The compiler will not warn you; the discipline must come from the programmer.

7. The Boy Scout Rule works at any scale.
The chapter ends with a class that is measurably better than it was before. Martin did not rewrite it — he made six small, local improvements. Each was safe because it preserved observable behavior. This is what “leaving the code cleaner than you found it” looks like in practice.

---

Application to Other Languages

The same class of smells appears in C++ and Python. The following examples use an analogous DiffCompactor scenario to show the same principles applied idiomatically.

C++ — DiffCompactor

Problem (C++ with f-prefix encoding and hidden temporal coupling):

// BAD — C++ version with same smells
class DiffCompactor {
public:
    DiffCompactor(int contextLen, std::string exp, std::string act)
        : fContextLen(contextLen), fExpected(std::move(exp)), fActual(std::move(act)) {}
 
    std::string compact(const std::string& message) {
        if (fExpected.empty() || fActual.empty() || fExpected == fActual)
            return message + " expected:<" + fExpected + "> but was:<" + fActual + ">";
        findCommonPrefix();   // sets fPrefixLen
        findCommonSuffix();   // silently depends on fPrefixLen — hidden coupling
        return message + formatDiff();
    }
 
private:
    int fContextLen;
    std::string fExpected;
    std::string fActual;
    int fPrefixLen = 0;
    int fSuffixLen = 0;   // named "fSuffixIndex" in original — misleading
 
    void findCommonPrefix() { /* ... */ }
    void findCommonSuffix() { /* reads fPrefixLen implicitly */ }
    std::string formatDiff() { /* ... */ }
};

After refactoring (C++17):

// GOOD — C++17 version: no encodings, explicit coupling, positive predicate
class DiffCompactor {
public:
    DiffCompactor(int contextLength, std::string expected, std::string actual)
        : contextLength_(contextLength),
          expected_(std::move(expected)),
          actual_(std::move(actual)) {}
 
    std::string formatCompactedComparison(const std::string& message) const {
        if (!canBeCompacted())
            return message + " expected:<" + expected_ + "> but was:<" + actual_ + ">";
 
        const int prefixLen = findCommonPrefix();
        const int suffixLen = findCommonSuffix(prefixLen);  // explicit dependency
        return message + formatDiff(prefixLen, suffixLen);
    }
 
private:
    const int contextLength_;
    const std::string expected_;
    const std::string actual_;
 
    bool canBeCompacted() const {
        return !expected_.empty() && !actual_.empty() && expected_ != actual_;
    }
 
    int findCommonPrefix() const {
        int prefix = 0;
        const int end = static_cast<int>(
            std::min(expected_.size(), actual_.size()));
        while (prefix < end && expected_[prefix] == actual_[prefix])
            ++prefix;
        return prefix;
    }
 
    // prefixLen passed explicitly — coupling is visible at the call site
    int findCommonSuffix(int prefixLen) const {
        int expSuffix = static_cast<int>(expected_.size()) - 1;
        int actSuffix = static_cast<int>(actual_.size()) - 1;
        while (expSuffix >= prefixLen && actSuffix >= prefixLen
               && expected_[expSuffix] == actual_[actSuffix]) {
            --expSuffix;
            --actSuffix;
        }
        return static_cast<int>(expected_.size()) - expSuffix;
    }
 
    std::string formatDiff(int prefixLen, int suffixLen) const {
        return computePrefix(prefixLen)
             + "[" + expected_.substr(prefixLen, expected_.size() - suffixLen - prefixLen + 1) + "]"
             + computeSuffix(suffixLen);
    }
 
    std::string computePrefix(int prefixLen) const {
        const int start = std::max(0, prefixLen - contextLength_);
        return (prefixLen > contextLength_ ? "..." : "")
             + expected_.substr(start, prefixLen - start);
    }
 
    std::string computeSuffix(int suffixLen) const {
        const int start = static_cast<int>(expected_.size()) - suffixLen + 1;
        const int end   = std::min(start + contextLength_,
                                   static_cast<int>(expected_.size()));
        return expected_.substr(start, end - start)
             + (start + contextLength_ < static_cast<int>(expected_.size()) ? "..." : "");
    }
};

Key C++-specific idioms used:

• Trailing underscore for member variables (contextLength_) — idiomatic C++ naming, no f or m_ encoding
• const correctness — all query methods are const
• std::move to avoid copies in the constructor
• static_cast<int> to avoid signed/unsigned comparison warnings

---

Python — DiffCompactor

Python makes several of these refactorings even more natural through @property decorators and dataclass.

Problem (Python with encoding and hidden coupling):**

# BAD — Python class with same smells
class DiffCompactor:
    def __init__(self, context_len, f_expected, f_actual):
        self.context_len = context_len
        self.f_expected = f_expected  # encoding in name — unnecessary
        self.f_actual = f_actual
        self.f_prefix = 0
        self.f_suffix = 0             # called f_suffix_index — misleading
 
    def compact(self, message):       # name describes action, not output
        if not self.f_expected or self.f_expected == self.f_actual:
            return f"{message} expected:<{self.f_expected}> but was:<{self.f_actual}>"
        self._find_common_prefix()    # sets self.f_prefix
        self._find_common_suffix()    # silently depends on self.f_prefix
        return message + self._format_diff()

After refactoring (Python 3.10+):

# GOOD — Python idiomatic version
from dataclasses import dataclass
 
 
@dataclass(frozen=True)
class DiffCompactor:
    context_length: int
    expected: str
    actual: str
 
    def format_compacted_comparison(self, message: str) -> str:
        """Return a formatted assertion failure message with compacted diff."""
        if not self._can_be_compacted():
            return f"{message} expected:<{self.expected}> but was:<{self.actual}>"
 
        prefix_len = self._find_common_prefix()
        suffix_len = self._find_common_suffix(prefix_len)  # explicit dependency
        compact_expected = self._compact(self.expected, prefix_len, suffix_len)
        compact_actual   = self._compact(self.actual,   prefix_len, suffix_len)
        return f"{message} expected:<{compact_expected}> but was:<{compact_actual}>"
 
    def _can_be_compacted(self) -> bool:
        return bool(self.expected) and bool(self.actual) and self.expected != self.actual
 
    def _find_common_prefix(self) -> int:
        for i, (e, a) in enumerate(zip(self.expected, self.actual)):
            if e != a:
                return i
        return min(len(self.expected), len(self.actual))
 
    # prefix_len passed explicitly — temporal dependency is visible
    def _find_common_suffix(self, prefix_len: int) -> int:
        exp = self.expected[::-1]
        act = self.actual[::-1]
        for i, (e, a) in enumerate(zip(exp, act)):
            if e != a or (len(self.expected) - i - 1) < prefix_len:
                return i
        return min(len(self.expected), len(self.actual)) - prefix_len
 
    def _compact(self, source: str, prefix_len: int, suffix_len: int) -> str:
        inner = source[prefix_len : len(source) - suffix_len]
        return (
            self._compute_prefix_context(prefix_len)
            + f"[{inner}]"
            + self._compute_suffix_context(suffix_len)
        )
 
    def _compute_prefix_context(self, prefix_len: int) -> str:
        start = max(0, prefix_len - self.context_length)
        ellipsis = "..." if prefix_len > self.context_length else ""
        return ellipsis + self.expected[start:prefix_len]
 
    def _compute_suffix_context(self, suffix_len: int) -> str:
        start = len(self.expected) - suffix_len
        end   = min(start + self.context_length, len(self.expected))
        ellipsis = "..." if start + self.context_length < len(self.expected) else ""
        return self.expected[start:end] + ellipsis

Python-specific idioms used:

• @dataclass(frozen=True) — the compactor is logically immutable; freeze it to prevent accidental mutation
• No f_ or m_ prefix — Python convention is plain names; leading underscore _ for private methods
• zip() for parallel iteration — more idiomatic than index arithmetic for the prefix scan
• f-strings — clearer than + concatenation for formatting
• Type annotations — str, int, -> str make the interface self-documenting

---

Checklist

Use this checklist when reviewing any class for similar smells:

• Are member variable names free of encoding prefixes (f, m_, _f)?
• Does every method name describe what it returns, not just what it does internally?
• Are all conditionals that guard the “happy path” expressed positively?
• Are there any void methods that depend on other void methods having run first?
• If temporal coupling exists, is prefixLength (or equivalent) passed explicitly?
• Do variable names use length vs. index accurately based on what they store?
• Are there local variables that shadow fields with near-identical names?
• Does each method do exactly one thing, at one level of abstraction?
• Could any method be further extracted into a helper with a more descriptive name?
• Is the public method name what the caller cares about (the output), not an internal detail?

---

Key Takeaways

1. Encoding in names is noise. The f prefix (and m_, _m_, this.) was invented for environments without IDE support. In modern code, it adds visual noise without information. Remove it.

2. Name variables by what they store, not by their role. suffixIndex vs. suffixLength is not a cosmetic difference — it tells the reader whether to expect a zero-based array offset or a count.

3. Temporal coupling must be made explicit. Any method that secretly depends on another method having been called first is a time bomb. The fix is to return values and pass them as parameters rather than sharing mutable state through fields.

4. Positive conditionals are faster to read. if (canBeCompacted()) scans in one pass. Convert negative guards to named positive predicates at the method boundary.

5. Method names describe outputs. formatCompactedComparison() tells the caller what they receive. compact() only tells them what happened internally. Prefer the former.

6. Refactoring has a flywheel effect. Each small improvement (renaming a variable) makes the next improvement (spotting a misplaced conditional) easier to see. Start with the cheapest fix; the expensive ones often disappear.

7. Even production code from respected projects has smells. This is not a criticism of the JUnit authors — it is a reminder that smells are cumulative and appear at every level of skill. Regular review is the only defense.

8. The Boy Scout Rule is achievable in minutes. Six smells. Six fixes. No behavior changed. The class is now easier to read and maintain than before. This is what continuous improvement looks like at the code level.

---

Related Resources

• ch17-smells-and-heuristics — Master catalog of smells referenced throughout this chapter (G28, G31, N1, N4, N6)
• ch14-successive-refinement — Companion case study: iterative cleanup of an Args parser; same refactoring discipline applied over more iterations
• ch02-meaningful-names — Deep treatment of the naming principles applied here (encodings, expressive names, avoiding disinformation)
• ch03-functions — The “functions do one thing” and “one level of abstraction” principles that motivate extracting findCommonPrefix / findCommonSuffix
• ch10-classes — Cohesion and single responsibility at the class level; the refactored ComparisonCompactor is an example of high cohesion

---

Last Updated: 2026-04-14