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;
}
Но и такое решение оставляет желать лучшего. Передача аргумента prefixIndex выглядит нелогично [G32]. Она устанавливает порядок вызова, но никоим образом не объясняет необходимость именно такого порядка. Другой программист может отменить внесенное изменение, так как ничто не указывает на то, что этот параметр действительно необходим.
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;
}
Функции findCommonPrefix и findCommonSuffix возвращаются к прежнему виду, функция findCommonSuffix переименовывается в findCommonPrefixAndSuffix, и в нее включается вызов findCommonPrefix до выполнения каких-либо других действий. Тем самым временная связь двух функций устанавливается гораздо более радикально, чем в предыдущем решении. Кроме того, новое решение со всей очевидностью демонстрирует, насколько уродлива функция findCommonPrefixAndSuffix. Давайте немного почистим ее.
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;
}
Так гораздо лучше. Новая версия кода очевидно показывает, что suffixIndex в действительности определяет длину суффикса, а прежнее имя было выбрано неудачно. Это относится и к prefixIndex, хотя в данном случае «индекс» и «длина» являются синонимами. Несмотря на это, использование термина «длина» выглядит более последовательно. Проблема в том, что значение переменной suffixIndex отсчитывается не от 0, а от 1, так что называть его «длиной» не совсем корректно (кстати, этим же обстоятельством объясняются загадочные прибавления +1 в computeCommonSuffix [G33]). Давайте исправим этот недостаток. Результат показан в листинге 15.4.
Листинг 15.4. ComparisonCompactor.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 : "");
}
Все +1 в computeCommonSuffix были заменены на -1 в charFromEnd, где это смотрится абсолютно логично; также были изменены два оператора <= в suffixOverlapsPrefix, где это тоже абсолютно логично. Это позволило переименовать suffixIndex в suffixLength, с заметным улучшением удобочитаемости кода.
Однако здесь возникла одна проблема. В ходе устранения +1 я заметил в compactString следующую строку:
if (suffixLength > 0)
Найдите ее в листинге 15.4. Так как suffixLength стало на 1 меньше, чем было прежде, мне следовало бы заменить оператор > оператором >=, но это выглядит нелогично. При более внимательном анализе мы видим, что команда if предотвращает присоединение суффикса с нулевой длиной. Но до внесения изменений команда if была бесполезной, потому что значение suffixIndex не могло быть меньше 1!
Это ставит под сомнение полезность обеих команд if в compactString! Похоже, обе команды можно исключить. Закомментируем их и проведем тестирование. Тесты прошли! Давайте изменим структуру compactString, чтобы удалить лишние команды if и значительно упростить самую функцию [G9].