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;