JUnit Internals

JUnit's ComparisonCompactor is the small utility that produces the familiar expected:<...B[X]D...> but was:<...B[Y]D...> message when a string assertion fails. Given two strings that differ, it finds the parts they have in common and collapses everything else into a compact diff. It is a good refactoring case study for a specific reason: the starting code is not bad. Kent Beck and Erich Gamma wrote it, it is fully covered by tests, and it works. The exercise is not "fix broken code," it is "make already-good code better," which is closer to the refactoring you actually do day to day than cleaning up a disaster.

Reading the tests before the code

The chapter's first move is to read the unit tests before reading the implementation. A well-named test suite documents the contract of a class better than prose does, and for ComparisonCompactor the tests spell out exactly what "compact" means for every edge case: matching prefixes, matching suffixes, no context, overlapping matches, null inputs. If you ever inherit a class with no documentation, the tests are the documentation. Reading them first means you refactor knowing what must keep working.

What was actually wrong with it

Nothing dramatic, which is the point. The problems are all small frictions that compound:

• Scope-encoded names. Every member variable is prefixed with f (fExpected, fPrefix, fContextLength), a convention from an era before IDEs could tell you at a glance whether a variable was a field or a local. The prefix adds nothing today and forces a small mental translation on every read.
• An unencapsulated, negative conditional. The compact method opens with if (fExpected == null || fActual == null || areStringsEqual()), a three-part condition inline, phrased as a negative ("don't compact when..."). A reader has to parse the whole expression before understanding what the branch is for.
• A misleading method name. The method is called compact, but when the early-exit condition is true it does not compact anything - it just formats the raw strings. The name promises a narrower behavior than the method delivers.
• Magic off-by-ones. fSuffix is calculated as a one-based length, which is why computeCommonSuffix is full of +1s scattered through the arithmetic. The moment you rebase suffixLength to be genuinely zero-based, the +1s disappear and two if statements that looked load-bearing turn out to be dead weight (comment them out, tests still pass, delete them).

The moves that fixed it

// Original ComparisonCompactor: builds the

// "expected:<...> but was:<...>" message for a failed string assertion.

class ComparisonCompactor {

constructor(contextLength, fExpected, fActual) {

this.fContextLength = contextLength;

this.fExpected = fExpected;

this.fActual = fActual;

}

 

compact(message) {

if (this.fExpected == null || this.fActual == null || this.areStringsEqual()) {

return formatMessage(message, this.fExpected, this.fActual);

}

this.findCommonPrefix();

this.findCommonSuffix();

const expected = this.compactString(this.fExpected);

const actual = this.compactString(this.fActual);

return formatMessage(message, expected, actual);

}

 

compactString(source) {

// fPrefix / fSuffix, the +1s, and the "f" prefix all need a reader

// to reconstruct what "prefix" and "suffix" even mean here.

let result = '[' + source.substring(this.fPrefix, source.length - this.fSuffix + 1) + ']';

if (this.fPrefix > 0) result = this.computeCommonPrefix() + result;

if (this.fSuffix > 0) result = result + this.computeCommonSuffix();

return result;

}

 

areStringsEqual() {

return this.fExpected === this.fActual;

}

}

// Final shape: the negative check is inverted, the misnamed

// compact() is split into formatting vs. compacting, and the

// "index" fields are renamed to what they actually measure.

class ComparisonCompactor {

constructor(contextLength, expected, actual) {

this.contextLength = contextLength;

this.expected = expected;

this.actual = actual;

}

 

formatCompactedComparison(message) {

let compactExpected = this.expected;

let compactActual = this.actual;

if (this.shouldBeCompacted()) {

this.findCommonPrefixAndSuffix();

compactExpected = this.compact(this.expected);

compactActual = this.compact(this.actual);

}

return formatMessage(message, compactExpected, compactActual);

}

 

shouldBeCompacted() {

return !(this.expected == null || this.actual == null || this.expected === this.actual);

}

 

compact(s) {

// Every fragment (ellipsis, context, delta) is its own named

// method; the function is now just their concatenation.

return this.startingEllipsis() + this.startingContext() +

'[' + this.delta(s) + ']' +

this.endingContext() + this.endingEllipsis();

}

}

The representative sequence, condensed into the comparison above: drop the f prefix, extract the inline conditional into a named predicate (shouldNotBeCompacted), invert it to read positively (shouldBeCompacted), then split the method in two. One method (formatCompactedComparison) does the formatting and owns the early-exit branch; a second, smaller method does nothing but the actual compacting. Renaming a variable to fix an off-by-one, then noticing that the fix makes a conditional unreachable, is refactoring at its most typical: one small, safe change exposes the next one.

One extraction didn't survive contact with reality. Computing the prefix and the suffix started out as two independent-looking methods, but finding the suffix secretly depended on the prefix having been found first - a hidden temporal coupling. Passing the prefix in as an argument made the ordering work, but the argument itself looked arbitrary to anyone reading the signature cold, so the two methods were merged into one findCommonPrefixAndSuffix instead, which makes the dependency impossible to get wrong. Martin is upfront that the final version also reverses a few earlier micro-decisions - some extracted methods get inlined back in, and a conditional's sense flips a second time - because refactoring converges through trial and error, not a straight line from bad to good.

The transferable lesson

None of these changes are individually impressive. That is exactly why the chapter is worth reading: it demonstrates that "clean code" is mostly the accumulation of many small, low-risk renames and extractions, each one made possible by the safety net of a passing test suite, rather than one clever rewrite. Even code written by the framework's own authors, with full test coverage, has room for a few more honest names and one fewer unencapsulated if.