(s1.length() - sfx + 1 < s1.length() - ctxt ? "..." : ""));
}
return result;
}
}
Авторы оставили эту модуль в очень хорошей форме. И все же «правило бойскаута[71]» гласит: все нужно оставлять чище, чем было до вашего прихода. Итак, как же улучшить исходный код в листинге 15.2?
Первое, что мне решительно не понравилось, — префикс f у имен переменных классов [N6]. В современных средах разработки подобное кодирование области видимости излишне. Давайте уберем все префиксы:
private int contextLength;
private String expected;
private String actual;
private int prefix;
private int suffix;
Также бросается в глаза неинкапсулированная условная команда в начале функции compact [G28].
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);
}
Инкапсуляция поможет лучше выразить намерения разработчика. Поэтому я создал метод с именем, поясняющим его смысл:
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();
}
Запись this.expected и this.actual в функции compact тоже оставляет желать лучшего. Это произошло, когда мы переименовали fExpected в expected. Зачем в функции используются переменные с именами, совпадающими с именами переменных класса? Ведь они имеют разный смысл [N4]? Неоднозначность в именах следует исключить.
String compactExpected = compactString(expected);
String compactActual = compactString(actual);
Отрицательные условия чуть сложнее для понимания, чем положительные [G29]. Чтобы проверяемое условие стало более понятным, мы инвертируем его:
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();
}
Имя функции compact выглядит немного странно [N7]. Хотя она выполняет сжатие строк, этого не произойдет, если canBeCompacted вернет false. Таким образом, выбор имени compact скрывает побочный эффект проверки. Также обратите внимание на то, что функция возвращает отформатированное сообщение, а не просто сжатые строки. Следовательно, функцию было бы правильнее назвать formatCompactedComparison. В этом случае она гораздо лучше читается вместе с аргументом:
public String formatCompactedComparison(String message) {
Тело команды if — то место, где выполняется фактическое сжатие строк expected и actual. Мы извлечем этот код в метод compactExpectedAndActual. Тем не менее все форматирование должно происходить в функции formatCompactedComparison. Функция compact... не должна делать ничего, кроме сжатия [G30]. Разобьем ее следующим образом:
...
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);
}
Обратите внимание: это преобразование заставило нас повысить compactExpected и compactActual до переменных класса. Еще мне не нравится то, что в двух последних строках новой функции возвращаются переменные, а в первых двух — нет. Это противоречит рекомендациям по использованию единых конвенций [G11]. Значит, функции findCommonPrefix и findCommonSuffix следует изменить так, чтобы они возвращали значения префикса и суффикса.
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;
}
Также следует изменить имена переменных класса так, чтобы они стали чуть более точными [N1]; в конце концов, обе переменные представляют собой индексы.
Тщательное изучение findCommonSuffix выявляет скрытую временную привязку [G31]; работа функции зависит от того, что значение prefixIndex вычисляется функцией findCommonPrefix. Если вызвать эти две функции в неверном порядке, вам предстоит непростой сеанс отладки. Чтобы эта временная привязка стала очевидной, значение prefixIndex будет передаваться при вызове findCommonSuffix в аргументе.