Appearance
第 15 章 JUnit Internals JUnit 内幕
JUnit is one of the most famous of all Java frameworks. As frameworks go, it is simple in conception, precise in definition, and elegant in implementation. But what does the code look like? In this chapter we'll critique an example drawn from the JUnit framework.
JUnit 是所有 Java 框架中最著名的框架之一。就框架而言，它的概念简洁，定义精确，实现优雅。但它的代码长什么样呢？在本章中，我们将审视来自 JUnit 框架的一个示例。
15.1 THE JUNIT FRAMEWORK JUnit 框架
JUnit has had many authors, but it began with Kent Beck and Eric Gamma together on a plane to Atlanta. Kent wanted to learn Java, and Eric wanted to learn about Kent's Smalltalk testing framework. "What could be more natural to a couple of geeks in cramped quarters than to pull out our laptops and start coding?"^1 After three hours of high-altitude work, they had written the basics of JUnit.
JUnit 有许多作者，但它始于 Kent Beck 和 Eric Gamma 共同搭乘的一趟飞往亚特兰大的航班。Kent 想学习 Java，而 Eric 想了解 Kent 的 Smalltalk 测试框架。"对于两个挤在狭小空间里的极客来说，还有什么比掏出笔记本电脑开始编程更自然的事呢？"^[1] 经过三个小时的高空作业，他们编写出了 JUnit 的基础。
- JUnit Pocket Guide, Kent Beck, O'Reilly, 2004, p. 43.
[1] 《JUnit Pocket Guide》，Kent Beck 著，O'Reilly 出版，2004 年，第 43 页。
The module we'll look at is the clever bit of code that helps identify string comparison errors. This module is called ComparisonCompactor. Given two strings that differ, such as ABCDE and ABXDE, it will expose the difference by generating a string such as <...B[X]D...>.
我们要审视的模块是一段巧妙的代码，用于帮助识别字符串比较错误。这个模块叫做 ComparisonCompactor。给定两个不同的字符串，例如 ABCDE 和 ABXDE，它会通过生成类似
<...B[X]D...>的字符串来暴露差异。
I could explain it further, but the test cases do a better job. So take a look at Listing 15-1 and you will understand the requirements of this module in depth. While you are at it, critique the structure of the tests. Could they be simpler or more obvious?
我可以做更多解释，但测试用例能更好地说明问题。所以请看一下代码清单 15-1，你将深入了解这个模块的需求。与此同时，审视一下这些测试的结构。它们能否更简单或更直观呢？
Listing 15-1 ComparisonCompactorTest.java
代码清单 15-1 ComparisonCompactorTest.java
java
package junit.tests.framework;
import junit.framework.ComparisonCompactor;
import junit.framework.TestCase;
public class ComparisonCompactorTest extends TestCase {
public void testMessage() {
String failure= new ComparisonCompactor(0, "b", "c").compact("a");
assertTrue("a expected:<[b]> but was:<[c]>".equals(failure));
}
public void testStartSame() {
String failure= new ComparisonCompactor(1, "ba", "bc").compact(null);
assertEquals("expected:<b[a]> but was:<b[c]>", failure);
}
public void testEndSame() {
String failure= new ComparisonCompactor(1, "ab", "cb").compact(null);
assertEquals("expected:<[a]b> but was:<[c]b>", failure);
}
public void testSame() {
String failure= new ComparisonCompactor(1, "ab", "ab").compact(null);
assertEquals("expected:<ab> but was:<ab>", failure);
}
public void testNoContextStartAndEndSame() {
String failure= new ComparisonCompactor(0, "abc", "adc").compact(null);
assertEquals("expected:<...[b]...> but was:<...[d]...>", failure);
}
public void testStartAndEndContext() {
String failure= new ComparisonCompactor(1, "abc", "adc").compact(null);
assertEquals("expected:<a[b]c> but was:<a[d]c>", failure);
}
public void testStartAndEndContextWithEllipses() {
String failure=
new ComparisonCompactor(1, "abcde", "abfde").compact(null);
assertEquals("expected:<...b[c]d...> but was:<...b[f]d...>", failure);
}
public void testComparisonErrorStartSameComplete() {
String failure= new ComparisonCompactor(2, "ab", "abc").compact(null);
assertEquals("expected:<ab[]> but was:<ab[c]>", failure);
}
public void testComparisonErrorEndSameComplete() {
String failure= new ComparisonCompactor(0, "bc", "abc").compact(null);
assertEquals("expected:<[]...> but was:<[a]...>", failure);
}
public void testComparisonErrorEndSameCompleteContext() {
String failure= new ComparisonCompactor(2, "bc", "abc").compact(null);
assertEquals("expected:<[]bc> but was:<[a]bc>", failure);
}
public void testComparisonErrorOverlapingMatches() {
String failure= new ComparisonCompactor(0, "abc", "abbc").compact(null);
assertEquals("expected:<...[]...> but was:<...[b]...>", failure);
}
public void testComparisonErrorOverlapingMatchesContext() {
String failure= new ComparisonCompactor(2, "abc", "abbc").compact(null);
assertEquals("expected:<ab[]c> but was:<ab[b]c>", failure);
}
public void testComparisonErrorOverlapingMatches2() {
String failure= new ComparisonCompactor(0, "abcdde",
"abcde").compact(null);
assertEquals("expected:<...[d]...> but was:<...[]...>", failure);
}
public void testComparisonErrorOverlapingMatches2Context() {
String failure=
new ComparisonCompactor(2, "abcdde", "abcde").compact(null);
assertEquals("expected:<...cd[d]e> but was:<...cd[]e>", failure);
}
public void testComparisonErrorWithActualNull() {
String failure= new ComparisonCompactor(0, "a", null).compact(null);
assertEquals("expected:<a> but was:<null>", failure);
}
public void testComparisonErrorWithActualNullContext() {
String failure= new ComparisonCompactor(2, "a", null).compact(null);
assertEquals("expected:<a> but was:<null>", failure);
}
public void testComparisonErrorWithExpectedNull() {
String failure= new ComparisonCompactor(0, null, "a").compact(null);
assertEquals("expected:<null> but was:<a>", failure);
}
public void testComparisonErrorWithExpectedNullContext() {
String failure= new ComparisonCompactor(2, null, "a").compact(null);
assertEquals("expected:<null> but was:<a>", failure);
}
public void testBug609972() {
String failure= new ComparisonCompactor(10, "S&P500", "0").compact(null);
assertEquals("expected:<[S&P50]0> but was:<[]0>", failure);
}
}I ran a code coverage analysis on the ComparisonCompactor using these tests. The code is 100 percent covered. Every line of code, every if statement and for loop, is executed by the tests. This gives me a high degree of confidence that the code works and a high degree of respect for the craftsmanship of the authors.
我使用这些测试对 ComparisonCompactor 进行了代码覆盖率分析。代码达到了 100% 的覆盖率。每一行代码、每一个 if 语句和 for 循环都被测试执行到了。这让我对代码的正确性有很高的信心，也对作者的技艺深感钦佩。
The code for ComparisonCompactor is in Listing 15-2. Take a moment to look over this code. I think you'll find it to be nicely partitioned, reasonably expressive, and simple in structure. Once you are done, then we'll pick the nits together.
ComparisonCompactor 的代码在代码清单 15-2 中。请花点时间浏览一下这段代码。你会发现它划分合理、表达清晰、结构简洁。等你看完后，我们将一起来挑挑它的毛病。
Listing 15-2 ComparisonCompactor.java (original)
代码清单 15-2 ComparisonCompactor.java（原始版本）
java
package junit.framework;
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);
}
}You might have a few complaints about this module. There are some long expressions and some strange +1s and so forth. But overall this module is pretty good. After all, it might have looked like Listing 15-3.
你可能对这个模块有一些意见。有一些冗长的表达式，还有一些奇怪的 +1 等等。但总体来说，这个模块还是相当不错的。毕竟，它本可能看起来像代码清单 15-3 那样。
Listing 15-3 ComparisonCompactor.java (defactored)
代码清单 15-3 ComparisonCompactor.java（反重构版本）
java
package junit.framework;
public class ComparisonCompactor {
private int ctxt;
private String s1;
private String s2;
private int pfx;
private int sfx;
public ComparisonCompactor(int ctxt, String s1, String s2) {
this.ctxt = ctxt;
this.s1 = s1;
this.s2 = s2;
}
public String compact(String msg) {
if (s1 == null || s2 == null || s1.equals(s2))
return Assert.format(msg, s1, s2);
pfx = 0;
for (; pfx < Math.min(s1.length(), s2.length()); pfx++) {
if (s1.charAt(pfx) != s2.charAt(pfx))
break;
}
int sfx1 = s1.length() - 1;
int sfx2 = s2.length() - 1;
for (; sfx2 >= pfx && sfx1 >= pfx; sfx2--, sfx1--) {
if (s1.charAt(sfx1) != s2.charAt(sfx2))
break;
}
sfx = s1.length() - sfx1;
String cmp1 = compactString(s1);
String cmp2 = compactString(s2);
return Assert.format(msg, cmp1, cmp2);
}
private String compactString(String s) {
String result =
"[" + s.substring(pfx, s.length() - sfx + 1) + "]";
if (pfx > 0)
result = (pfx > ctxt ? "..." : "") +
s1.substring(Math.max(0, pfx - ctxt), pfx) + result;
if (sfx > 0) {
int end = Math.min(s1.length() - sfx + 1 + ctxt, s1.length());
result = result + (s1.substring(s1.length() - sfx + 1, end) +
(s1.length() - sfx + 1 < s1.length() - ctxt ? "..." : ""));
}
return result;
}
}Even though the authors left this module in very good shape, the Boy Scout Rule^2 tells us we should leave it cleaner than we found it. So, how can we improve on the original code in Listing 15-2?
即使作者把这个模块的状况保持得非常好，童子军规则^[2] 告诉我们应该让它比我们发现时更整洁。那么，我们如何改进代码清单 15-2 中的原始代码呢？
- See "The Boy Scout Rule" on page 14.
[2] 参见第 14 页的"童子军规则"。
The first thing I don't care for is the f prefix for the member variables [N6]. Today's environments make this kind of scope encoding redundant. So let's eliminate all the f's.
我首先不喜欢的是成员变量的 f 前缀 [N6]。如今的开发环境使得这种作用域编码变得多余。所以让我们去掉所有的 f。
java
private int contextLength;
private String expected;
private String actual;
private int prefix;
private int suffix;Next, we have an unencapsulated conditional at the beginning of the compact function [G28].
接下来，compact 函数开头有一个未封装的条件判断 [G28]。
java
public String compact(String message) {
if (expected == null || actual == null || areStringsEqual())
return Assert.format(message, expected, actual);
findCommonPrefix();
findCommonSuffix();
String expected = compactString(this.expected);
String actual = compactString(this.actual);
return Assert.format(message, expected, actual);
}This conditional should be encapsulated to make our intent clear. So let's extract a method that explains it.
这个条件判断应该被封装起来，使我们的意图更加清晰。所以让我们提取一个方法来解释它。
java
public String compact(String message) {
if (shouldNotCompact())
return Assert.format(message, expected, actual);
findCommonPrefix();
findCommonSuffix();
String expected = compactString(this.expected);
String actual = compactString(this.actual);
return Assert.format(message, expected, actual);
}
private boolean shouldNotCompact() {
return expected == null || actual == null || areStringsEqual();
}I don't much care for the this.expected and this.actual notation in the compact function. This happened when we changed the name of fExpected to expected. Why are there variables in this function that have the same names as the member variables? Don't they represent something else [N4]? We should make the names unambiguous.
我不太喜欢 compact 函数中的 this.expected 和 this.actual 这种写法。这是在我们把 fExpected 改名为 expected 时产生的。为什么这个函数中会有与成员变量同名的变量？它们难道不代表其他东西吗 [N4]？我们应该让名称更加明确。
java
String compactExpected = compactString(expected);
String compactActual = compactString(actual);Negatives are slightly harder to understand than positives [G29]. So let's turn that if statement on its head and invert the sense of the conditional.
否定形式比肯定形式更难理解 [G29]。所以让我们把那个 if 语句翻转过来，反转条件的含义。
java
public String compact(String message) {
if (canBeCompacted()) {
findCommonPrefix();
findCommonSuffix();
String compactExpected = compactString(expected);
String compactActual = compactString(actual);
return Assert.format(message, compactExpected, compactActual);
} else {
return Assert.format(message, expected, actual);
}
}
private boolean canBeCompacted() {
return expected != null && actual != null && ! areStringsEqual();
}The name of the function is strange [N7]. Although it does compact the strings, it actually might not compact the strings if canBeCompacted returns false. So naming this function compact hides the side effect of the error check. Notice also that the function returns a formatted message, not just the compacted strings. So the name of the function should really be formatCompactedComparison. That makes it read a lot better when taken with the function argument:
函数的名称很奇怪 [N7]。虽然它确实会压缩字符串，但如果 canBeCompacted 返回 false，它实际上可能不会压缩字符串。所以将这个函数命名为 compact 隐藏了错误检查的副作用。还要注意，这个函数返回的是格式化的消息，而不仅仅是压缩后的字符串。所以函数的名称实际上应该是 formatCompactedComparison。这样与函数参数一起使用时读起来会好得多：
java
public String formatCompactedComparison(String message) {The body of the if statement is where the true compacting of the expected and actual strings is done. We should extract that as a method named compactExpectedAndActual. However, we want the formatCompactedComparison function to do all the formatting. The compact... function should do nothing but compacting [G30]. So let's split it up as follows:
if 语句的主体才是真正压缩 expected 和 actual 字符串的地方。我们应该把它提取为一个名为 compactExpectedAndActual 的方法。然而，我们希望 formatCompactedComparison 函数完成所有格式化工作。compact... 函数应该只做压缩，不做其他事情 [G30]。所以让我们按如下方式拆分：
java
…
private String compactExpected;
private String compactActual;
…
public String formatCompactedComparison(String message) {
if (canBeCompacted()) {
compactExpectedAndActual();
return Assert.format(message, compactExpected, compactActual);
} else {
return Assert.format(message, expected, actual);
}
}
private void compactExpectedAndActual() {
findCommonPrefix();
findCommonSuffix();
compactExpected = compactString(expected);
compactActual = compactString(actual);
}Notice that this required us to promote compactExpected and compactActual to member variables. I don't like the way that the last two lines of the new function return variables, but the first two don't. They aren't using consistent conventions [G11]. So we should change findCommonPrefix and findCommonSuffix to return the prefix and suffix values.
注意，这要求我们将 compactExpected 和 compactActual 提升为成员变量。我不喜欢新函数中最后两行返回变量值而前两行不返回的方式。它们没有使用一致的惯例 [G11]。所以我们应该让 findCommonPrefix 和 findCommonSuffix 返回 prefix 和 suffix 的值。
java
private void compactExpectedAndActual() {
prefixIndex = findCommonPrefix();
suffixIndex = findCommonSuffix();
compactExpected = compactString(expected);
compactActual = compactString(actual);
}
private int findCommonPrefix() {
int prefixIndex = 0;
int end = Math.min(expected.length(), actual.length());
for (; prefixIndex < end; prefixIndex++) {
if (expected.charAt(prefixIndex) != actual.charAt(prefixIndex))
break;
}
return prefixIndex;
}
private int findCommonSuffix() {
int expectedSuffix = expected.length() - 1;
int actualSuffix = actual.length() - 1;
for (; actualSuffix >= prefixIndex && expectedSuffix >= prefixIndex;
actualSuffix--, expectedSuffix--) {
if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix))
break;
}
return expected.length() - expectedSuffix;
}We should also change the names of the member variables to be a little more accurate [N1]; after all, they are both indices.
我们还应该将成员变量的名称改得更准确一些 [N1]；毕竟，它们都是索引。
Careful inspection of findCommonSuffix exposes a hidden temporal coupling [G31]; it depends on the fact that prefixIndex is calculated by findCommonPrefix. If these two functions were called out of order, there would be a difficult debugging session ahead. So, to expose this temporal coupling, let's have findCommonSuffix take the prefixIndex as an argument.
仔细检查 findCommonSuffix 会发现一个隐藏的时序耦合 [G31]；它依赖于 prefixIndex 由 findCommonPrefix 计算这一事实。如果这两个函数被以错误的顺序调用，将会面临一段艰难的调试过程。所以，为了暴露这个时序耦合，让我们让 findCommonSuffix 接受 prefixIndex 作为参数。
java
private void compactExpectedAndActual() {
prefixIndex = findCommonPrefix();
suffixIndex = findCommonSuffix(prefixIndex);
compactExpected = compactString(expected);
compactActual = compactString(actual);
}
private int findCommonSuffix(int prefixIndex) {
int expectedSuffix = expected.length() - 1;
int actualSuffix = actual.length() - 1;
for (; actualSuffix >= prefixIndex && expectedSuffix >= prefixIndex;
actualSuffix--, expectedSuffix--) {
if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix))
break;
}
return expected.length() - expectedSuffix;
}I'm not really happy with this. The passing of the prefixIndex argument is a bit arbitrary [G32]. It works to establish the ordering but does nothing to explain the need for that ordering. Another programmer might undo what we have done because there's no indication that the parameter is really needed. So let's take a different tack.
我对这个结果并不太满意。传递 prefixIndex 参数有点随意 [G32]。它确实建立了顺序，但对于解释为何需要这个顺序毫无帮助。另一个程序员可能会撤销我们所做的，因为没有任何迹象表明这个参数是真正需要的。所以让我们换个方法。
java
private void compactExpectedAndActual() {
findCommonPrefixAndSuffix();
compactExpected = compactString(expected);
compactActual = compactString(actual);
}
private void findCommonPrefixAndSuffix() {
findCommonPrefix();
int expectedSuffix = expected.length() - 1;
int actualSuffix = actual.length() - 1;
for (;
actualSuffix >= prefixIndex && expectedSuffix >= prefixIndex;
actualSuffix--, expectedSuffix--
) {
if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix))
break;
}
suffixIndex = expected.length() - expectedSuffix;
}
private void findCommonPrefix() {
prefixIndex = 0;
int end = Math.min(expected.length(), actual.length());
for (; prefixIndex < end; prefixIndex++)
if (expected.charAt(prefixIndex) != actual.charAt(prefixIndex))
break;
}We put findCommonPrefix and findCommonSuffix back the way they were, changing the name of findCommonSuffix to findCommonPrefixAndSuffix and having it call findCommonPrefix before doing anything else. That establishes the temporal nature of the two functions in a much more dramatic way than the previous solution. It also points out how ugly findCommonPrefixAndSuffix is. Let's clean it up now.
我们把 findCommonPrefix 和 findCommonSuffix 恢复原样，将 findCommonSuffix 的名称改为 findCommonPrefixAndSuffix，并让它在执行其他操作之前先调用 findCommonPrefix。这比之前的解决方案更强烈地确立了两个函数之间的时序关系。它也指出了 findCommonPrefixAndSuffix 有多么丑陋。现在让我们来清理它。
java
private void findCommonPrefixAndSuffix() {
findCommonPrefix();
int suffixLength = 1;
for (; !suffixOverlapsPrefix(suffixLength); suffixLength++) {
if (charFromEnd(expected, suffixLength) !=
charFromEnd(actual, suffixLength))
break;
}
suffixIndex = suffixLength;
}
private char charFromEnd(String s, int i) {
return s.charAt(s.length()-i);}
private boolean suffixOverlapsPrefix(int suffixLength) {
return actual.length() - suffixLength < prefixLength ||
expected.length() - suffixLength < prefixLength;
}This is much better. It exposes that the suffixIndex is really the length of the suffix and is not well named. The same is true of the prefixIndex, though in that case "index" and "length" are synonymous. Even so, it is more consistent to use "length." The problem is that the suffixIndex variable is not zero based; it is 1 based and so is not a true length. This is also the reason that there are all those +1s in computeCommonSuffix [G33]. So let's fix that. The result is in Listing 15-4.
这样好多了。它暴露了 suffixIndex 实际上是后缀的长度，名称并不恰当。prefixIndex 也是如此，虽然在这种情况下"index"和"length"是同义的。即便如此，使用"length"更加一致。问题是 suffixIndex 变量不是从零开始的；它是从 1 开始的，因此不是真正的长度。这也是 computeCommonSuffix 中所有那些 +1 的原因 [G33]。所以让我们修复它。结果在代码清单 15-4 中。
Listing 15-4 ComparisonCompactor.java (interim)
代码清单 15-4 ComparisonCompactor.java（中间版本）
java
public class ComparisonCompactor {
…
private int suffixLength;
…
private void findCommonPrefixAndSuffix() {
findCommonPrefix();
suffixLength = 0;
for (; !suffixOverlapsPrefix(suffixLength); suffixLength++) {
if (charFromEnd(expected, suffixLength) !=
charFromEnd(actual, suffixLength))
break;
}
}
private char charFromEnd(String s, int i) {
return s.charAt(s.length() - i - 1);
}
private boolean suffixOverlapsPrefix(int suffixLength) {
return actual.length() - suffixLength <= prefixLength ||
expected.length() - suffixLength <= prefixLength;
}
…
private String compactString(String source) {
String result =
DELTA_START +
source.substring(prefixLength, source.length() - suffixLength) +
DELTA_END;
if (prefixLength > 0)
result = computeCommonPrefix() + result;
if (suffixLength > 0)
result = result + computeCommonSuffix();
return result;
}
…
private String computeCommonSuffix() {
int end = Math.min(expected.length() - suffixLength +
contextLength, expected.length()
);
return
expected.substring(expected.length() - suffixLength, end) +
(expected.length() - suffixLength <
expected.length() - contextLength ?
ELLIPSIS : "");
}We replaced the +1s in computeCommonSuffix with a -1 in charFromEnd, where it makes perfect sense, and two <= operators in suffixOverlapsPrefix, where they also make perfect sense. This allowed us to change the name of suffixIndex to suffixLength, greatly enhancing the readability of the code.
我们用 charFromEnd 中的 -1（在那里完全合理）和 suffixOverlapsPrefix 中的两个 <= 运算符（在那里同样合理）替换了 computeCommonSuffix 中的 +1。这使得我们可以将 suffixIndex 重命名为 suffixLength，大大增强了代码的可读性。
There is a problem however. As I was eliminating the +1s, I noticed the following line in compactString:
然而有一个问题。当我消除那些 +1 时，我注意到 compactString 中有这样一行：
java
if (suffixLength > 0)Take a look at it in Listing 15-4. By rights, because suffixLength is now one less than it used to be, I should change the > operator to a >= operator. But that makes no sense. It makes sense now! This means that it didn't use to make sense and was probably a bug. Well, not quite a bug. Upon further analysis we see that the if statement now prevents a zero length suffix from being appended. Before we made the change, the if statement was nonfunctional because suffixIndex could never be less than one!
看看代码清单 15-4 中的这行。按理说，因为 suffixLength 现在比以前小了一，我应该把 > 运算符改成 >= 运算符。但这说不通。现在这样才说得通！这意味着以前说不通，可能是个 bug。嗯，不完全是 bug。进一步分析后我们看到，这个 if 语句现在能防止将零长度的后缀附加上去。在我们做改动之前，这个 if 语句是没有实际作用的，因为 suffixIndex 永远不会小于 1！
This calls into question both if statements in compactString! It looks as though they could both be eliminated. So let's comment them out and run the tests. They passed! So let's restructure compactString to eliminate the extraneous if statements and make the function much simpler [G9].
这让人质疑 compactString 中的两个 if 语句！看起来它们都可以被消除。所以让我们把它们注释掉并运行测试。测试通过了！所以让我们重构 compactString，消除多余的 if 语句，使函数更加简洁 [G9]。
java
private String compactString(String source) {
return
computeCommonPrefix() +
DELTA_START +
source.substring(prefixLength, source.length() - suffixLength) +
DELTA_END +
computeCommonSuffix();
}This is much better! Now we see that the compactString function is simply composing the fragments together. We can probably make this even clearer. Indeed, there are lots of little cleanups we could do. But rather than drag you through the rest of the changes, I'll just show you the result in Listing 15-5.
这样好多了！现在我们看到 compactString 函数只是简单地将各个片段组合在一起。我们可能还能让它更清晰。确实，还有很多小的清理工作可以做。但与其拖着你看完其余的改动，不如直接展示代码清单 15-5 中的最终结果。
Listing 15-5 ComparisonCompactor.java (final)
代码清单 15-5 ComparisonCompactor.java（最终版本）
java
package junit.framework;
public class ComparisonCompactor {
private static final String ELLIPSIS = "...";
private static final String DELTA_END = "]";
private static final String DELTA_START = "[";
private int contextLength;
private String expected;
private 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 (shouldBeCompacted()) {
findCommonPrefixAndSuffix();
compactExpected = compact(expected);
compactActual = compact(actual);
}
return Assert.format(message, compactExpected, compactActual);
}
private boolean shouldBeCompacted() {
return !shouldNotBeCompacted();
}
private boolean shouldNotBeCompacted() {
return expected == null ||
actual == null ||
expected.equals(actual);
}
private void findCommonPrefixAndSuffix() {
findCommonPrefix();
suffixLength = 0;
for (; !suffixOverlapsPrefix(); suffixLength++) {
if (charFromEnd(expected, suffixLength) !=
charFromEnd(actual, suffixLength)
)
break;
}
}
private char charFromEnd(String s, int i) {
return s.charAt(s.length() - i - 1);
}
private boolean suffixOverlapsPrefix() {
return actual.length() - suffixLength <= prefixLength ||
expected.length() - suffixLength <= prefixLength;
}
private void findCommonPrefix() {
prefixLength = 0;
int end = Math.min(expected.length(), actual.length());
for (; prefixLength < end; prefixLength++)
if (expected.charAt(prefixLength) != actual.charAt(prefixLength))
break;
}
private String compact(String s) {
return new StringBuilder()
.append(startingEllipsis())
.append(startingContext())
.append(DELTA_START)
.append(delta(s))
.append(DELTA_END)
.append(endingContext())
.append(endingEllipsis())
.toString();
}
private String startingEllipsis() {
return prefixLength > contextLength ? ELLIPSIS : "";
}
private String startingContext() {
int contextStart = Math.max(0, prefixLength - contextLength);
int contextEnd = prefixLength;
return expected.substring(contextStart, contextEnd);
}
private String delta(String s) {
int deltaStart = prefixLength;
int deltaEnd = s.length() - suffixLength;
return s.substring(deltaStart, deltaEnd);
}
private String endingContext() {
int contextStart = expected.length() - suffixLength;
int contextEnd =
Math.min(contextStart + contextLength, expected.length());
return expected.substring(contextStart, contextEnd);
}
private String endingEllipsis() {
return (suffixLength > contextLength ? ELLIPSIS : "");
}
}This is actually quite pretty. The module is separated into a group of analysis functions and another group of synthesis functions. They are topologically sorted so that the definition of each function appears just after it is used. All the analysis functions appear first, and all the synthesis functions appear last.
这实际上相当漂亮。这个模块被分成一组分析函数和另一组合成函数。它们按拓扑排序，使得每个函数的定义出现在它被使用的地方之后。所有的分析函数出现在前面，所有的合成函数出现在后面。
If you look carefully, you will notice that I reversed several of the decisions I made earlier in this chapter. For example, I inlined some extracted methods back into formatCompactedComparison, and I changed the sense of the shouldNotBeCompacted expression. This is typical. Often one refactoring leads to another that leads to the undoing of the first. Refactoring is an iterative process full of trial and error, inevitably converging on something that we feel is worthy of a professional.
如果你仔细观察，会注意到我反转了本章前面做出的几个决定。例如，我把一些提取出来的方法内联回了 formatCompactedComparison，并且改变了 shouldNotBeCompacted 表达式的含义。这是很典型的。通常一次重构会导致另一次重构，而后者又会导致撤销前一次的改动。重构是一个充满试错的迭代过程，不可避免地会收敛到我们觉得配得上专业人士水准的结果。
15.2 CONCLUSION 结论
And so we have satisfied the Boy Scout Rule. We have left this module a bit cleaner than we found it. Not that it wasn't clean already. The authors had done an excellent job with it. But no module is immune from improvement, and each of us has the responsibility to leave the code a little better than we found it.
于是我们满足了童子军规则。我们让这个模块比发现时更整洁了一些。并不是说它原本就不整洁。作者们做得非常出色。但没有任何模块能免于改进，我们每个人都有责任让代码比发现时稍微好一点。