Настройки текста:
Посвящается Анне-Марии — бессмертной любви всей моей жизни
Джеймс О. КоплинМёрруп, Дания
Бьёрн Страуструп, создатель C++ и автор книги «The C++ Programming Language»
Я люблю, чтобы мой код был элегантным и эффективным. Логика должны быть достаточно прямолинейной, чтобы ошибкам было трудно спрятаться; зависимости — минимальными, чтобы упростить сопровождение; обработка ошибок — полной в соответствии с выработанной стратегией; а производительность — близкой к оптимальной, чтобы не искушать людей загрязнять код беспринципными оптимизациями. Чистый код хорошо решает одну задачу.
Грэди Буч, автор книги «Object Oriented Analysis and Design with Applications»
Чистый код прост и прямолинеен. Чистый код читается, как хорошо написанная проза. Чистый код никогда не затемняет намерения проектировщика; он полон четких абстракций и простых линий передачи управления.
«Большой» Дэйв Томас, основатель OTI, крестный отец стратегии Eclipse
Чистый код может читаться и усовершенствоваться другими разработчиками, кроме его исходного автора. Для него написаны модульные и приемочные тесты. В чистом коде используются содержательные имена. Для выполнения одной операции в нем используется один путь (вместо нескольких разных). Чистый код обладает минимальными зависимостями, которые явно определены, и четким, минимальным API. Код должен быть грамотным, потому что в зависимости от языка не вся необходимая информация может быть четко выражена в самом коде.
Майкл Физерс, автор книги «Working Effectively with Legacy Code»
Я мог бы перечислить все признаки, присущие чистому коду, но существует один важнейший признак, из которого следуют все остальные. Чистый код всегда выглядит так, словно его автор над ним тщательно потрудился. Вы не найдете никаких очевидных возможностей для его улучшения. Все они уже были продуманы автором кода. Попытавшись представить возможные усовершенствования, вы снова придете к тому, с чего все началось: вы рассматриваете код, тщательно продуманный и написанный настоящим мастером, небезразличным к своему ремеслу.
Карьера Рона началась с программирования на языке Fortran. С тех пор он писал код практически на всех языках и на всех компьютерах. К его словам стоит прислушаться.Рон Джеффрис, автор книг «Extreme Programming Installed» и «Extreme Programming Adventures in C#»
За последние коды я постоянно руководствуюсь «правилами простого кода», сформулированными Беком. В порядке важности, простой код: — проходит все тесты; — не содержит дубликатов; — выражает все концепции проектирования, заложенные в систему; — содержит минимальное количество сущностей: классов, методов, функций и т.д. Из всех правил я уделяю основное внимание дублированию. Если что-то делается в программе снова и снова, это свидетельствует о том, что какая-то мысленная концепция не нашла представления в коде. Я пытаюсь понять, что это такое, а затем пытаюсь выразить идею более четко. Выразительность для меня прежде всего означает содержательность имен. Обычно я провожу переименования по несколько раз, пока не остановлюсь на окончательном варианте. В современных средах программирования — таких, как Eclipse — переименование выполняется легко, поэтому изменения меня не беспокоят. Впрочем, выразительность не ограничивается одними лишь именами. Я также смотрю, не выполняет ли объект или метод более одной операции. Если это объект, то его, вероятно, стоит разбить на два и более объекта. Если это метод, я всегда применяю к нему прием «извлечения метода»; в итоге у меня остается основной метод, который более четко объясняет, что он делает, и несколько подметодов, объясняющих, как он это делает. Отсутствие дублирования и выразительности являются важнейшими составляющими чистого кода в моем понимании. Даже если при улучшении грязного кода вы будете руководствоваться только этими двумя целями, разница в качестве кода может быть огромной. Однако существует еще одна цель, о которой я также постоянно помню, хотя объяснить ее будет несколько сложнее. После многолетней работы мне кажется, что все программы состоят из очень похожих элементов. Для примера возьмем операцию «найти элемент в коллекции». Независимо от того, работаем ли мы с базой данных, содержащий информацию о работниках, или хеш-таблицей с парами «ключ-значение», или массивом с однотипными объектами, на практике часто возникает задача извлечь конкретный элемент из этой коллекции. В подобных ситуациях я часто инкапсулирую конкретную реализацию в более абстрактном методе или классе. Это открывает пару интересных возможностей. Я могу определить для нужной функциональности какую-нибудь простую реализацию (например, хеш-таблицу), но поскольку все ссылки прикрыты моей маленькой абстракцией, реализацию можно в любой момент изменить. Я могу быстро двигаться вперед, сохраняя возможность внести изменения позднее. Кроме того, абстракция часто привлекает мое внимание к тому, что же «действительно» происходит в программе, и удерживает меня от реализации поведения коллекций там, где в действительности достаточно более простых способов получения нужной информации. Сокращение дублирования, высокая выразительность и раннее построение простых абстракций. Все это составляет чистый код в моем понимании.
Уорд Каннингем, создатель Wiki, создатель Fit, один из создателей экстремального программирования. Вдохновитель написания книги «Design Patterns». Духовный лидер Smalltalk и объектно-ориентированного подхода. Крестный отец всех, кто тщательно относится к написанию кода.
Вы работаете с чистым кодом, если каждая функция делает примерно то, что вы ожидали. Код можно назвать красивым, если у вас также создается впечатление, что язык был создан специально для этой задачи.
Тим Оттингер
int d; // Прошедшее время
Имя d не передает ровным счетом ничего. Оно не ассоциируется ни с временными интервалами, ни с днями. Его следует заменить другим именем, которое указывает, что именно измеряется и в каких единицах:
int elapsedTimeInDays;
int daysSinceCreation;
int daysSinceModification;
int fileAgeInDays;
Содержательные имена существенно упрощают понимание и модификацию кода. Например, что делает следующий фрагмент?
public List<int[]> getThem() {
List<int[]> list1 = new ArrayList<int[]>();
for (int[] x : theList)
if (x[0] == 4)
list1.add(x);
return list1;
}
Почему мы не можем сразу сказать, что делает этот код? В нем нет сложных выражений. Пробелы и отступы расставлены грамотно. В коде задействованы только три переменные и две константы. В нем нет никаких хитроумных классов или полиморфных методов, только список массивов (по крайней мере на первый взгляд).
Проблема кроется не в сложности кода, а в его неочевидности, то есть степени, в которой контекст не следует явно из самого кода. Код подразумевает, что мы знаем ответы на вопросы:
1. Какие данные хранятся в theList?
2. Чем так важен элемент theList с нулевым индексом?
3. Какой особый смысл имеет значение 4?
4. Как будет использоваться возвращаемый список?
Ответы на все эти вопросы не следуют из примера, хотя и могли бы. Допустим, мы работаем над игрой «Сапер». Игровое поле представлено в виде списка ячеек с именем theList. Переименуем его в gameBoard.
Каждая ячейка игрового поля представлена простым массивом. Далее выясняется, что в элементе с нулевым индексом хранится код состояния, а код 4 означает «флажок установлен». Даже простое присваивание имен всем этим концепциям существенно улучшает код:
public List<int[]> getFlaggedCells() {
List<int[]> flaggedCells = new ArrayList<int[]>();
for (int[] cell : gameBoard)
if (cell[STATUS_VALUE] == FLAGGED)
flaggedCells.add(cell);
return flaggedCells;
}
Обратите внимание: простота кода несколько не изменилась. Новая версия содержит точно такое же количество операторов и констант, с абсолютно таким же количеством уровней вложенности. Однако код стал существенно более понятным.
Можно пойти еще дальше и написать простой класс для представления ячеек вместо использования массива int. В класс включается функция, передающая намерения программиста (назовем ее isFlagged); она скрывает «волшебные» числа. В результате мы получаем новую версию функции:
public List<Cell> getFlaggedCells() {
List<Cell> flaggedCells = new ArrayList<Cell>();
for (Cell cell : gameBoard)
if (cell.isFlagged())
flaggedCells.add(cell);
return flaggedCells;
}
Не изменилось ничего, кроме имен — но теперь можно легко понять, что здесь происходит. Такова сила выбора хороших имен.
int a = l;
if ( O == l )
a = O1;
else
l = 01;
Возможно, некоторым читателям этот совет покажется надуманным, однако мы неоднократно видели код, в котором подобных ухищрений было предостаточно. В одном случае автор кода даже предложил использовать другой шрифт, чтобы различия стали более очевидными — в дальнейшем это решение должно было передаваться всем будущим разработчикам на словах или в письменном документе. Простое переименование решает проблему окончательно и без создания новых документов.
public static void copyChars(char a1[], char a2[]) {
for (int i = 0; i < a1.length; i++) {
a2[i] = a1[i];
}
}
Такая функция будет читаться намного лучше, если присвоить аргументам имена source и destination.
Неинформативные слова также применяются для создания бессодержательных различий. Допустим, у вас имеется класс Product. Создав другой класс с именем ProductInfo или ProductData, вы создаете разные имена, которые по сути обозначают одно и то же. Info и Data не несут полезной информации, как и артикли a, an и the.
Следует учесть, что использование префиксов a и the вовсе не является ошибкой, но только при условии, что они создают осмысленные различия. Например, префикс a может присваиваться всем локальным переменным, а префикс the — всем аргументам функций[6]. Проблема возникает тогда, когда вы называете переменную theZork, потому что в программе уже есть другая переменная с именем zork.
Неинформативные слова избыточны. Слово variable никогда не должно встречаться в именах переменных. Слово table никогда не должно встречаться в именах таблиц. Чем имя NameString лучше Name? Разве имя может быть, скажем, вещественным числом? Если может, то это нарушает предыдущее правило о дезинформации. Представьте, что в программе присутствуют два класса с именами Customer и CustomerObject. Что вы можете сказать о различиях между ними? Какой класс предоставляет лучший путь к истории платежей клиента?
Эта проблема встретилась нам в одном реально существующем приложении. Мы изменили имена, чтобы защитить виновных, но точная форма ошибки выглядит так:
getActiveAccount();
getActiveAccounts();
getActiveAccountInfo();
Как участвующему в проекте программисту понять, какую из этих функций вызывать в конкретном случае?
При отсутствии жестких именных схем имя moneyAmount не отличается от money, customerInfo не отличается от customer, accountData не отличается от account, а theMessage — от message. Записывайте различающиеся имена так, чтобы читатель кода понимал, какой смысл заложен в этих различиях.
class DtaRcrd102 {
private Date genymdhms;
private Date modymdhms;
private final String pszqint = "102";
/* ... */
};
и
class Customer {
private Date generationTimestamp;
private Date modificationTimestamp;;
private final String recordId = "102";
/* ... */
};
Теперь становится возможным осмысленный разговор: «Эй, Майк, глянь-ка на эту запись! В поле временного штампа заносится завтрашняя дата! Разве такое возможно?»
for (int j=0; j<34; j++) {
s += (t[j]*4)/5;
}
и
int realDaysPerIdealDay = 4;
const int WORK_DAYS_PER_WEEK = 5;
int sum = 0;
for (int j=0; j < NUMBER_OF_TASKS; j++) {
int realTaskDays = taskEstimate[j] * realDaysPerIdealDay;
int realTaskWeeks = (realdays / WORK_DAYS_PER_WEEK);
sum += realTaskWeeks;
}
Имя sum в этом фрагменте не слишком содержательно, но по крайней мере его удобно искать. Сознательное присваивание имен увеличивает длину функции, но подумайте, насколько проще найти WORK_DAYS_PER_WEEK, чем искать все вхождения цифры 5 и фильтровать список до позиций с нужным смыслом.
PhoneNumber phoneString;
// Имя не изменяется при изменении типа!
public class Part {
private String m_dsc; // Текстовое описание
void setName(String name) {
m_dsc = name;
}
}
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
public class Part {
String description;
void setDescription(String description) {
this.description = description;
}
}
Кроме того, люди быстро учатся игнорировать префиксы (и суффиксы), чтобы видеть содержательную часть имени. Чем больше мы читаем код, тем реже замечаем префиксы. В конечном итоге префикс превращается в невидимый балласт, характерный для старого кода.
string name = employee.getName();
customer.setName("mike");
if (paycheck.isPosted())...
При перегрузке конструкторов используйте статические методы-фабрики с именами, описывающими аргументы. Например, запись
Complex fulcrumPoint = Complex.FromRealNumber(23.0);
обычно лучше записи
Complex fulcrumPoint = new Complex(23.0);
Рассмотрите возможность принудительного использования таких методов; для этого соответствующие конструкторы объявляются приватными.
private void printGuessStatistics(char candidate, int count) {
String number;
String verb;
String pluralModifier;
if (count == 0) {
number = "no";
verb = "are";
pluralModifier = "s";
} else if (count == 1) {
number = "1";
verb = "is";
pluralModifier = "";
} else {
number = Integer.toString(count);
verb = "are";
pluralModifier = "s";
}
String guessMessage = String.format(
"There %s %s %s%s", verb, number, candidate, pluralModifier
);
print(guessMessage);
}
Функция длинновата, а переменные используются на всем ее протяжении. Чтобы разделить функцию на меньшие смысловые фрагменты, следует создать класс GuessStatisticsMessage и сделать три переменные полями этого класса. Тем самым мы предоставим очевидный контекст для трех переменных — теперь абсолютно очевидно, что эти переменные являются частью GuessStatisticsMessage. Уточнение контекста также позволяет заметно улучшить четкость алгоритма за счет его деления на меньшие функции (листинг 2.2).
public class GuessStatisticsMessage {
private String number;
private String verb;
private String pluralModifier;
public String make(char candidate, int count) {
createPluralDependentMessageParts(count);
return String.format(
"There %s %s %s%s",
verb, number, candidate, pluralModifier );
}
private void createPluralDependentMessageParts(int count) {
if (count == 0) {
thereAreNoLetters();
} else if (count == 1) {
thereIsOneLetter();
} else {
thereAreManyLetters(count);
}
}
private void thereAreManyLetters(int count) {
number = Integer.toString(count);
verb = "are";
pluralModifier = "s";
}
private void thereIsOneLetter() {
number = "1";
verb = "is";
pluralModifier = "";
}
private void thereAreNoLetters() {
number = "no";
verb = "are";
pluralModifier = "s";
}
}
public static String testableHtml(
PageData pageData,
boolean includeSuiteSetup
) throws Exception {
WikiPage wikiPage = pageData.getWikiPage();
StringBuffer buffer = new StringBuffer();
if (pageData.hasAttribute("Test")) {
if (includeSuiteSetup) {
WikiPage suiteSetup =
PageCrawlerImpl.getInheritedPage(
SuiteResponder.SUITE_SETUP_NAME, wikiPage
);
if (suiteSetup != null) {
WikiPagePath pagePath =
suiteSetup.getPageCrawler().getFullPath(suiteSetup);
String pagePathName = PathParser.render(pagePath);
buffer.append("!include -setup .")
.append(pagePathName)
.append("\n");
}
}
WikiPage setup =
PageCrawlerImpl.getInheritedPage("SetUp", wikiPage);
if (setup != null) {
WikiPagePath setupPath =
wikiPage.getPageCrawler().getFullPath(setup);
String setupPathName = PathParser.render(setupPath);"
buffer.append("!include -setup .")
.append(setupPathName)
.append("\n");
}
}
buffer.append(pageData.getContent());
if (pageData.hasAttribute("Test")) {
WikiPage teardown =
PageCrawlerImpl.getInheritedPage("TearDown", wikiPage);
if (teardown != null) {
WikiPagePath tearDownPath =
wikiPage.getPageCrawler().getFullPath(teardown);
String tearDownPathName = PathParser.render(tearDownPath);
buffer.append("\n")
.append("!include -teardown .")
.append(tearDownPathName)
.append("\n");
}
if (includeSuiteSetup) {
WikiPage suiteTeardown =
PageCrawlerImpl.getInheritedPage(
SuiteResponder.SUITE_TEARDOWN_NAME,
wikiPage
);
if (suiteTeardown != null) {
WikiPagePath pagePath =
suiteTeardown.getPageCrawler().getFullPath (suiteTeardown);
String pagePathName = PathParser.render(pagePath);
buffer.append("!include -teardown .")
.append(pagePathName)
.append("\n");
}
}
}
pageData.setContent(buffer.toString());
return pageData.getHtml();
}
Удалось ли вам разобраться с функцией за три минуты? Вероятно, нет. В ней происходит слишком много всего, и притом на разных уровнях абстракции. Загадочные строки и непонятные вызовы функций смешиваются в конструкциях if двойной вложенности, к тому же зависящих от состояния флагов.
Но после выделения нескольких методов, переименований и небольшой реструктуризации мне удалось представить смысл этой функции в девяти строках листинга 3.2. Посмотрим, удастся ли вам разобраться в ней за следующие три минуты.
public static String renderPageWithSetupsAndTeardowns(
PageData pageData, boolean isSuite
) throws Exception {
boolean isTestPage = pageData.hasAttribute("Test");
if (isTestPage) {
WikiPage testPage = pageData.getWikiPage();
StringBuffer newPageContent = new StringBuffer();
includeSetupPages(testPage, newPageContent, isSuite);
newPageContent.append(pageData.getContent());
includeTeardownPages(testPage, newPageContent, isSuite);
pageData.setContent(newPageContent.toString());
}
return pageData.getHtml();
}
Если только вы не занимаетесь активным изучением FitNesse, скорее всего, вы не разберетесь во всех подробностях. Но по крайней мере вы поймете, что функция включает в тестовую страницу какие-то начальные и конечные блоки, а потом генерирует код HTML. Если вы знакомы с JUnit[11], то, скорее всего, поймете, что эта функция является частью тестовой инфраструктуры на базе Web. И конечно, это правильное предположение. Прийти к такому выводу на основании листинга 3.2 несложно, но из листинга 3.1 это, мягко говоря, неочевидно.
Что же делает функцию из листинга 3.2 такой понятной и удобочитаемой? Как заставить функцию передавать намерения разработчика? Какие атрибуты функции помогут случайному читателю составить интуитивное представление о выполняемых ей задачах?
public static String renderPageWithSetupsAndTeardowns(
PageData pageData, boolean isSuite) throws Exception {
if (isTestPage(pageData))
includeSetupAndTeardownPages(pageData, isSuite);
return pageData.getHtml();
}
public Money calculatePay(Employee e)
throws InvalidEmployeeType {
switch (e.type) {
case COMMISSIONED:
return calculateCommissionedPay(e);
case HOURLY:
return calculateHourlyPay(e);
case SALARIED:
return calculateSalariedPay(e);
default:
throw new InvalidEmployeeType(e.type);
}
}
Эта функция имеет ряд недостатков. Во-первых, она велика, а при добавлении новых типов работников она будет разрастаться. Во-вторых, она совершенно очевидно выполняет более одной операции. В-третьих, она нарушает принцип единой ответственности[15], так как у нее существует несколько возможных причин изменения. В-четвертых, она нарушает принцип открытости/закрытости[16], потому что код функции должен изменяться при каждом добавлении новых типов. Но, пожалуй, самый серьезный недостаток заключается в том, что программа может содержать неограниченное количество других функций с аналогичной структурой, например:
isPayday(Employee e, Date date)
или
deliverPay(Employee e, Money pay)
и так далее. Все эти функции будут иметь все ту же ущербную структуру.
Решение проблемы (листинг 3.5) заключается в том, чтобы похоронить команду switch в фундаменте АБСТРАКТНОЙ ФАБРИКИ [GOF] и никому ее не показывать. Фабрика использует команду switch для создания соответствующих экземпляров потомков Employee, а вызовы функций calculatePay, isPayDay, deliverPay и т.д. проходят полиморфную передачу через интерфейс Employee.
public abstract class Employee {
public abstract boolean isPayday();
public abstract Money calculatePay();
public abstract void deliverPay(Money pay);
}
-----------------
public interface EmployeeFactory {
public Employee makeEmployee(EmployeeRecord r) throws InvalidEmployeeType;
}
-----------------
public class EmployeeFactoryImpl implements EmployeeFactory {
public Employee makeEmployee(EmployeeRecord r) throws InvalidEmployeeType {
switch (r.type) {
case COMMISSIONED:
return new CommissionedEmployee(r) ;
case HOURLY:
return new HourlyEmployee(r);
case SALARIED:
return new SalariedEmploye(r);
default:
throw new InvalidEmployeeType(r.type);
}
}
}
Мое общее правило в отношении команд switch гласит, что эти команды допустимы, если они встречаются в программе однократно, используются для создания полиморфных объектов и скрываются за отношениями наследования, чтобы оставаться невидимыми для остальных частей системы [G23]. Конечно, правил без исключений не бывает и в некоторых ситуациях приходится нарушать одно или несколько условий этого правила.
Circle makeCircle(double x, double y, double radius);
Circle makeCircle(Point center, double radius);
Сокращение количества аргументов посредством создания объектов может показаться жульничеством, но это не так. Если переменные передаются совместно как единое целое (как переменные x и y в этом примере), то, скорее всего, вместе они образуют концепцию, заслуживающую собственного имени.
String.format("%s worked %.2f hours.", name, hours);
Если все переменные аргументы считаются равноправными, как в этом примере, то их совокупность эквивалентна одному аргументу типа List. По этой причине функция String.format фактически является бинарной. И действительно, следующее объявление String.format подтверждает это:
public String format(String format, Object... args)
Следовательно, в данном случае действуют уже знакомые правила. Функции с переменным списком аргументов могут быть унарными, бинарными и даже тернарными, но использовать большее количество аргументов было бы ошибкой.
void monad(Integer... args);
void dyad(String name, Integer... args);
void triad(String name, int count, Integer... args);
public class UserValidator {
private Cryptographer cryptographer;
public boolean checkPassword(String userName, String password)
{
User user = UserGateway.findByName(userName);
if (user != User.NULL) {
String codedPhrase = user.getPhraseEncodedByPassword();
String phrase = cryptographer.decrypt(codedPhrase, password);
if ("Valid Password".equals(phrase)) {
Session.initialize();
return true;
}
}
return false;
}
}
Разумеется, побочным эффектом является вызов Session.initialize(). Имя checkPassword сообщает, что функция проверяет пароль. Оно ничего не говорит о том, что функция инициализирует сеанс. Таким образом, тот, кто поверит имени функции, рискует потерять текущие сеансовые данные, когда он решит проверить данные пользователя.
Побочный эффект создает временную привязку. А именно, функция checkPassword может вызываться только в определенные моменты времени (когда инициализация сеанса может быть выполнена безопасно). Несвоевременный вызов может привести к непреднамеренной потере сеансовых данных. Временные привязки создают массу проблем, особенно когда они прячутся в побочных эффектах. Если без временной привязки не обойтись, этот факт должен быть четко оговорен в имени функции. В нашем примере функцию можно было бы переименовать в checkPasswordAndInitializeSession, хотя это безусловно нарушает правило «одной операции».
appendFooter(s);
Присоединяет ли эта функция s в качестве завершающего блока к чему-то другому? Или она присоединяет какой-то завершающий блок к s? Является ли s входным или выходным аргументом? Конечно, можно посмотреть на сигнатуру функции и получить ответ:
public void appendFooter(StringBuffer report)
Вопрос снимается, но только после проверки объявления. Все, что заставляет обращаться к сигнатуре функции, нарушает естественный ритм чтения кода. Подобных «повторных заходов» следует избегать.
До наступления эпохи объектно-ориентированного программирования без выходных аргументов иногда действительно не удавалось обойтись. Но в ОО-языках эта проблема в целом исчезла, потому что сама функция может вызываться для выходного аргумента. Иначе говоря, функцию appendFooter лучше вызывать в виде
report.appendFooter();
В общем случае выходных аргументов следует избегать. Если ваша функция должна изменять чье-то состояние, пусть она изменяет состояние своего объекта-владельца.
public boolean set(String attribute, String value);
Функция присваивает значение атрибуту с указанным именем и возвращает true, если присваивание прошло успешно, или false, если такой атрибут не существует. Это приводит к появлению странных конструкций вида
if (set("username", "unclebob"))...
Представьте происходящее с точки зрения читателя кода. Что проверяет это условие? Что атрибут "username" содержит ранее присвоенное значение "unclebob"? Или что проверяет атрибуту "username" успешно присвоено значение "unclebob"? Смысл невозможно вывести из самого вызова, потому что мы не знаем, чем в данном случае является слово set — глаголом или прилагательным.
Автор предполагал, что set является глаголом, но в контексте команды if это имя скорее воспринимается как прилагательное. Таким образом, команда читается в виде «Если атрибуту username ранее было присвоено значение unclebob», а не «присвоить атрибуту username значение unclebob, и если все прошло успешно, то…» Можно было бы попытаться решить проблему, переименовав функцию set в setAndCheckIfExists, но это не особенно улучшает удобочитаемость команды if. Полноценное решение заключается в отделении команды от запроса, чтобы в принципе исключить любую неоднозначность.
if (attributeExists("username")) {
setAttribute("username", "unclebob");
...
}
if (deletePage(page) == E_OK)
Такие конструкции не страдают от смешения глаголов с прилагательными, но они приводят к созданию структур слишком глубокой вложенности. При возвращении кода ошибки возникает проблема: вызывающая сторона должна немедленно отреагировать на ошибку.
if (deletePage(page) == E_OK) {
if (registry.deleteReference(page.name) == E_OK) {
if (configKeys.deleteKey(page.name.makeKey()) == E_OK){
logger.log("page deleted");
} else {
logger.log("configKey not deleted");
}
} else
{
logger.log("deleteReference from registry failed");
}
} else {
logger.log("delete failed");
return E_ERROR;
}
С другой стороны, если вместо возвращения кодов ошибок используются исключения, то код обработки ошибок изолируется от ветви нормального выполнения и упрощается:
try {
deletePage(page);
registry.deleteReference(page.name);
configKeys.deleteKey(page.name.makeKey());
}
catch (Exception e) {
logger.log(e.getMessage());
}
public void delete(Page page) {
try {
deletePageAndAllReferences(page);
}
catch (Exception e) {
logError(e);
}
}
private void deletePageAndAllReferences(Page page) throws Exception {
deletePage(page);
registry.deleteReference(page.name);
configKeys.deleteKey(page.name.makeKey());
}
private void logError(Exception e) {
logger.log(e.getMessage());
}
В этом примере функция delete специализируется на обработке ошибок. В этой функции легко разобраться, а потом забыть о ней. Функция deletePageAndAllReferences специализируется на процессе полного удаления страницы. Читая ее, можно не обращать внимания на обработку ошибок. Таким образом, код нормального выполнения отделяется от кода обработки ошибок, а это упрощает его понимание и модификацию.
public enum Error {
OK,
INVALID,
NO_SUCH,
LOCKED,
OUT_OF_RESOURCES,
WAITING_FOR_EVENT;
}
Подобные классы называются магнитами зависимостей; они должны импортироваться и использоваться многими другими классами. При любых изменениях перечисления Error все эти классы приходится компилировать и развертывать заново[18]. Это обстоятельство создает негативную нагрузку на класс Error. Программистам не хочется добавлять новые ошибки, чтобы не создавать себе проблем со сборкой и развертыванием. Соответственно, вместо добавления новых кодов ошибок они предпочитают использовать старые.
Если вместо кодов ошибок использовать исключения, то новые исключения определяются производными от класса исключения. Их включение в программу не требует перекомпиляции или повторного развертывания[19].
package fitnesse.html;
import fitnesse.responders.run.SuiteResponder;
import fitnesse.wiki.*;
public class SetupTeardownIncluder {
private PageData pageData;
private boolean isSuite;
private WikiPage testPage;
private StringBuffer newPageContent;
private PageCrawler pageCrawler;
public static String render(PageData pageData) throws Exception {
return render(pageData, false);
}
public static String render(PageData pageData, boolean isSuite)
throws Exception {
return new SetupTeardownIncluder(pageData).render(isSuite);
}
private SetupTeardownIncluder(PageData pageData) {
this.pageData = pageData;
testPage = pageData.getWikiPage();
pageCrawler = testPage.getPageCrawler();
newPageContent = new StringBuffer();
}
private String render(boolean isSuite) throws Exception {
this.isSuite = isSuite;
if (isTestPage())
includeSetupAndTeardownPages();
return pageData.getHtml();
}
private boolean isTestPage() throws Exception {
return pageData.hasAttribute("Test");
}
private void includeSetupAndTeardownPages() throws Exception {
includeSetupPages();
includePageContent();
includeTeardownPages();
updatePageContent();
}
private void includeSetupPages() throws Exception {
if (isSuite)
includeSuiteSetupPage();
includeSetupPage();
}
private void includeSuiteSetupPage() throws Exception {
include(SuiteResponder.SUITE_SETUP_NAME, "-setup");
}
private void includeSetupPage() throws Exception {
include("SetUp", "-setup");
}
private void includePageContent() throws Exception {
newPageContent.append(pageData.getContent());
}
private void includeTeardownPages() throws Exception {
includeTeardownPage();
if (isSuite)
includeSuiteTeardownPage();
}
private void includeTeardownPage() throws Exception {
include("TearDown", "-teardown");
}
private void includeSuiteTeardownPage() throws Exception {
include(SuiteResponder.SUITE_TEARDOWN_NAME, "-teardown");
}
private void updatePageContent() throws Exception {
pageData.setContent(newPageContent.toString());
}
private void include(String pageName, String arg) throws Exception {
WikiPage inheritedPage = findInheritedPage(pageName);
if (inheritedPage != null) {
String pagePathName = getPathNameForPage(inheritedPage);
buildIncludeDirective(pagePathName, arg);
}
}
private WikiPage findInheritedPage(String pageName) throws Exception {
return PageCrawlerImpl.getInheritedPage(pageName, testPage);
}
private String getPathNameForPage(WikiPage page) throws Exception {
WikiPagePath pagePath = pageCrawler.getFullPath(page);
return PathParser.render(pagePath);
}
private void buildIncludeDirective(String pagePathName, String arg) {
newPageContent
.append("\n!include ")
.append(arg)
.append(" .")
.append(pagePathName)
.append("\n");
}
}
Не комментируйте плохой код — перепишите его.Ничто не помогает так, как уместный комментарий. Ничто не загромождает модуль так, как бессодержательные и безапелляционные комментарии. Ничто не приносит столько вреда, как старый, утративший актуальность комментарий, распространяющий ложь и дезинформацию. Комментарии — не список Шиндлера. Не стоит относиться к ним как к «абсолютному добру». На самом деле комментарии в лучшем случае являются неизбежным злом. Если бы языки программирования были достаточно выразительными или если бы мы умели искусно пользоваться этими языками для выражения своих намерений, то потребность в комментариях резко снизилась бы, а может, и вовсе сошла «на нет». Грамотное применение комментариев должно компенсировать нашу неудачу в выражении своих мыслей в коде. Обратите внимание на слово «неудачу». Я абсолютно серьезно. Комментарий — всегда признак неудачи. Мы вынуждены использовать комментарии, потому что нам не всегда удается выразить свои мысли без них, однако гордиться здесь нечем. Итак, вы оказались в ситуации, в которой необходимо написать комментарий? Хорошенько подумайте, нельзя ли пойти по другому пути и выразить свои намерения в коде. Каждый раз, когда вам удается это сделать, — похлопайте себя по плечу. Каждый раз, когда вы пишете комментарий, — поморщитесь и ощутите свою неудачу. Почему я так настроен против комментариев? Потому что они лгут. Не всегда и не преднамеренно, но это происходит слишком часто. Чем древнее комментарий, чем дальше он расположен от описываемого им кода, тем больше вероятность того, что он просто неверен. Причина проста: программисты не могут нормально сопровождать комментарии. Программный код изменяется и эволюционирует. Его фрагменты перемещаются из одного места в другое, раздваиваются, размножаются и сливаются. К сожалению, комментарии не всегда сопровождают их — и не всегда могут сопровождать их. Слишком часто комментарии отделяются от описываемого ими кода и превращаются в пометки непонятной принадлежности, с постоянно снижающейся точностью. Посмотрите, что произошло с этим комментарием и той строкой, которую он должен описывать:Брайан У. Керниган и П. Дж. Плауэр[21]
MockRequest request;
private final String HTTP_DATE_REGEXP =
"[SMTWF][a-z]{2}\\,\\s[0-9]{2}\\s[JFMASOND][a-z]{2}\\s"+
"[0-9]{4}\\s[0-9]{2}\\:[0-9]{2}\\:[0-9]{2}\\sGMT";
private Response response;
private FitNesseContext context;
private FileResponder responder;
private Locale saveLocale;
// Пример: "Tue, 02 Apr 2003 22:18:49 GMT"
Другие переменные экземпляра (вероятно, добавленные позднее) вклинились между константой HTTP_DATE_REGEXP и пояснительным комментарием.
На это можно возразить, что программисты должны быть достаточно дисциплинированными, чтобы поддерживать в своем коде актуальные, точные и релевантные комментарии. Согласен, должны. Но я бы предпочел, чтобы вместо этого программист постарался сделать свой код настолько четким и выразительным, чтобы комментарии были попросту не нужны.
Неточные комментарии гораздо вреднее, чем полное отсутствие комментариев. Они обманывают и сбивают с толку. Они создают у программиста невыполнимые ожидания. Они устанавливают устаревшие правила, которые не могут (или не должны) соблюдаться в будущем.
Истину можно найти только в одном месте: в коде. Только код может правдиво сообщить, что он делает. Это единственный источник действительно достоверной информации. Таким образом, хотя комментарии иногда необходимы, мы потратим немало усилий для того, чтобы свести их использование к минимуму.
// Проверить, положена ли работнику полная премия
if ((employee.flags & HOURLY_FLAG) &&
(employee.age > 65))
Или с таким:
if (employee.isEligibleForFullBenefits())
Чтобы объяснить большую часть ваших намерений в коде, достаточно нескольких секунд. Нередко задача сводится с созданию функции, которая сообщает то же, что и комментарий, который вы собираетесь написать.
// Copyright (C) 2003,2004,2005 by Object Mentor, Inc. All rights reserved.
// Публикуется на условиях лицензии GNU General Public License версии 2 и выше.
Такие комментарии не должны представлять собой комментарии или юридические трактаты. Вместо того чтобы перечислять в комментарии все условия, по возможности ограничьтесь ссылкой на стандартную лицензию или другой внешний документ.
// Возвращает тестируемый экземпляр Responder.
protected abstract Responder responderInstance();
Такие комментарии бывают полезными, но там, где это возможно, информацию лучше передавать в имени функции. Например, в данном примере вполне можно обойтись и без комментария — достаточно переименовать функцию в responderBeingTested.
А вот другой, более уместный пример:
// Поиск по формату: kk:mm:ss EEE, MMM dd, yyyy
Pattern timeMatcher = Pattern.compile(
"\\d*:\\d*:\\d* \\w*, \\w* \\d*, \\d*");
На этот раз комментарий сообщает, что регулярное выражение предназначено для идентификации времени и даты, отформатированных функцией SimpleDateFormat.format с заданной форматной строкой. И все же код стал бы лучше (и понятнее), если бы мы переместили этот код в специальный класс, преобразующий форматы даты и времени. Тогда комментарий, вероятно, стал бы излишним.
public int compareTo(Object o)
{
if(o instanceof WikiPagePath)
{
WikiPagePath p = (WikiPagePath) o;
String compressedName = StringUtil.join(names, "");
String compressedArgumentName = StringUtil.join(p.names, "");
return compressedName.compareTo(compressedArgumentName);
}
return 1; // Больше, потому что относится к правильному типу.
}
Или другой, еще лучший пример. Возможно, вы не согласитесь с тем, как программист решает проблему, но по крайней мере вы знаете, что он пытается сделать.
public void testConcurrentAddWidgets() throws Exception {
WidgetBuilder widgetBuilder =
new WidgetBuilder(new Class[]{BoldWidget.class});
String text = "'''bold text'''";
ParentWidget parent =
new BoldWidget(new MockWidgetRoot(), "'''bold text'''");
AtomicBoolean failFlag = new AtomicBoolean();
failFlag.set(false);
// Мы пытаемся спровоцировать "состояние гонки",
// создавая большое количество программных потоков.
for (int i = 0; i < 25000; i++) {
WidgetBuilderThread widgetBuilderThread =
new WidgetBuilderThread(widgetBuilder, text, parent, failFlag);
Thread thread = new Thread(widgetBuilderThread);
thread.start();
}
assertEquals(false, failFlag.get());
}
public void testCompareTo() throws Exception
{
WikiPagePath a = PathParser.parse("PageA");
WikiPagePath ab = PathParser.parse("PageA.PageB");
WikiPagePath b = PathParser.parse("PageB");
WikiPagePath aa = PathParser.parse("PageA.PageA");
WikiPagePath bb = PathParser.parse("PageB.PageB");
WikiPagePath ba = PathParser.parse("PageB.PageA");
assertTrue(a.compareTo(a) == 0); // a == a
assertTrue(a.compareTo(b) != 0); // a != b
assertTrue(ab.compareTo(ab) == 0); // ab == ab
assertTrue(a.compareTo(b) == -1); // a < b
assertTrue(aa.compareTo(ab) == -1); // aa < ab
assertTrue(ba.compareTo(bb) == -1); // ba < bb
assertTrue(b.compareTo(a) == 1); // b > a
assertTrue(ab.compareTo(aa) == 1); // ab > aa
assertTrue(bb.compareTo(ba) == 1); // bb > ba
}
Конечно, при этом возникает существенный риск, что пояснительный комментарий окажется неверным. Просмотрите код примера и убедитесь, как трудно проверить его правильность. Это объясняет как необходимость пояснений, так и связанный с ними риск. Итак, прежде чем писать такие комментарии, убедитесь в том, что лучшего способа не существует, и еще внимательнее следите за их правильностью.
// Не запускайте, если только не располагаете
// излишками свободного времени.
public void _testWithReallyBigFile()
{
writeLinesToFile(10000000);
response.setBody(testFile);
response.readyToSend(this);
String responseString = output.toString();
assertSubString("Content-Length: 1000000000", responseString);
assertTrue(bytesSent > 1000000000);
}
Конечно, в наше время тестовый сценарий следовало бы отключить при помощи атрибута @Ignore с соответствующей пояснительной строкой: @Ignore("Слишком долго выполняется"). Но до появления JUnit 4 запись с начальным символом подчеркивания перед именем метода считалась стандартной. Комментарий, при всей его несерьезности, хорошо доносит свое сообщение до читателя.
А вот другой, более выразительный пример:
public static SimpleDateFormat makeStandardHttpDateFormat()
{
// Класс SimpleDateFormat не является потоково-безопасным,
// поэтому экземпляры должны создаваться независимо друг от друга.
SimpleDateFormat df = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss z");
df.setTimeZone(TimeZone.getTimeZone("GMT"));
return df;
}
Возможно, вы возразите, что у задачи есть и более удачные решения. Пожалуй, я соглашусь с вами. Однако комментарий в том виде, в котором он здесь приведен, выглядит абсолютно разумно. По крайней мере он помешает излишне ретивому программисту использовать статический инициализатор по соображениям эффективности.
// TODO - На данный момент эта функция не используется.
// Ситуация изменится при переходе к отладочной модели.
protected VersionInfo makeVersion() throws Exception
{
return null;
}
Комментарии TODO напоминают о том, что, по мнению программиста, сделать необходимо, но по какой-то причине нельзя сделать прямо сейчас. Например, комментарий может напомнить о необходимости удаления устаревшей функции или предложить кому-то другому поучаствовать в решении проблемы — скажем, придумать более удачное имя или внести изменения, зависящие от запланированного события. Впрочем, чем бы ни был комментарий TODO, это не повод оставлять плохой код в системе.
В наши дни в любой хорошей рабочей среде имеется функция поиска всех комментариев TODO, так что потеря таких комментариев маловероятна. И все же код не должен загромождаться лишними комментариями TODO. Регулярно просматривайте их и удаляйте те, которые потеряли актуальность.
String listItemContent = match.group(3).trim();
// Вызов trim() очень важен. Он удаляет начальные пробелы,
// чтобы строка успешно интерпретировалась как список.
new ListItemWidget(this, listItemContent, this.level + 1);
return buildList(text.substring(match.end()));
public void loadProperties()
{
try
{
String propertiesPath = propertiesLocation + "/" + PROPERTIES_FILE;
FileInputStream propertiesStream = new FileInputStream(propertiesPath);
loadedProperties.load(propertiesStream);
}
catch(IOException e)
{
// Если нет файла свойств, загружаются настройки по умолчанию
}
}
Что означает комментарий в блоке catch? Очевидно, он что-то означал для автора, но для читателя этот смысл не доходит. Видимо, если мы получаем IOException, это означает, что файл свойств отсутствует; в этом случае должны загружаться все настройки по умолчанию. Но кто загружает эти настройки? Были ли они загружены перед вызовом loadProperties.load? Или вызов loadProperties.load перехватывает исключение, загружает настройки по умолчанию, а затем передает исключение нам, чтобы мы могли его проигнорировать? Или loadProperties.load загружает настройки по умолчанию до того, как вы попытались загрузить файл? Автор пытался успокоить себя относительно того факта, что он оставил блок catch пустым? Или — и это самая пугающая возможность — автор хотел напомнить себе, что позднее нужно вернуться и написать код загрузки настроек по умолчанию?
Чтобы разобраться в происходящем, нам остается только изучить код других частей системы. Любой комментарий, смысл которого приходится искать в других модулях, не несет полезной информации и не стоит битов, затраченных на его написание.
// Вспомогательный метод; возвращает управление, когда значение this.closed истинно.
// Инициирует исключение при достижении тайм-аута.
public synchronized void waitForClose(final long timeoutMillis)
throws Exception
{
if(!closed)
{
wait(timeoutMillis);
if(!closed)
throw new Exception("MockResponseSender could not be closed");
}
}
Какой цели достигает этот комментарий? Конечно, он несет не больше информации, чем программный код. Он не объясняет код, не предоставляет обоснований и не раскрывает намерений. Он читается не проще, чем сам код. Более того, комментарий уступает коду в точности и навязывает читателю эту неточность взамен истинного понимания. Он напоминает жуликоватого торговца подержанными машинами, уверяющего, что вам незачем заглядывать под капот.
А теперь рассмотрим легион бесполезных, избыточных комментариев Javadoc из листинга 4.2, позаимствованных из Tomcat. Эти комментарии только загромождают код и скрывают его смысл. Никакой пользы для документирования от них нет. Что еще хуже, я привел только несколько начальных комментариев — в этом модуле их намного больше.
public abstract class ContainerBase
implements Container, Lifecycle, Pipeline,
MBeanRegistration, Serializable {
/**
* Задержка процессора для этого компонента.
*/
protected int backgroundProcessorDelay = -1;
/**
* Поддержка событий жизненного цикла для этого компонента.
*/
protected LifecycleSupport lifecycle =
new LifecycleSupport(this);
/**
* Слушатели контейнерных событий для этого контейнера.
*/
protected ArrayList listeners = new ArrayList();
/**
* Реализация загрузчика, связанная с контейнером.
*/
protected Loader loader = null;
/**
* Реализация журнального компонента, связанная с контейнером.
*/
protected Log logger = null;
/**
* Имя журнального компонента.
*/
protected String logName = null;
/**
* Реализация менеджера, связанная с контейнером.
*/
protected Manager manager = null;
/**
* Кластер, связанный с контейнером.
*/
protected Cluster cluster = null;
/**
* Удобочитаемое имя контейнера.
*/
protected String name = null;
/**
* Родительский контейнер, по отношению к которому
* данный контейнер является дочерним.
*/
protected Container parent = null;
/**
* Загрузчик родительского класса, задаваемый при назначении загрузчика.
*/
protected ClassLoader parentClassLoader = null;
/**
* Объект Pipeline, связанный с данным контейнером.
*/
protected Pipeline pipeline = new StandardPipeline(this);
/**
* Объект Realm, связанный с контейнером.
*/
protected Realm realm = null;
/**
* Объект ресурсов DirContext, связанный с контейнером
*/
protected DirContext resources = null;
/**
*
* @param title Название диска
* @param author Автор диска
* @param tracks Количество дорожек на диске
* @param durationInMinutes Продолжительность воспроизведения в минутах
*/
public void addCD(String title, String author,
int tracks, int durationInMinutes) {
CD cd = new CD();
cd.title = title;
cd.author = author;
cd.tracks = tracks;
cd.duration = duration;
cdList.add(cd);
}
* Изменения (начиная с 11 октября 2001)
* --------------------------
* 11.10.2001 : Реорганизация класса и его перемещение в новый пакет
* com.jrefinery.date (DG);
* 05.11.2001 : Добавление метода getDescription(), устранение класса
* NotableDate (DG);
* 12.11.2001 : С устранением класса NotableDate IBD требует включения
* метода setDescription() (DG); исправление ошибок
* в методах getPreviousDayOfWeek(), getFollowingDayOfWeek()
* и getNearestDayOfWeek() (DG);
* 05.12.2001 : Исправление ошибки в классе SpreadsheetDate (DG);
* 29.05.2002 : Перемещение констант месяцев в отдельный интерфейс
* (MonthConstants) (DG);
* 27.08.2002 : Исправление ошибки в методе addMonths() с подачи N???levka Petr (DG);
* 03.10.2002 : Исправление ошибок по сообщениям Checkstyle (DG);
* 13.03.2003 : Реализация Serializable (DG);
* 29.05.2003 : Исправление ошибки в методе addMonths (DG);
* 04.09.2003 : Реализация Comparable. Обновление isInRange javadocs (DG);
* 05.01.2005 : Исправление ошибки в методе addYears() (1096282) (DG);
Когда-то создание и сопровождение журнальных записей в начале каждого модуля было оправдано. У нас еще не было систем управления исходным кодом, которые делали это за нас. В наши дни длинные журналы только загромождают и усложняют код. Их следует полностью удалить из ваших программ.
/**
* Конструктор по умолчанию.
*/
protected AnnualDateRule() {
}
Да неужели? А как насчет этого:
/** День месяца. */
private int dayOfMonth;
И наконец, апофеоз избыточности:
/**
* Возвращает день месяца.
*
* @return день месяца.
*/
public int getDayOfMonth() {
return dayOfMonth;
}
Эти комментарии настолько бесполезны, что мы учимся не обращать на них внимания. В процессе чтения кода наш взгляд просто скользит мимо них. Рано или поздно код вокруг таких комментариев изменяется, и они начинают лгать.
Первый комментарий в листинге 4.4 кажется уместным. Он объясняет, почему блок catch игнорируется. Но второй комментарий не несет полезной информации. Видимо, программист настолько вышел из себя при написании этих блоков try/catch в этой функции, что ему понадобилось «выпустить пар».
private void startSending()
{
try
{
doSending();
}
catch(SocketException e)
{
// Нормально. Кто-то прервал запрос.
}
catch(Exception e)
{
try
{
response.add(ErrorResponder.makeExceptionString(e));
response.closeAll();
}
catch(Exception e1)
{
// Ну хватит уже!
}
}
}
Вместо того чтобы давать выход чувствам в бесполезном комментарии, программисту следовало понять, что раздражение можно было снять улучшением структуры кода. Ему стоило направить свою энергию на выделение последнего блока try/catch в отдельную функцию, как показано в листинге 4.5.
private void startSending()
{
try
{
doSending();
}
catch(SocketException e)
{
// Нормально. Кто-то прервал запрос.
}
catch(Exception e)
{
addExceptionAndCloseResponse(e);
}
}
private void addExceptionAndCloseResponse(Exception e)
{
try
{
response.add(ErrorResponder.makeExceptionString(e));
response.closeAll();
}
catch(Exception e1)
{
}
}
Искушение создать очередной «шумовой комментарий» следует заменить решимостью очистить код. Вы сами увидите, что это сделает вашу работу более приятной и эффективной.
/** Имя. */
private String name;
/** Версия. */
private String version;
/** Название лицензии. */
private String licenceName;
/** Версия. */
private String info;
Прочитайте эти комментарии повнимательнее. Заметили ошибку копирования/вставки? Если авторы не следят за ними в момент написания (или вставки), то как можно ожидать, что эти комментарии принесут пользу читателю?
// Зависит ли модуль из глобального списка <mod> от подсистемы,
// частью которой является наш код?
if (smodule.getDependSubsystems().contains(subSysMod.getSubSystem()))
Его можно было бы перефразировать без комментария в следующем виде:
ArrayList moduleDependees = smodule.getDependSubsystems();
String ourSubSystem = subSysMod.getSubSystem();
if (moduleDependees.contains(ourSubSystem))
Возможно (хотя и маловероятно), автор исходного кода сначала написал комментарий, а затем — соответствующий ему код. Но после этого автор должен был переработать свой код, как это сделал я, чтобы комментарий можно было удалить.
// Действия //////////////////////////////////
В отдельных случаях объединение функций под такими заголовками имеет смысл. Но в общем случае они составляют балласт, от которого следует избавиться — особенно от назойливой серии косых черт в конце.
Взгляните на дело под таким углом: заголовки привлекают внимание только в том случае, если они встречаются не слишком часто. Используйте их умеренно и только тогда, когда они приносят ощутимую пользу. При слишком частом употреблении заголовков читатель воспринимает их как фоновый шум и перестает обращать на них внимание.
public class wc {
public static void main(String[] args) {
BufferedReader in = new BufferedReader(new InputStreamReader(System.in));
String line;
int lineCount = 0;
int charCount = 0;
int wordCount = 0;
try {
while ((line = in.readLine()) != null) {
lineCount++;
charCount += line.length();
String words[] = line.split("\\W");
wordCount += words.length;
} //while
System.out.println("wordCount = " + wordCount);
System.out.println("lineCount = " + lineCount);
System.out.println("charCount = " + charCount);
} // try
catch (IOException e) {
System.err.println("Error:" + e.getMessage());
} //catch
} //main
}
/* Добавлено Риком */
Системы контроля исходного кода отлично запоминают, кто и когда внес то или иное исправление. Нет необходимости загрязнять код подобными ссылками. Может показаться, что такие комментарии помогают другим определить, с кем следует обсуждать данный фрагмент кода. Однако в действительности эти комментарии остаются в коде на долгие годы и со временем становятся все менее точными и актуальными.
И снова лучшим источником подобной информации является система контроля исходного кода.
InputStreamResponse response = new InputStreamResponse();
response.setBody(formatter.getResultStream(), formatter.getByteCount());
// InputStream resultsStream = formatter.getResultStream();
// StreamReader reader = new StreamReader(resultsStream);
// response.setContent(reader.read(formatter.getByteCount()));
У других программистов, видящих закомментированный код, не хватает храбрости удалить его. Они полагают, что код оставлен не зря и слишком важен для удаления. В итоге закомментированный код скапливается, словно осадок на дне бутылки плохого вина.
Следующий код взят из общих модулей Apache:
this.bytePos = writeBytes(pngIdBytes, 0);
//hdrPos = bytePos;
writeHeader();
writeResolution();
//dataPos = bytePos;
if (writeImageData()) {
writeEnd();
this.pngBytes = resizeByteArray(this.pngBytes, this.maxPos);
}
else {
this.pngBytes = null;
}
return this.pngBytes;
Почему эти две строки кода закомментированы? Они важны? Их оставили как напоминание о будущих изменениях? Или это «хлам», который кто-то закомментировал сто лет назад и не удосужился убрать из программы?
В 60-е годы закомментированный код мог быть действительно полезен. Но с тех пор у нас давно появились хорошие системы контроля исходного кода. Эти системы запоминают изменения в коде за нас. Нам уже не нужно закрывать их комментариями. Просто удалите ненужный код. Он никуда не исчезнет. Честное слово.
/**
* Задача для запуска тестов.
* Задача запускает тесты fitnesse и публикует результаты.
* <p/>
* <pre>
* Usage:
* <taskdef name="execute-fitnesse-tests"
* classname="fitnesse.ant.ExecuteFitnesseTestsTask"
* classpathref="classpath" />
* OR
* <taskdef classpathref="classpath"
* resource="tasks.properties" />
* <p/>
* <execute-fitnesse-tests
* suitepage="FitNesse.SuiteAcceptanceTests"
* fitnesseport="8082"
* resultsdir="${results.dir}"
* resultshtmlpage="fit-results.html"
* classpathref="classpath" />
* </pre>
*/
/**
* Порт, на котором будет работать fitnesse. По умолчанию <b>8082</b>.
*
* @param fitnessePort
*/
public void setFitnessePort(int fitnessePort)
{
this.fitnessePort = fitnessePort;
}
/*
RFC 2045 - Multipurpose Internet Mail Extensions (MIME)
Часть 1: Формат тел сообщений
раздел 6.8. Кодирование данных Base64
В процессе кодирования 24-разрядные группы входных битов представляются
в виде выходных строк из 4 закодированных символов. Слева направо 24-разрядная
входная группа образуется посредством конкатенации 38-разрядных входных групп.
Далее эти 24 бита интерпретируются как 4 конкатенированных 6-разрядных группы,
каждая из которых преобразуется в одну цифру алфавита base64. При кодировании
потока битов в кодировке base64 предполагается, что битовый поток упорядочивается
от старшего значащего бита. Иначе говоря, первым битом потока будет старший бит
первого 8-битового байта, а восьмым - младший бит первого 8-битого байта и т.д.
*/
/*
* Начать с массива, размер которого достаточен для хранения
* всех пикселов (плюс байты фильтра), плюс еще 200 байт
* для данных заголовка
*/
this.pngBytes = new byte[((this.width + 1) * this.height * 3) + 200];
Что такое «байты фильтра»? Они как-то связаны с +1? Или с *3? И с тем и с другим? Один пиксел соответствует одному байту? И почему 200? Цель комментария — объяснить код, который не объясняет сам себя. Плохо, когда сам комментарий нуждается в объяснениях.
/**
* Класс генерирует простые числа в диапазоне до максимального значения,
* заданного пользователем, по алгоритму "Решета Эратосфена".
* <p>
* Эратосфен Киренский, 276 год до н.э., Ливия -- * 194 год до н.э., Александрия.
* Первый ученый, вычисливший длину земного меридиана. Известен своими работами
* о календарях с високосным годом, заведовал Александрийской библиотекой.
* <p>
* Алгоритм весьма прост. Берем массив целых чисел, начиная с 2, и вычеркиваем
* из него все числа, кратные 2. Находим следующее невычеркнутое число
* и вычеркиваем все его кратные. Повторяем до тех пор, пока не дойдем
* до квадратного корня верхней границы диапазона.
*
* @author Альфонс
* @version 13 февраля 2002 u
*/
import java.util.*;
public class GeneratePrimes
{
/**
* @param maxValue - верхняя граница диапазона.
*/
public static int[] generatePrimes(int maxValue)
{
if (maxValue >= 2) // Единственно допустимый случай
{
// Объявления
int s = maxValue + 1; // Размер массива
boolean[] f = new boolean[s];
int i;
// Инициализировать массив значениями true.
for (i = 0; i < s; i++)
f[i] = true;
// Удалить числа, заведомо не являющиеся простыми.
f[0] = f[1] = false;
// Отсев
int j;
for (i = 2; i < Math.sqrt(s) + 1; i++)
{
if (f[i]) // Если элемент i не вычеркнут, вычеркнуть кратные ему.
{
for (j = 2 * i; j < s; j += i)
f[j] = false; // Кратные числа не являются простыми.
}
}
// Сколько простых чисел осталось?
int count = 0;
for (i = 0; i < s; i++)
{
if (f[i])
count++; // Приращение счетчика
}
int[] primes = new int[count];
// Переместить простые числа в результат
for (i = 0, j = 0; i < s; i++)
{
if (f[i]) // Если простое
primes[j++] = i;
}
return primes; // Вернуть простые числа
}
else // maxValue < 2
return new int[0]; // Вернуть пустой массив при недопустимых входных данных.
}
}
В листинге 4.8 приведена переработанная версия того же модуля. Обратите внимание: применение комментариев стало намного более ограниченным. Во всем модуле осталось всего два комментария пояснительного характера.
/**
* Класс генерирует простые числа до максимального значения, заданного
* пользователем, по алгоритму "Решета Эратосфена".
* Берем массив целых чисел, начиная с 2, и вычеркиваем
* из него все числа, кратные 2. Находим следующее невычеркнутое число
* и вычеркиваем все числа, кратные ему. Повторяем до тех пор, пока из массива
* не будут вычеркнуты все кратные.
*/
public class PrimeGenerator
{
private static boolean[] crossedOut;
private static int[] result;
public static int[] generatePrimes(int maxValue)
{
if (maxValue < 2)
return new int[0];
else
{
uncrossIntegersUpTo(maxValue);
crossOutMultiples();
putUncrossedIntegersIntoResult();
return result;
}
}
private static void uncrossIntegersUpTo(int maxValue)
{
crossedOut = new boolean[maxValue + 1];
for (int i = 2; i < crossedOut.length; i++)
crossedOut[i] = false;
}
private static void crossOutMultiples()
{
int limit = determineIterationLimit();
for (int i = 2; i <= limit; i++)
if (notCrossed(i))
crossOutMultiplesOf(i);
}
private static int determineIterationLimit()
{
// Каждое кратное в массиве имеет простой множитель, больший либо равный
// квадратному корню из размера массива. Следовательно, вычеркивать элементы,
// кратные числам, превышающих квадратный корень, не нужно.
double iterationLimit = Math.sqrt(crossedOut.length);
return (int) iterationLimit;
}
private static void crossOutMultiplesOf(int i)
{
for (int multiple = 2*i;
multiple < crossedOut.length;
multiple += i)
crossedOut[multiple] = true;
}
private static boolean notCrossed(int i)
{
return crossedOut[i] == false;
}
private static void putUncrossedIntegersIntoResult()
{
result = new int[numberOfUncrossedIntegers()];
for (int j = 0, i = 2; i < crossedOut.length; i++)
if (notCrossed(i))
result[j++] = i;
}
private static int numberOfUncrossedIntegers()
{
int count = 0;
for (int i = 2; i < crossedOut.length; i++)
if (notCrossed(i))
count++;
return count;
}
}
Можно возразить, что первый комментарий избыточен, потому что он практически полностью повторяет код самой функции generatePrimes. И все же я считаю, что этот комментарий упрощает понимание алгоритма пользователем, поэтому я склонен оставить его.
Второй комментарий почти стопроцентно необходим. Он объясняет смысл использования квадратного корня как верхней границы цикла. Мне не удалось найти ни простого имени переменной, ни другой структуры кода, которые бы наглядно передавали это обстоятельство. С другой стороны, само использование квадратного корня может быть иллюзией. Действительно ли ограничение цикла квадратным корнем способно сэкономить время? Не уйдет ли на его вычисление больше времени, чем я экономлю? Об этом стоит подумать. Использование квадратного корня в качестве верхней границы цикла тешит мои наклонности старого хакера, работавшего на C и ассемблере, но я не уверен, что оно оправдает время и усилия, необходимые читателям кода для его понимания.
package fitnesse.wikitext.widgets;
import java.util.regex.*;
public class BoldWidget extends ParentWidget {
public static final String REGEXP = "'''.+?'''";
private static final Pattern pattern = Pattern.compile("'''(.+?)'''",
Pattern.MULTILINE + Pattern.DOTALL
};
public BoldWidget(ParentWidget parent, String text) throws Exception {
super(parent);
Matcher match = pattern.matcher(text);
match.find();
addChildWidgets(match.group(1));
}
public String render() throws Exception {
StringBuffer html = new StringBuffer("<b>");
html.append(childHtml()).append("</b>");
return html.toString();
}
}
Удаление пустых строк, как в листинге 5.2, имеет весьма тяжелые последствия для удобочитаемости кода.
package fitnesse.wikitext.widgets;
import java.util.regex.*;
public class BoldWidget extends ParentWidget {
public static final String REGEXP = "'''.+?'''";
private static final Pattern pattern = Pattern.compile("'''(.+?)'''",
Pattern.MULTILINE + Pattern.DOTALL);
public BoldWidget(ParentWidget parent, String text) throws Exception {
super(parent);
Matcher match = pattern.matcher(text);
match.find();
addChildWidgets(match.group(1));}
public String render() throws Exception {
StringBuffer html = new StringBuffer("<b>");
html.append(childHtml()).append("</b>");
return html.toString();
}
}
Эффект становится еще более заметным, если на секунду отвести глаза от листинга. В первом примере группировка строк сразу бросается в глаза, а второй пример выглядит как сплошная каша, притом что два листинга различаются только вертикальными разделителями.
public class ReporterConfig {
/**
* Имя класса слушателя
*/
private String m_className;
/**
* Свойства слушателя
*/
private List<Property> m_properties = new ArrayList<Property>();
public void addProperty(Property property) {
m_properties.add(property);
}
Листинг 5.4 читается гораздо проще. Он нормально воспринимается «с одного взгляда» — по крайней мере, для меня. Я смотрю на него и сразу вижу, что передо мной класс с двумя переменными и одним методом; для этого мне не приходится вертеть головой или бегать по строчкам глазами. В предыдущем листинге для достижения того же уровня понимания приходится потрудиться намного больше.
public class ReporterConfig {
private String m_className;
private List<Property> m_properties = new ArrayList<Property>();
public void addProperty(Property property) {
m_properties.add(property);
}
private static void readPreferences() {
InputStream is= null;
try {
is= new FileInputStream(getPreferencesFile());
setPreferences(new Properties(getPreferences()));
getPreferences().load(is);
} catch (IOException e) {
try {
if (is != null)
is.close();
} catch (IOException e1) {
}
}
}
Управляющие переменные циклов обычно объявляются внутри конструкции цикла, как в следующей симпатичной маленькой функции из того же источника.
public int countTestCases() {
int count= 0;
for (Test each : tests)
count += each.countTestCases();
return count;
}
В отдельных случаях переменная может объявляться в начале блока или непосредственно перед циклом в длинной функции. Пример такого объявления представлен в следующем фрагменте очень длинной функции из TestNG.
...
for (XmlTest test : m_suite.getTests()) {
TestRunner tr = m_runnerFactory.newTestRunner(this, test);
tr.addListener(m_textReporter);
m_testRunners.add(tr);
invoker = tr.getInvoker();
for (ITestNGMethod m : tr.getBeforeSuiteMethods()) {
beforeSuiteMethods.put(m.getMethod(), m);
}
for (ITestNGMethod m : tr.getAfterSuiteMethods()) {
afterSuiteMethods.put(m.getMethod(), m);
}
}
...
Переменные экземпляров, напротив, должны объявляться в начале класса. Это не увеличивает вертикальное расстояние между переменными, потому что в хорошо спроектированном классе они используются многими, если не всеми, методами класса.
Размещение переменных экземпляров становилось причиной ожесточенных споров. В C++ обычно применялось так называемое правило ножниц, при котором все переменные экземпляров размещаются внизу. С другой стороны, в Java они обычно размещаются в начале класса. Я не вижу причин для использования каких-либо других конвенций. Здесь важно то, что переменные экземпляров должны объявляться в одном хорошо известном месте. Все должны знать, где следует искать объявления.
Для примера рассмотрим странный класс TestSuite из JUnit 4.3.1. Я основательно сократил этот класс, чтобы лучше выразить свою мысль. Где-то в середине листинга вдруг обнаруживаются объявления двух переменных экземпляров. Если бы автор сознательно хотел спрятать их, трудно найти более подходящее место. Читатель кода может наткнуться на эти объявления только случайно (как я).
public class TestSuite implements Test {
static public Test createTest(Class<? extends TestCase> theClass,
String name) {
...
}
public static Constructor<? extends TestCase>
getTestConstructor(Class<? extends TestCase> theClass)
throws NoSuchMethodException {
...
}
public static Test warning(final String message) {
...
}
private static String exceptionToString(Throwable t) {
...
}
private String fName;
private Vector<Test> fTests= new Vector<Test>(10);
public TestSuite() {
}
public TestSuite(final Class<? extends TestCase> theClass) {
...
}
public TestSuite(Class<? extends TestCase> theClass, String name) {
...
}
... ... ... ... ...
}
Зависимые функции. Если одна функция вызывает другую, то эти функции должны располагаться вблизи друг от друга по вертикали, а вызывающая функция должна находиться над вызываемой (если это возможно). Тем самым формируется естественная структура программного кода. Если это правило будет последовательно соблюдаться, читатели кода будут уверены в том, что определения функций следуют неподалеку от их вызовов. Для примера возьмем фрагмент FitNesse из листинга 5.5. Обратите внимание на то, как верхняя функция вызывает нижние, и как они, в свою очередь, вызывают функции более низкого уровня. Такая структура позволяет легко найти вызываемые функции и значительно улучшает удобочитаемость всего модуля.
public class WikiPageResponder implements SecureResponder {
protected WikiPage page;
protected PageData pageData;
protected String pageTitle;
protected Request request;
protected PageCrawler crawler;
public Response makeResponse(FitNesseContext context, Request request)
throws Exception {
String pageName = getPageNameOrDefault(request, "FrontPage");
loadPage(pageName, context);
if (page == null)
return notFoundResponse(context, request);
else
return makePageResponse(context);
}
private String getPageNameOrDefault(Request request, String defaultPageName)
{
String pageName = request.getResource();
if (StringUtil.isBlank(pageName))
pageName = defaultPageName;
return pageName;
}
protected void loadPage(String resource, FitNesseContext context)
throws Exception {
WikiPagePath path = PathParser.parse(resource);
crawler = context.root.getPageCrawler();
crawler.setDeadEndStrategy(new VirtualEnabledPageCrawler());
page = crawler.getPage(context.root, path);
if (page != null)
pageData = page.getData();
}
private Response notFoundResponse(FitNesseContext context, Request request)
throws Exception {
return new NotFoundResponder().makeResponse(context, request);
}
private SimpleResponse makePageResponse(FitNesseContext context)
throws Exception {
pageTitle = PathParser.render(crawler.getFullPath(page));
String html = makeHtml(context);
SimpleResponse response = new SimpleResponse();
response.setMaxAge(0);
response.setContent(html);
return response;
}
...
Заодно этот фрагмент дает хороший пример хранения констант на соответствующем уровне [G35]. Константу «FrontPage» можно было бы объявить в функции getPageNameOrDefault, но тогда хорошо известная и ожидаемая константа оказалась бы погребенной в функции неуместно низкого уровня. Лучше переместить эту константу вниз – от того места, где ее следовало бы ввести, к месту ее фактического использования.
Концептуальное родство. Некоторые фрагменты кода требуют, чтобы их разместили вблизи от других фрагментов. Такие фрагменты обладают определенным концептуальным родством. Чем сильнее родство, тем меньше должно быть вертикальное расстояние между ними.
public class Assert {
static public void assertTrue(String message, boolean condition) {
if (!condition)
fail(message);
}
static public void assertTrue(boolean condition) {
assertTrue(null, condition);
}
static public void assertFalse(String message, boolean condition) {
assertTrue(message, !condition);
}
static public void assertFalse(boolean condition) {
assertFalse(null, condition);
}
...
Эти функции обладают сильным концептуальным родством, потому что они используют единую схему выбора имен и выполняют разные варианты одной базовой операции. Тот факт, что они вызывают друг друга, вторичен. Даже без него эти функции все равно следовало бы разместить поблизости друг от друга.
private void measureLine(String line) {
lineCount++;
int lineSize = line.length();
totalChars += lineSize;
lineWidthHistogram.addLine(lineSize, lineCount);
recordWidestLine(lineSize);
}
Знаки присваивания окружены пробелами, обеспечивающими их визуальное выделение. Операторы присваивания состоят из двух основных элементов: левой и правой частей. Пробелы наглядно подчеркивают это разделение.
С другой стороны, я не стал отделять имена функций от открывающих скобок. Это обусловлено тем, что имя функции тесно связано с ее аргументами. Пробелы изолируют их вместо того, чтобы объединять. Я также разделил аргументы в скобках пробелами, чтобы выделить запятые и подчеркнуть, что аргументы не зависят друг от друга.
Пробелы также применяются для визуального обозначения приоритета операторов:
public class Quadratic {
public static double root1(double a, double b, double c) {
double determinant = determinant(a, b, c);
return (-b + Math.sqrt(determinant)) / (2*a);
}
public static double root2(int a, int b, int c) {
double determinant = determinant(a, b, c);
return (-b - Math.sqrt(determinant)) / (2*a);
}
private static double determinant(double a, double b, double c) {
return b*b - 4*a*c;
}
}
Обратите внимание, как хорошо читаются формулы. Между множителями нет пробелов, потому что они обладают высоким приоритетом. Слагаемые разделяются пробелами, так как сложение и вычитание имеют более низкий приоритет.
К сожалению, в большинстве программ форматирования кода приоритет операторов не учитывается, и во всех случаях применяются одинаковые пропуски. Нетривиальные изменения расстояний, как в приведенном примере, теряются после переформатирования кода.
public class FitNesseExpediter implements ResponseSender
{
private Socket socket;
private InputStream input;
private OutputStream output;
private Request request;
private Response response;
private FitNesseContext context;
protected long requestParsingTimeLimit;
private long requestProgress;
private long requestParsingDeadline;
private boolean hasError;
public FitNesseExpediter(Socket s,
FitNesseContext context) throws Exception
{
this.context = context;
socket = s;
input = s.getInputStream();
output = s.getOutputStream();
requestParsingTimeLimit = 10000;
}
Однако потом я обнаружил, что такое выравнивание не приносит пользы. Оно визуально выделяет совсем не то, что требуется, и отвлекает читателя от моих истинных намерений. Например, в приведенном выше списке объявлений читатель просматривает имена переменных, не обращая внимания на их типы. Аналогичным образом в списке команд присваивания возникает соблазн просмотреть правосторонние значения, не замечая оператора присваивания. Ситуация усугубляется тем, что средства автоматического форматирования обычно удаляют подобное выравнивание.
Поэтому в итоге я отказался от этого стиля форматирования. Сейчас я отдаю предпочтение невыровненным объявлениям и присваиваниям, как в следующем фрагменте, потому что они помогают выявить один важный дефект. Если в программе встречаются длинные списки, нуждающиеся в выравнивании, то проблема кроется в длине списка, а не в отсутствии выравнивания. Длина списков объявлений в классе FitNesseExpediter наводит на мысль, что этот класс необходимо разделить.
public class FitNesseExpediter implements ResponseSender
{
private Socket socket;
private InputStream input;
private OutputStream output;
private Request request;
private Response response;
private FitNesseContext context;
protected long requestParsingTimeLimit;
private long requestProgress;
private long requestParsingDeadline;
private boolean hasError;
public FitNesseExpediter(Socket s, FitNesseContext context) throws Exception
{
this.context = context;
socket = s;
input = s.getInputStream();
output = s.getOutputStream();
requestParsingTimeLimit = 10000;
}
public class FitNesseServer implements SocketServer { private FitNesseContext
context; public FitNesseServer(FitNesseContext context) { this.context =
context; } public void serve(Socket s) { serve(s, 10000); } public void
serve(Socket s, long requestTimeout) { try { FitNesseExpediter sender = new
FitNesseExpediter(s, context);
sender.setRequestParsingTimeLimit(requestTimeout); sender.start(); }
catch(Exception e) { e.printStackTrace(); } } }
-----
public class FitNesseServer implements SocketServer {
private FitNesseContext context;
public FitNesseServer(FitNesseContext context) {
this.context = context;
}
public void serve(Socket s) {
serve(s, 10000);
}
public void serve(Socket s, long requestTimeout) {
try {
FitNesseExpediter sender = new FitNesseExpediter(s, context);
sender.setRequestParsingTimeLimit(requestTimeout);
sender.start();
}
catch (Exception e) {
e.printStackTrace();
}
}
}
Наше зрение быстро охватывает структуру файла с отступами. Мы почти мгновенно находим переменные, конструкторы и методы. Всего за несколько секунд можно понять, что класс предоставляет простой интерфейс для работы с сокетом, с поддержкой тайм-аута. С другой стороны, разобраться в версии без отступов без тщательного анализа практически невозможно.
Нарушения отступов. Иногда возникает соблазн нарушить правила расстановки отступов в коротких командах if, коротких циклах while или коротких функциях. Но каждый раз, когда я поддавался этому искушению, я почти всегда возвращался и расставлял отступы, как положено. Таким образом, я стараюсь не сворачивать блоки в одну строку, как в этом фрагменте:
public class CommentWidget extends TextWidget
{
public static final String REGEXP = "^#[^\r\n]*(?:(?:\r\n)|\n|\r)?";
public CommentWidget(ParentWidget parent, String text){super(parent, text);}
public String render() throws Exception {return ""; }
}
Вместо этого я предпочитаю развернутые блоки с правильными отступами:
public class CommentWidget extends TextWidget {
public static final String REGEXP = "^#[^\r\n]*(?:(?:\r\n)|\n|\r)?";
public CommentWidget(ParentWidget parent, String text) {
super(parent, text);
}
public String render() throws Exception {
return "";
}
}
while (dis.read(buf, 0, readBufferSize) != -1)
;
public class CodeAnalyzer implements JavaFileAnalysis {
private int lineCount;
private int maxLineWidth;
private int widestLineNumber;
private LineWidthHistogram lineWidthHistogram;
private int totalChars;
public CodeAnalyzer() {
lineWidthHistogram = new LineWidthHistogram();
}
public static List<File> findJavaFiles(File parentDirectory) {
List<File> files = new ArrayList<File>();
findJavaFiles(parentDirectory, files);
return files;
}
private static void findJavaFiles(File parentDirectory, List<File> files) {
for (File file : parentDirectory.listFiles()) {
if (file.getName().endsWith(".java"))
files.add(file);
else if (file.isDirectory())
findJavaFiles(file, files);
}
}
public void analyzeFile(File javaFile) throws Exception {
BufferedReader br = new BufferedReader(new FileReader(javaFile));
String line;
while ((line = br.readLine()) != null)
measureLine(line);
}
private void measureLine(String line) {
lineCount++;
int lineSize = line.length();
totalChars += lineSize;
lineWidthHistogram.addLine(lineSize, lineCount);
recordWidestLine(lineSize);
}
private void recordWidestLine(int lineSize) {
if (lineSize > maxLineWidth) {
maxLineWidth = lineSize;
widestLineNumber = lineCount;
}
}
public int getLineCount() {
return lineCount;
}
public int getMaxLineWidth() {
return maxLineWidth;
}
public int getWidestLineNumber() {
return widestLineNumber;
}
public LineWidthHistogram getLineWidthHistogram() {
return lineWidthHistogram;
}
public double getMeanLineWidth() {
return (double)totalChars/lineCount;
}
public int getMedianLineWidth() {
Integer[] sortedWidths = getSortedWidths();
int cumulativeLineCount = 0;
for (int width : sortedWidths) {
cumulativeLineCount += lineCountForWidth(width);
if (cumulativeLineCount > lineCount/2)
return width;
}
throw new Error("Cannot get here");
}
private int lineCountForWidth(int width) {
return lineWidthHistogram.getLinesforWidth(width).size();
}
private Integer[] getSortedWidths() {
Set<Integer> widths = lineWidthHistogram.getWidths();
Integer[] sortedWidths = (widths.toArray(new Integer[0]));
Arrays.sort(sortedWidths);
return sortedWidths;
}
}
public class Point {
public double x;
public double y;
}
public interface Point {
double getX();
double getY();
void setCartesian(double x, double y);
double getR();
double getTheta();
void setPolar(double r, double theta);
}
Элегантность решения из листинга 6.2 заключается в том, что внешний пользователь не знает, какие координаты использованы в реализации — прямоугольные или полярные. А может, еще какие-нибудь! Тем не менее интерфейс безусловно напоминает структуру данных.
Однако он представляет нечто большее, чем обычную структуру данных. Его методы устанавливают политику доступа к данным. Пользователь может читать значения координат независимо друг от друга, но присваивание координат должно выполняться одновременно, в режиме атомарной операции.
С другой стороны, листинг 6.1 явно реализован в прямоугольных координатах, а пользователь вынужден работать с этими координатами независимо. Более того, такое решение раскрывает реализацию даже в том случае, если бы переменные были объявлены приватными, и мы использовали одиночные методы чтения/записи.
Скрытие реализации не сводится к созданию прослойки функций между переменными. Скрытие реализации направлено на формирование абстракций! Класс не просто ограничивает доступ к переменным через методы чтения/записи. Вместо этого он предоставляет абстрактные интерфейсы, посредством которых пользователь оперирует с сущностью данных. Знать, как эти данные реализованы, ему при этом не обязательно.
Возьмем листинги 6.3 и 6.4. В первом случае для получения информации о запасе топлива используются конкретные физические показатели, а во втором — абстрактные проценты. В первом, конкретном случае можно быть уверенным в том, что методы представляют собой обычные методы доступа к переменным. Во втором, абстрактном случае пользователь не имеет ни малейшего представления о фактическом формате данных.
public interface Vehicle {
double getFuelTankCapacityInGallons();
double getGallonsOfGasoline();
}
Abstract Vehicle
public interface Vehicle {
double getPercentFuelRemaining();
}
В обоих примерах вторая реализация является предпочтительной. Мы не хотим раскрывать подробности строения данных. Вместо этого желательно использовать представление данных на абстрактном уровне. Задача не решается простым использованием интерфейсов и/или методов чтения/записи. Чтобы найти лучший способ представления данных, содержащихся в объекте, необходимо серьезно поразмыслить. Бездумное добавление методов чтения и записи — худший из всех возможных вариантов.
public class Square {
public Point topLeft;
public double side;
}
public class Rectangle {
public Point topLeft;
public double height;
public double width;
}
public class Circle {
public Point center;
public double radius;
}
public class Geometry {
public final double PI = 3.141592653589793;
public double area(Object shape) throws NoSuchShapeException
{
if (shape instanceof Square) {
Square s = (Square)shape;
return s.side * s.side;
}
else if (shape instanceof Rectangle) {
Rectangle r = (Rectangle)shape;
return r.height * r.width;
}
else if (shape instanceof Circle) {
Circle c = (Circle)shape;
return PI * c.radius * c.radius;
}
throw new NoSuchShapeException();
}
}
Объектно-ориентированный программист недовольно поморщится и пожалуется на процедурную природу реализации — и будет прав. Но возможно, его презрительная усмешка не обоснована. Подумайте, что произойдет при включении в Geometry функции perimeter(). Классы фигур остаются неизменными! И все остальные классы, зависящие от них, тоже остаются неизменными! С другой стороны, при добавлении новой разновидности фигур мне придется изменять все функции Geometry, чтобы они могли работать с ней. Перечитайте еще раз. Обратите внимание на то, что эти два условия диаметрально противоположны.
Теперь рассмотрим объектно-ориентированное решение из листинга 6.6. Метод area() является полиморфным, класс Geometry становится лишним. Добавление новой фигуры не затрагивает ни одну из существующих функций, но при добавлении новой функции приходится изменять все фигуры![24]
Polymorphic Shapes
public class Square implements Shape {
private Point topLeft;
private double side;
public double area() {
return side*side;
}
}
public class Rectangle implements Shape {
private Point topLeft;
private double height;
private double width;
public double area() {
return height * width;
}
}
public class Circle implements Shape {
private Point center;
private double radius;
public final double PI = 3.141592653589793;
public double area() {
return PI * radius * radius;
}
}
И снова мы наблюдаем взаимодополняющую природу этих двух определений. В этом проявляется основополагающая дихотомия между объектами и структурами данных.
Процедурный код (код, использующий структуры данных) позволяет легко добавлять новые функции без изменения существующих структур данных. Объектно-ориентированный код, напротив, упрощает добавление новых классов без изменения существующих функций.
Обратные утверждения также истинны.
Процедурный код усложняет добавление новых структур данных, потому что оно требует изменения всех функций. Объектно-ориентированный код усложняет добавление новых функций, потому что для этого должны измениться все классы.
Таким образом, то, что сложно в ОО, просто в процедурном программировании, а то, что сложно в процедурном программировании, просто в ОО!
В любой сложной системе возникают ситуации, когда вместо новых функций в систему требуется включить новые типы данных. Для таких ситуаций объекты и объектно-ориентированное программирование особенно уместны. Впрочем, бывает и обратное — вместо новых типов данных требуется добавить новые функции. Тогда лучше подходит процедурный код и структуры данных.
Опытные программисты хорошо знают: представление о том, что все данные должны представляться в виде объектов — миф. Иногда предпочтительны простые структуры данных и процедуры, работающие с ними.
final String outputDir = ctxt.getOptions().getScratchDir().getAbsolutePath();
Options opts = ctxt.getOptions();
File scratchDir = opts.getScratchDir();
final String outputDir = scratchDir.getAbsolutePath();
final String outputDir = ctxt.options.scratchDir.absolutePath;
Ситуация существенно упростилась бы, если бы структуры данных просто содержали открытые переменные без функций, а объекты — приватные переменные с открытыми функциями. Однако некоторые существующие инфраструктуры и стандарты (например, Beans) требуют, чтобы даже простые структуры данных имели методы чтения и записи.
ctxt.getAbsolutePathOfScratchDirectoryOption();
или
ctx.getScratchDirectoryOption().getAbsolutePath()
Первый вариант приведет к разрастанию набора методов объекта ctxt. Второй вариант предполагает, что getScratchDirectoryOption() возвращает структуру данных, а не объект. Ни один из вариантов не вызывает энтузиазма.
Если ctxt является объектом, то мы должны приказать ему выполнить некую операцию, а не запрашивать у него информацию о его внутреннем устройстве. Зачем нам понадобился абсолютный путь к временному каталогу? Что мы собираемся с ним делать? Рассмотрим следующий фрагмент того же модуля (расположенный на много строк ниже):
String outFile = outputDir + "/" + className.replace('.', '/') + ".class";
FileOutputStream fout = new FileOutputStream(outFile);
BufferedOutputStream bos = new BufferedOutputStream(fout);
Смешение разных уровней детализации [G34][G6] выглядит немного пугающе. Точки, косые черты, расширения файлов и объекты File не должны так беспечно перемешиваться между собой и с окружающим кодом. Но если не обращать на это внимания, мы видим, что абсолютный путь временного каталога определялся для создания временного файла с заданным именем.
Так почему бы не приказать объекту ctxt выполнить эту операцию?
BufferedOutputStream bos = ctxt.createScratchFileStream(classFileName);
Выглядит вполне разумно! Такое решение позволяет объекту ctxt скрыть свое внутреннее строение, а текущей функции не приходится нарушать закон Деметры, перемещаясь между объектами, о которых ей знать не положено.
public class Address {
private String street;
private String streetExtra;
private String city;
private String state;
private String zip;
public Address(String street, String streetExtra,
String city, String state, String zip) {
this.street = street;
this.streetExtra = streetExtra;
this.city = city;
this.state = state;
this.zip = zip;
}
public String getStreet() {
return street;
}
public String getStreetExtra() {
return streetExtra;
}
public String getCity() {
return city;
}
public String getState() {
return state;
}
public String getZip() {
return zip;
}
}
Майкл Физерс
public class DeviceController {
...
public void sendShutDown() {
DeviceHandle handle = getHandle(DEV1);
// Проверить состояние устройства
if (handle != DeviceHandle.INVALID) {
// Сохранить состояние устройства в поле записи
retrieveDeviceRecord(handle);
// Если устройство не приостановлено, отключить его
if (record.getStatus() != DEVICE_SUSPENDED) {
pauseDevice(handle);
clearDeviceWorkQueue(handle);
closeDevice(handle);
} else {
logger.log("Device suspended. Unable to shut down");
}
} else {
logger.log("Invalid handle for: " + DEV1.toString());
}
}
...
}
У обоих решений имеется общий недостаток: они загромождают код на стороне вызова. Вызывающая сторона должна проверять ошибки немедленно после вызова. К сожалению, об этом легко забыть. По этой причине при обнаружении ошибки лучше инициировать исключение. Код вызова становится более понятным, а его логика не скрывается за кодом обработки ошибок.
В листинге 7.2 представлен тот же код с выдачей исключений в методах, способных обнаруживать ошибки.
Обратите внимание, насколько чище стал код. Причем дело даже не в эстетике. Качество кода возросло, потому что два аспекта, которые прежде были тесно переплетены — алгоритм отключения устройства и обработка ошибок, — теперь изолированы друг от друга. Вы можете рассмотреть их по отдельности и разобраться в каждом из них независимо.
public class DeviceController {
...
public void sendShutDown() {
try {
tryToShutDown();
} catch (DeviceShutDownError e) {
logger.log(e);
}
}
private void tryToShutDown() throws DeviceShutDownError {
DeviceHandle handle = getHandle(DEV1);
DeviceRecord record = retrieveDeviceRecord(handle);
pauseDevice(handle);
clearDeviceWorkQueue(handle);
closeDevice(handle);
}
private DeviceHandle getHandle(DeviceID id) {
...
throw new DeviceShutDownError("Invalid handle for: " + id.toString());
...
}
...
}
@Test(expected = StorageException.class)
public void retrieveSectionShouldThrowOnInvalidFileName() {
sectionStore.retrieveSection("invalid - file");
}
Для теста необходимо создать следующую программную заглушку:
public List<RecordedGrip> retrieveSection(String sectionName) {
// Пусто, пока не появится реальная реализация
return new ArrayList<RecordedGrip>();
}
Тест завершается неудачей, потому что код не инициирует исключения. Затем мы изменяем свою реализацию так, чтобы она попыталась обратиться к несуществующему файлу. При попытке выполнения происходит исключение:
public List<RecordedGrip> retrieveSection(String sectionName) {
try {
FileInputStream stream = new FileInputStream(sectionName)
} catch (Exception e) {
throw new StorageException("retrieval error", e);
}
return new ArrayList<RecordedGrip>();
}
Теперь тест проходит успешно, потому что мы перехватили исключение. На этой стадии можно переработать код. Тип перехватываемого исключения сужается до типа, реально инициируемого конструктором FileInputStream, то есть FileNotFoundException:
public List<RecordedGrip> retrieveSection(String sectionName) {
try {
FileInputStream stream = new FileInputStream(sectionName);
stream.close();
} catch (FileNotFoundException e) {
throw new StorageException("retrieval error", e);
}
return new ArrayList<RecordedGrip>();
}
Определив область видимости при помощи структуры try-catch, мы можем использовать методологию TDD для построения остальной необходимой логики. Эта логика размещается между созданием FileInputStream и закрытием, а в ее коде можно считать, что все операции были выполнены без ошибок.
Попробуйте написать тесты, принудительно инициирующие исключения, а затем включите в обработчик поведение, обеспечивающее прохождение тестов. Это заставит вас построить транзакционную область видимости блока try и поможет сохранить ее транзакционную природу.
ACMEPort port = new ACMEPort(12);
try {
port.open();
} catch (DeviceResponseException e) {
reportPortError(e);
logger.log("Device response exception", e);
} catch (ATM1212UnlockedException e) {
reportPortError(e);
logger.log("Unlock exception", e);
} catch (GMXError e) {
reportPortError(e);
logger.log("Device response exception");
} finally {
…
}
Конструкция содержит множество повторений, и это неудивительно. В большинстве ситуаций при обработке исключений выполняются относительно стандартные действия, не зависящие от их реальной причины. Мы должны сохранить ошибку и убедиться в том, что работа программы может быть продолжена. В этом случае, поскольку выполняемая работа остается более или менее постоянной независимо от исключения, код можно существенно упростить — для этого мы создаем «обертку» для вызываемой функции API и обеспечиваем возвращение стандартного типа исключения:
LocalPort port = new LocalPort(12);
try {
port.open();
} catch (PortDeviceFailure e) {
reportError(e);
logger.log(e.getMessage(), e);
} finally {
…
}
Класс LocalPort представляет собой простую обертку, которая перехватывает и преобразует исключения, инициированные классом ACMEPort:
public class LocalPort {
private ACMEPort innerPort;
public LocalPort(int portNumber) {
innerPort = new ACMEPort(portNumber);
}
public void open() {
try {
innerPort.open();
} catch (DeviceResponseException e) {
throw new PortDeviceFailure(e);
} catch (ATM1212UnlockedException e) {
throw new PortDeviceFailure(e);
} catch (GMXError e) {
throw new PortDeviceFailure(e);
}
}
…
}
Обертки — вроде той, которую мы определили для ACMEPort, — бывают очень полезными. Более того, инкапсуляция вызовов сторонних API принадлежит к числу стандартных приемов. Создавая обертку для стороннего вызова, вы сокращаете до минимума зависимость от него в своем коде: в будущем вы можете переключиться на другую библиотеку без сколько-нибудь заметных проблем. Обертки также упрощают имитацию сторонних вызовов в ходе тестирования кода.
Последнее преимущество оберток заключается в том, что вы не ограничиваетесь архитектурными решениями разработчика API. Вы можете определить тот API, который вам удобен. В предыдущем примере мы определили для всех сбоев порта один тип исключения, и код от этого стал намного чище.
Часто в определенной области кода бывает достаточно одного класса исключения. Информация, передаваемая с исключением, позволяет различать разные виды ошибок. Используйте разные классы исключений только в том случае, если вы намерены перехватывать одни исключения, разрешая прохождение других типов.
try {
MealExpenses expenses = expenseReportDAO.getMeals(employee.getID());
m_total += expenses.getTotal();
} catch(MealExpensesNotFound e) {
m_total += getMealPerDiem();
}
Если работник предъявил счет по затратам на питание, то сумма включается в общий итог. Если счет отсутствует, то работнику за этот день начисляется определенная сумма. Исключение загромождает логику программы. А если бы удалось обойтись без обработки особого случая? Это позволило бы заметно упростить код:
MealExpenses expenses = expenseReportDAO.getMeals(employee.getID());
m_total += expenses.getTotal();
Можно ли упростить код до такой формы? Оказывается, можно. Мы можем изменить класс ExpenseReportDAO, чтобы он всегда возвращал объект MealExpense. При отсутствии предъявленного счета возвращается объект MealExpense, у которого в качестве затрат указана стандартная сумма, начисляемая за день:
public class PerDiemMealExpenses implements MealExpenses {
public int getTotal() {
// Вернуть стандартные ежедневные затраты на питание
}
}
Такое решение представляет собой реализацию паттерна ОСОБЫЙ СЛУЧАЙ [Fowler]. Программист создает класс или настраивает объект так, чтобы он обрабатывал особый случай за него. Это позволяет избежать обработки исключительного поведения в клиентском коде. Все необходимое поведение инкапсулируется в объекте особого случая.
public void registerItem(Item item) {
if (item != null) {
ItemRegistry registry = persistentStore.getItemRegistry();
if (registry != null) {
Item existing = registry.getItem(item.getID());
if (existing.getBillingPeriod().hasRetailOwner()) {
existing.register(item);
}
}
}
}
Если ваша кодовая база содержит подобный код, возможно, вы не видите в нем ничего плохого, но это не так! Возвращая null, мы фактически создаем для себя лишнюю работу, а для вызывающей стороны — лишние проблемы. Стоит пропустить всего одну проверку null, и приложение «уходит в штопор».
А вы заметили, что во второй строке вложенной команды if проверка null отсутствует? Что произойдет во время выполнения, если значение persistentStore окажется равным null? Произойдет исключение NullPointerException; либо кто-то перехватит его на верхнем уровне, либо не перехватит. В обоих случаях все будет плохо. Как реагировать на исключение NullPointerException, возникшее где-то в глубинах вашего приложения?
Легко сказать, что проблемы в приведенном коде возникли из-за пропущенной проверки null. В действительности причина в другом: этих проверок слишком много. Если у вас возникает желание вернуть null из метода, рассмотрите возможность выдачи исключения или возвращения объекта «особого случая». Если ваш код вызывает метод стороннего API, способный вернуть null, создайте для него обертку в виде метода, который инициирует исключение или возвращает объект особого случая.
Довольно часто объекты особых случаев легко решают проблему. Допустим, у вас имеется код следующего вида:
List<Employee> employees = getEmployees();
if (employees != null) {
for(Employee e : employees) {
totalPay += e.getPay();
}
}
Сейчас метод getEmployees может возвращать null, но так ли это необходимо? Если изменить getEmployee так, чтобы метод возвращал пустой список, код станет чище:
List<Employee> employees = getEmployees();
for(Employee e : employees) {
totalPay += e.getPay();
}
К счастью, в Java существует метод Collections.emptyList(), который возвращает заранее определенный неизменяемый список, и мы можем воспользоваться им для своих целей:
public List<Employee> getEmployees() {
if( .. there are no employees .. )
return Collections.emptyList();
}
Такое решение сводит к минимуму вероятность появления NullPointerException, а код становится намного чище.
public class MetricsCalculator
{
public double xProjection(Point p1, Point p2) {
return (p2.x — p1.x) * 1.5;
}
…
}
Что произойдет, если при вызове будет передан аргумент null?
calculator.xProjection(null, new Point(12, 13));
Конечно, возникнет исключение NullPointerException.
Как исправить его? Можно создать новый тип исключения и инициировать его в методе:
public class MetricsCalculator
{
public double xProjection(Point p1, Point p2) {
if (p1 == null || p2 == null) {
throw InvalidArgumentException(
"Invalid argument for MetricsCalculator.xProjection");
}
return (p2.x — p1.x) * 1.5;
}
}
Стало лучше? Пожалуй, лучше, чем NullPointerException, но вспомните: для InvalidArgumentException приходится определять обработчик. Что должен делать этот обработчик? Возьметесь предложить хорошую идею?
Существует другая альтернатива: можно воспользоваться набором проверочных директив assert:
public class MetricsCalculator
{
public double xProjection(Point p1, Point p2) {
assert p1 != null : "p1 should not be null";
assert p2 != null : "p2 should not be null";
return (p2.x — p1.x) * 1.5;
}
}
Неплохо с точки зрения документирования, но проблема не решена. Если при вызове передать null, произойдет ошибка времени выполнения.
В большинстве языков программирования не существует хорошего способа справиться со случайной передачей null с вызывающей стороны. А раз так, разумно запретить передачу null по умолчанию. В этом случае вы будете знать, что присутствие null в списке аргументов свидетельствует о возникшей проблеме; это будет способствовать уменьшению количества ошибок, сделанных по неосторожности.
Джеймс Гренинг
• clear() void – Map
• containsKey(Object key) boolean – Map
• containsValue(Object value) boolean – Map
• entrySet() Set – Map
• equals(Object o) boolean – Map
• get(Object key) Object – Map
• getClass() Class<? extends Object> – Object
• hashCode() int – Map
• isEmpty() boolean – Map
• keySet() Set – Map
• notify() void – Object
• notifyAll() void – Object
• put(Object key, Object value) Object – Map
• putAll(Map t) void – Map
• remove(Object key) Object – Map
• size() int – Map
• toString() String – Object
• values() Collection – Map
• wait() void – Object
• wait(long timeout) void – Object
• wait(long timeout, int nanos) void – Object
Рис. 8.1. Методы Map
Map sensors = new HashMap();
Когда другой части кода понадобится обратиться к элементу, мы видим код следующего вида:
Sensor s = (Sensor)sensors.get(sensorId );
Причем видим его не только в этом месте, но снова и снова по всему коду. Клиент кода несет ответственность за получение Object из Map и его приведение к правильному типу. Такое решение работает, но «чистым» его не назовешь. Кроме того, этот код не излагает свою историю, как ему положено. Удобочитаемость кода можно было бы заметно улучшить при помощи шаблонов (параметризованных контейнеров):
Map<Sensor> sensors = new HashMap<Sensor>();
...
Sensor s = sensors.get(sensorId );
Но и такая реализация не решает проблемы: Map<Sensor> предоставляет намного больше возможностей, чем нам хотелось бы.
Свободная передача Map<Sensor> по системе означает, что в случае изменения интерфейса Map исправления придется вносить во множестве мест. Казалось бы, такие изменения маловероятны, но вспомните, что интерфейс изменился при добавлении поддержки шаблонов в Java 5. В самом деле, мы видели системы, разработчики которых воздерживались от использования шаблонов из-за большого количества потенциальных изменений, связанных с частым использованием Map.
Ниже представлен другой, более чистый вариант использования Map. С точки зрения пользователя Sensors совершенно не важно, используются шаблоны или нет. Это решение стало (и всегда должно быть) подробностью реализации.
public class Sensors {
private Map sensors = new HashMap();
public Sensor getById(String id) {
return (Sensor) sensors.get(id);
}
//...
}
Граничный интерфейс (Map) скрыт от пользователя. Он может развиваться независимо, практически не оказывая никакого влияния на остальные части приложения. Применение шаблонов уже не создает проблем, потому что все преобразования типов выполняются в классе Sensors.
Этот интерфейс также приспособлен и ограничен в соответствии с потребностями приложения. Код становится более понятным, а возможности злоупотреблений со стороны пользователя сокращаются. Класс Sensors может обеспечивать выполнение архитектурных требований и требований бизнес-логики.
Поймите правильно: мы не предлагаем инкапсулировать каждое применение Map в этой форме. Скорее, мы рекомендуем ограничить передачу Map (или любого другого граничного интерфейса) по системе. Если вы используете граничный интерфейс вроде Map, держите его внутри класса (или тесно связанного семейства классов), в которых он используется. Избегайте его возвращения или передачи в аргументах при вызовах методов общедоступных API.
@Test
public void testLogCreate() {
Logger logger = Logger.getLogger("MyLogger");
logger.info("hello");
}
При запуске журнальный модуль выдает ошибку. В описании ошибки говорится, что нам понадобится нечто под названием Appender. После непродолжительных поисков в документации обнаруживается класс ConsoleAppender. Соответственно, мы создаем объект ConsoleAppender и проверяем, удалось ли нам раскрыть секреты вывода журнала на консоль:
@Test
public void testLogAddAppender() {
Logger logger = Logger.getLogger("MyLogger");
ConsoleAppender appender = new ConsoleAppender();
logger.addAppender(appender);
logger.info("hello");
}
На этот раз выясняется, что у объекта Appender нет выходного потока. Странно – логика подсказывает, что он должен быть. После небольшой помощи от Google опробуется следующее решение:
@Test
public void testLogAddAppender() {
Logger logger = Logger.getLogger("MyLogger");
logger.removeAllAppenders();
logger.addAppender(new ConsoleAppender(
new PatternLayout("%p %t %m%n"),
ConsoleAppender.SYSTEM_OUT));
logger.info("hello");
}
Заработало; на консоли выводится сообщение со словом «hello»! На первый взгляд происходящее выглядит немного странно: мы должны указывать ConsoleAppender, что данные выводятся на консоль.
Еще интереснее, что при удалении аргумента ConsoleAppender.SystemOut сообщение «hello» все равно выводится. Но если убрать аргумент PatternLayout, снова начинаются жалобы на отсутствие выходного потока. Все это выглядит очень странно.
После более внимательного чтения документации мы видим, что конструктор ConsoleAppender по умолчанию «не имеет конфигурации» – весьма неочевидное и бесполезное решение. Похоже, это ошибка (или по крайней мере нелогичность) в log4j.
После некоторых поисков, чтения документации и тестирования мы приходим к листингу 8.1. Попутно мы получили много полезной информации о том, как работает log4j, и закодировали ее в наборе простых модульных тестов.
public class LogTest {
private Logger logger;
@Before
public void initialize() {
logger = Logger.getLogger("logger");
logger.removeAllAppenders();
Logger.getRootLogger().removeAllAppenders();
}
@Test
public void basicLogger() {
BasicConfigurator.configure();
logger.info("basicLogger");
}
@Test
public void addAppenderWithStream() {
logger.addAppender(new ConsoleAppender(
new PatternLayout("%p %t %m%n"),
ConsoleAppender.SYSTEM_OUT));
logger.info(“addAppenderWithStream”);
}
@Test
public void addAppenderWithoutStream() {
logger.addAppender(new ConsoleAppender(
new PatternLayout("%p %t %m%n")));
logger.info("addAppenderWithoutStream");
}
}
Теперь мы знаем, как инициализировать простейший консольный вывод и можем воплотить эти знания в специализированном журнальном классе, чтобы изолировать остальной код приложения от граничного интерфейса log4j.
void Timer::ScheduleCommand(Command* theCommand, int milliseconds)
Идея была проста; метод Execute класса Command выполнялся в новом программном потоке с заданной задержкой в миллисекундах. Оставалось понять, как его тестировать.
Я соорудил простую управляющую программу, которая прослушивала события клавиатуры. Каждый раз, когда на клавиатуре вводился символ, программа планировала выполнение команды, повторяющей этот же символ пять секунд спустя. Затем я настучал на клавиатуре ритмичную мелодию и подождал, пока эта мелодия «появится» на экране спустя пять секунд.
«Мне… нужна такая девушка… как та… которую нашел мой старый добрый папа…»
Я напевал эту мелодию, нажимая клавишу «.», а потом пропел ее снова, когда точки начали появляться на экране.
И это был весь тест! Я убедился в том, что программа работает, показал ее своим коллегам и выкинул тестовый код.
Как я уже говорил, наша профессия прошла долгий путь. Сейчас я бы написал комплексный тест, проверяющий, что все углы и закоулки моего кода работают именно так, как положено. Я бы изолировал свой код от операционной системы, не полагаясь на стандартное выполнение по таймеру. Я бы самостоятельно реализовал хронометраж, чтобы тестирование проходило под моим полным контролем. Запланированные команды устанавливали бы логические флаги, а потом тестовый код выполнял бы мою программу в пошаговом режиме, наблюдая за состоянием флагов и их переходами из ложного состояния в истинное по прохождении нужного времени.
Когда у меня накопился бы пакет тестов, я бы позаботился о том, чтобы эти тесты были удобными для любого другого программиста, которому потребуется работать с моим кодом. Я бы проследил за тем, чтобы тесты и код поставлялись вместе, в одном исходном пакете.
Да, мы прошли долгий путь; но дорога еще не пройдена до конца. Движения гибких методологий и TDD поощряют многих программистов писать автоматизированные модульные тесты, а их ряды ежедневно пополняются новыми сторонниками. Однако в лихорадочном стремлении интегрировать тестирование в свою работу многие программисты упускают более тонкие и важные аспекты написания хороших тестов.
public void testGetPageHieratchyAsXml() throws Exception
{
crawler.addPage(root, PathParser.parse("PageOne"));
crawler.addPage(root, PathParser.parse("PageOne.ChildOne"));
crawler.addPage(root, PathParser.parse("PageTwo"));
request.setResource("root");
request.addInput("type", "pages");
Responder responder = new SerializedPageResponder();
SimpleResponse response =
(SimpleResponse) responder.makeResponse(
new FitNesseContext(root), request);
String xml = response.getContent();
assertEquals("text/xml", response.getContentType());
assertSubString("<name>PageOne</name>", xml);
assertSubString("<name>PageTwo</name>", xml);
assertSubString("<name>ChildOne</name>", xml);
}
public void testGetPageHieratchyAsXmlDoesntContainSymbolicLinks()
throws Exception
{
WikiPage pageOne = crawler.addPage(root, PathParser.parse("PageOne"));
crawler.addPage(root, PathParser.parse("PageOne.ChildOne"));
crawler.addPage(root, PathParser.parse("PageTwo"));
PageData data = pageOne.getData();
WikiPageProperties properties = data.getProperties();
WikiPageProperty symLinks = properties.set(SymbolicPage.PROPERTY_NAME);
symLinks.set("SymPage", "PageTwo");
pageOne.commit(data);
request.setResource("root");
request.addInput("type", "pages");
Responder responder = new SerializedPageResponder();
SimpleResponse response =
(SimpleResponse) responder.makeResponse(
new FitNesseContext(root), request);
String xml = response.getContent();
assertEquals("text/xml", response.getContentType());
assertSubString("<name>PageOne</name>", xml);
assertSubString("<name>PageTwo</name>", xml);
assertSubString("<name>ChildOne</name>", xml);
assertNotSubString("SymPage", xml);
}
public void testGetDataAsHtml() throws Exception
{
crawler.addPage(root, PathParser.parse("TestPageOne"), "test page");
request.setResource("TestPageOne");
request.addInput("type", "data");
Responder responder = new SerializedPageResponder();
SimpleResponse response =
(SimpleResponse) responder.makeResponse(
new FitNesseContext(root), request);
String xml = response.getContent();
assertEquals("text/xml", response.getContentType());
assertSubString("test page", xml);
assertSubString("<Test", xml);
}
Например, присмотритесь к вызовам PathParser, преобразующим строки в экземпляры PagePath, используемые обходчиками (crawlers). Это преобразование абсолютно несущественно для целей тестирования и только затемняет намерения автора. Второстепенные подробности, окружающие создание ответчика, а также сбор и преобразование ответа тоже представляют собой обычный шум. Также обратите внимание на неуклюжий способ построения URL-адреса запроса из ресурса и аргумента. (Я участвовал в написании этого кода, поэтому считаю, что вправе критиковать его.)
В общем, этот код не предназначался для чтения. На несчастного читателя обрушивается целый водопад мелочей, в которых необходимо разобраться, чтобы уловить в тестах хоть какой-то смысл.
Теперь рассмотрим усовершенствованные тесты в листинге 9.2. Они делают абсолютно то же самое, но код был переработан в более ясную и выразительную форму.
public void testGetPageHierarchyAsXml() throws Exception {
makePages("PageOne", "PageOne.ChildOne", "PageTwo");
submitRequest("root", "type:pages");
assertResponseIsXML();
assertResponseContains(
"<name>PageOne</name>", "<name>PageTwo</name>", "<name>ChildOne</name>"
);
}
public void testSymbolicLinksAreNotInXmlPageHierarchy() throws Exception {
WikiPage page = makePage("PageOne");
makePages("PageOne.ChildOne", "PageTwo");
addLinkTo(page, "PageTwo", "SymPage");
submitRequest("root", "type:pages");
assertResponseIsXML();
assertResponseContains(
"<name>PageOne</name>", "<name>PageTwo</name>", "<name>ChildOne</name>"
);
assertResponseDoesNotContain("SymPage");
}
public void testGetDataAsXml() throws Exception {
makePageWithContent("TestPageOne", "test page");
submitRequest("TestPageOne", "type:data");
assertResponseIsXML();
assertResponseContains("test page", "<Test");
}
В структуре тестов очевидно воплощен паттерн ПОСТРОЕНИЕ-ОПЕРАЦИИ-ПРОВЕРКА[29]. Каждый тест четко делится на три части. Первая часть строит тестовые данные, вторая часть выполняет операции с тестовыми данными, а третья часть проверяет, что операция привела к ожидаемым результатам.
Обратите внимание: большая часть раздражающих мелочей исчезла. Тесты не делают ничего лишнего, и в них используются только действительно необходимые типы данных и функции.
Любой программист, читающий эти тесты, очень быстро разберется в том, что они делают, не сбиваясь с пути и не увязнув в лишних подробностях.
@Test
public void turnOnLoTempAlarmAtThreashold() throws Exception {
hw.setTemp(WAY_TOO_COLD);
controller.tic();
assertTrue(hw.heaterState());
assertTrue(hw.blowerState());
assertFalse(hw.coolerState());
assertFalse(hw.hiTempAlarm());
assertTrue(hw.loTempAlarm());
}
Конечно, этот листинг содержит множество ненужных подробностей. Например, что делает функция tic? Я бы предпочел, чтобы читатель не задумывался об этом в ходе чтения теста. Читатель должен думать о другом: соответствует ли конечное состояние системы его представлениям о «слишком низкой» температуре.
Обратите внимание: в ходе чтения теста вам постоянно приходится переключаться между названием проверяемого состояния и условием проверки. Вы смотрите на heaterState (состояние обогревателя), а затем ваш взгляд скользит налево к assertTrue. Вы смотрите на coolerState (состояние охладителя), а ваш взгляд отступает к assertFalse. Все эти перемещения утомительны и ненадежны. Они усложняют чтение теста.
В листинге 9.4 представлена новая форма теста, которая читается гораздо проще.
@Test
public void turnOnLoTempAlarmAtThreshold() throws Exception {
wayTooCold();
assertEquals("HBchL", hw.getState());
}
Конечно, я скрыл функцию tic, создав более понятную функцию wayTooCold. Но особого внимания заслуживает странная строка в вызове assertEquals. Верхний регистр означает включенное состояние, нижний регистр — выключенное состояние, а буквы всегда следуют в определенном порядке: {обогреватель, подача воздуха, охладитель, сигнал о высокой температуре, сигнал о низкой температуре}.
Хотя такая форма близка к нарушению правила о мысленных преобразованиях[30], в данном случае она выглядит уместной. Если вам известен смысл этих обозначений, ваш взгляд скользит по строке в одном направлении и вы можете быстро интерпретировать результаты. Чтение таких тестов почти что доставляет удовольствие. Взгляните на листинг 9.5 и убедитесь, как легко понять их смысл.
@Test
public void turnOnCoolerAndBlowerIfTooHot() throws Exception {
tooHot();
assertEquals("hBChl", hw.getState());
}
@Test
public void turnOnHeaterAndBlowerIfTooCold() throws Exception {
tooCold();
assertEquals("HBchl", hw.getState());
}
@Test
public void turnOnHiTempAlarmAtThreshold() throws Exception {
wayTooHot();
assertEquals("hBCHl", hw.getState());
}
@Test
public void turnOnLoTempAlarmAtThreshold() throws Exception {
wayTooCold();
assertEquals("HBchL", hw.getState());
}
Функция getState приведена в листинге 9.6. Обратите внимание: эффективность этого кода оставляет желать лучшего. Чтобы сделать его более эффективным, вероятно, мне стоило использовать класс StringBuffer.
public String getState() {
String state = "";
state += heater ? "H" : "h";
state += blower ? "B" : "b";
state += cooler ? "C" : "c";
state += hiTempAlarm ? "H" : "h";
state += loTempAlarm ? "L" : "l";
return state;
}
Класс StringBuffer некрасив и неудобен. Даже в коде продукта я стараюсь избегать его, если это не приводит к большим потерям; конечно, в коде из листинга 9.6 потери невелики. Однако следует учитывать, что приложение пишется для встроенной системы реального времени, в которой вычислительные ресурсы и память сильно ограничены. С другой стороны, в среде тестирования такие ограничения отсутствуют.
В этом проявляется природа двойного стандарта. Многое из того, что вы никогда не станете делать в среде эксплуатации продукта, абсолютно нормально выглядит в среде тестирования. Обычно речь идет о затратах памяти или эффективности работы процессора — но никогда о проблемах чистоты кода.
public void testGetPageHierarchyAsXml() throws Exception {
givenPages("PageOne", "PageOne.ChildOne", "PageTwo");
whenRequestIsIssued("root", "type:pages");
thenResponseShouldBeXML();
}
public void testGetPageHierarchyHasRightTags() throws Exception {
givenPages("PageOne", "PageOne.ChildOne", "PageTwo");
whenRequestIsIssued("root", "type:pages");
thenResponseShouldContain(
"<name>PageOne</name>", "<name>PageTwo</name>", "<name>ChildOne</name>"
);
}
Обратите внимание: я переименовал функции в соответствии со стандартной схемой given-when-then [RSpec]. Это еще сильнее упрощает чтение тестов. К сожалению, такое разбиение приводит к появлению большого количества дублирующегося кода.
Чтобы избежать дублирования, можно воспользоваться паттерном ШАБЛОННЫЙ МЕТОД [GOF], включить части given/when в базовый класс, а части then — в различные производные классы. А можно создать отдельный тестовый класс, поместить части given и when в функцию @Before, а части then — в каждую функцию @Test. Но похоже, такой механизм слишком сложен для столь незначительной проблемы. В конечном итоге я предпочел решение с множественными директивами assert из листинга 9.2.
Я думаю, что правило «одного assert» является хорошей рекомендацией. Обычно я стараюсь создать предметно-ориентированный язык тестирования, который это правило поддерживает, как в листинге 9.5. Но при этом я не боюсь включать в свои тесты более одной директивы assert. Вероятно, лучше всего сказать, что количество директив assert в тесте должно быть сведено к минимуму.
/**
* Тесты для метода addMonths().
*/
public void testAddMonths() {
SerialDate d1 = SerialDate.createInstance(31, 5, 2004);
SerialDate d2 = SerialDate.addMonths(1, d1);
assertEquals(30, d2.getDayOfMonth());
assertEquals(6, d2.getMonth());
assertEquals(2004, d2.getYYYY());
SerialDate d3 = SerialDate.addMonths(2, d1);
assertEquals(31, d3.getDayOfMonth());
assertEquals(7, d3.getMonth());
assertEquals(2004, d3.getYYYY());
SerialDate d4 = SerialDate.addMonths(1, SerialDate.addMonths(1, d1));
assertEquals(30, d4.getDayOfMonth());
assertEquals(7, d4.getMonth());
assertEquals(2004, d4.getYYYY());
}
Вероятно, три тестовые функции должны выглядеть так:
• Given: последний день месяца, состоящего из 31 дня (например, май).
1) When: при добавлении одного месяца, последним днем которого является 30-е число (например, июнь), датой должно быть 30-е число этого месяца, а не 31-е.
2) When: при добавлении двух месяцев, когда последним днем второго месяца является 31-е число, датой должно быть 31-е число.
• Given: последний день месяца, состоящего из 30 дней (например, июнь).
1) When: при добавлении одного месяца, последним днем которого является 31-е число, датой должно быть 30-е число этого месяца, а не 31-е.
В такой формулировке видно, что среди разнородных тестов скрывается одно общее правило. При увеличении месяца дата не может превысить последнее число нового месяца. Следовательно, в результате увеличение месяца для 28 февраля должна быть получена дата 28 марта. В текущей версии это условие не проверяется, хотя такой тест был бы полезен.
Таким образом, проблема не в множественных директивах assert в разных секциях листинга 9.8. Проблема в том, что тест проверяет более одной концепции. Так что, вероятно, лучше всего сформулировать это правило так: количество директив assert на концепцию должно быть минимальным, и в тестовой функции должна проверяться только одна концепция.
Совместно с Джеффом Лангром
public class SuperDashboard extends JFrame implements MetaDataUser
public String getCustomizerLanguagePath()
public void setSystemConfigPath(String systemConfigPath)
public String getSystemConfigDocument()
public void setSystemConfigDocument(String systemConfigDocument)
public boolean getGuruState()
public boolean getNoviceState()
public boolean getOpenSourceState()
public void showObject(MetaObject object)
public void showProgress(String s)
public boolean isMetadataDirty()
public void setIsMetadataDirty(boolean isMetadataDirty)
public Component getLastFocusedComponent()
public void setLastFocused(Component lastFocused)
public void setMouseSelectState(boolean isMouseSelected)
public boolean isMouseSelected()
public LanguageManager getLanguageManager()
public Project getProject()
public Project getFirstProject()
public Project getLastProject()
public String getNewProjectName()
public void setComponentSizes(Dimension dim)
public String getCurrentDir()
public void setCurrentDir(String newDir)
public void updateStatus(int dotPos, int markPos)
public Class[] getDataBaseClasses()
public MetadataFeeder getMetadataFeeder()
public void addProject(Project project)
public boolean setCurrentProject(Project project)
public boolean removeProject(Project project)
public MetaProjectHeader getProgramMetadata()
public void resetDashboard()
public Project loadProject(String fileName, String projectName)
public void setCanSaveMetadata(boolean canSave)
public MetaObject getSelectedObject()
public void deselectObjects()
public void setProject(Project project)
public void editorAction(String actionName, ActionEvent event)
public void setMode(int mode)
public FileManager getFileManager()
public void setFileManager(FileManager fileManager)
public ConfigManager getConfigManager()
public void setConfigManager(ConfigManager configManager)
public ClassLoader getClassLoader()
public void setClassLoader(ClassLoader classLoader)
public Properties getProps()
public String getUserHome()
public String getBaseDir()
public int getMajorVersionNumber()
public int getMinorVersionNumber()
public int getBuildNumber()
public MetaObject pasting(
MetaObject target, MetaObject pasted, MetaProject project)
public void processMenuItems(MetaObject metaObject)
public void processMenuSeparators(MetaObject metaObject)
public void processTabPages(MetaObject metaObject)
public void processPlacement(MetaObject object)
public void processCreateLayout(MetaObject object)
public void updateDisplayLayer(MetaObject object, int layerIndex)
public void propertyEditedRepaint(MetaObject object)
public void processDeleteObject(MetaObject object)
public boolean getAttachedToDesigner()
public void processProjectChangedState(boolean hasProjectChanged)
public void processObjectNameChanged(MetaObject object)
public void runProject()
public void setAçowDragging(boolean allowDragging)
public boolean allowDragging()
public boolean isCustomizing()
public void setTitle(String title)
public IdeMenuBar getIdeMenuBar()
public void showHelper(MetaObject metaObject, String propertyName)
// ... и еще много других, не-открытых методов...
}
А если бы класс SuperDashboard содержал только методы, приведенные в листинге 10.2?
public class SuperDashboard extends JFrame implements MetaDataUser
public Component getLastFocusedComponent()
public void setLastFocused(Component lastFocused)
public int getMajorVersionNumber()
public int getMinorVersionNumber()
public int getBuildNumber()
}
Пять методов — не слишком много, не так ли? В нашем случае слишком, потому что несмотря на малое количество методов, класс SuperDashboard по-прежнему имеет слишком много ответственностей.
Имя класса должно описывать его ответственности. В сущности, имя должно стать первым фактором, способствующим определению размера класса. Если для класса не удается подобрать четкое, короткое имя, вероятно, он слишком велик. Чем туманнее имя класса, тем больше вероятность, что он имеет слишком много ответственностей. В частности, присутствие в именах классов слов-проныр «Processor», «Manager» и «Super» часто свидетельствует о нежелательном объединении ответственностей.
Краткое описание класса должно укладываться примерно в 25 слов, без выражений «если», «и», «или» и «но». Как бы вы описали класс SuperDashboard? «Класс предоставляет доступ к компоненту, который последним имел фокус ввода, и позволяет отслеживать номера версии и сборки». Первое «и» указывает на то, что SuperDashboard имеет слишком много ответственностей.
public class Version {
public int getMajorVersionNumber()
public int getMinorVersionNumber()
public int getBuildNumber()
}
Принцип единой ответственности — одна из самых важных концепций в объектно-ориентированном проектировании. Кроме того, его относительно несложно понять и соблюдать. Но как ни странно, принцип единой ответственности часто оказывается самым нарушаемым принципом проектирования классов. Мы постоянно встречаем классы, которые делают слишком много всего. Почему?
Заставить программу работать и написать чистый код — совершенно разные вещи. Обычно мы думаем прежде всего о том, чтобы наш код заработал, а не о его структуре и чистоте. И это абсолютно законно. Разделение ответственности в работе программиста играет не менее важную роль, чем в наших программах.
К сожалению, слишком многие из нас полагают, что после того, как программа заработает, их работа закончена. Мы не переключаемся на усовершенствование ее структуры и чистоты. Мы переходим к следующей задаче вместо того, чтобы сделать шаг назад и разделить разбухшие классы на отдельные блоки с единой ответственностью.
В то же время многие разработчики опасаются, что множество небольших узкоспециализированных классов затруднит понимание общей картины. Их беспокоит то, что им придется переходить от класса к классу, чтобы разобраться в том, как решается более крупная задача.
Однако система с множеством малых классов имеет не больше «подвижных частей», чем система с несколькими большими классами. В последней тоже придется разбираться, и это будет ничуть не проще. Так что вопрос заключается в следующем: хотите ли вы, чтобы ваши инструменты были разложены по ящикам с множеством небольших отделений, содержащих четко определенные и подписанные компоненты? Или вы предпочитаете несколько больших ящиков, в которые можно сваливать все подряд?
Каждая крупная система содержит большой объем рабочей логики и обладает высокой сложностью. Первоочередной целью управления этой сложностью является формирование структуры, при которой разработчик знает, где искать то, что ему требуется, и в любой момент времени может досконально знать только ту часть системы, которая непосредственно относится к его работе. Напротив, в системе с большими, многоцелевыми классами нам неизбежно приходится разбираться с множеством аспектов, которые в данный момент нас не интересуют.
Еще раз выделю основные моменты: система должна состоять из множества мелких классов, а не из небольшого числа больших. Каждый класс инкапсулирует одну ответственность, имеет одну причину для изменения и взаимодействует с другими классами для реализации желаемого поведения системы.
public class Stack {
private int topOfStack = 0;
List<Integer> elements = new LinkedList<Integer>();
public int size() {
return topOfStack;
}
public void push(int element) {
topOfStack++;
elements.add(element);
}
public int pop() throws PoppedWhenEmpty {
if (topOfStack == 0)
throw new PoppedWhenEmpty();
int element = elements.get(--topOfStack);
elements.remove(topOfStack);
return element;
}
}
Стратегия компактных функций и коротких списков параметров иногда приводит к росту переменных экземпляров, используемых подмножеством методов. Это почти всегда свидетельствует о том, что по крайней мере один класс пытается выделиться из более крупного класса. Постарайтесь разделить переменные и методы на два и более класса, чтобы новые классы обладали более высокой связностью.
package literatePrimes;
public class PrintPrimes {
public static void main(String[] args) {
final int M = 1000;
final int RR = 50;
final int CC = 4;
final int WW = 10;
final int ORDMAX = 30;
int P[] = new int[M + 1];
int PAGENUMBER;
int PAGEOFFSET;
int ROWOFFSET;
int C;
int J;
int K;
boolean JPRIME;
int ORD;
int SQUARE;
int N;
int MULT[] = new int[ORDMAX + 1];
J = 1;
K = 1;
P[1] = 2;
ORD = 2;
SQUARE = 9;
while (K < M) {
do {
J = J + 2;
if (J == SQUARE) {
ORD = ORD + 1;
SQUARE = P[ORD] * P[ORD];
MULT[ORD - 1] = J;
}
N = 2;
JPRIME = true;
while (N < ORD && JPRIME) {
while (MULT[N] < J)
MULT[N] = MULT[N] + P[N] + P[N];
if (MULT[N] == J)
JPRIME = false;
N = N + 1;
}
} while (!JPRIME);
K = K + 1;
P[K] = J;
}
{
PAGENUMBER = 1;
PAGEOFFSET = 1;
while (PAGEOFFSET <= M) {
System.out.println("The First " + M +
" Prime Numbers --- Page " + PAGENUMBER);
System.out.println("");
for (ROWOFFSET = PAGEOFFSET; ROWOFFSET < PAGEOFFSET + RR; ROWOFFSET++){
for (C = 0; C < CC;C++)
if (ROWOFFSET + C * RR <= M)
System.out.format("%10d", P[ROWOFFSET + C * RR]);
System.out.println("");
}
System.out.println("\f");
PAGENUMBER = PAGENUMBER + 1;
PAGEOFFSET = PAGEOFFSET + RR * CC;
}
}
}
}
Записанная в виде одной функции, эта программа представляет собой полную неразбериху. Многоуровневая вложенность, множество странных переменных, структура с жесткой привязкой… По крайней мере, одну большую функцию следует разбить на несколько меньших функций.
В листингах 10.6–10.8 показано, что получается после разбиения кода из листинга 10.5 на меньшие классы и функции, с выбором осмысленных имен для классов, функций и переменных.
package literatePrimes;
public class PrimePrinter {
public static void main(String[] args) {
final int NUMBER_OF_PRIMES = 1000;
int[] primes = PrimeGenerator.generate(NUMBER_OF_PRIMES);
final int ROWS_PER_PAGE = 50;
final int COLUMNS_PER_PAGE = 4;
RowColumnPagePrinter tablePrinter =
new RowColumnPagePrinter(ROWS_PER_PAGE,
COLUMNS_PER_PAGE,
"The First " + NUMBER_OF_PRIMES +
" Prime Numbers");
tablePrinter.print(primes);
}
}
package literatePrimes;
import java.io.PrintStream;
public class RowColumnPagePrinter {
private int rowsPerPage;
private int columnsPerPage;
private int numbersPerPage;
private String pageHeader;
private PrintStream printStream;
public RowColumnPagePrinter(int rowsPerPage,
int columnsPerPage,
String pageHeader) {
this.rowsPerPage = rowsPerPage;
this.columnsPerPage = columnsPerPage;
this.pageHeader = pageHeader;
numbersPerPage = rowsPerPage * columnsPerPage;
printStream = System.out;
}
public void print(int data[]) {
int pageNumber = 1;
for (int firstIndexOnPage = 0;
firstIndexOnPage < data.length;
firstIndexOnPage += numbersPerPage) {
int lastIndexOnPage =
Math.min(firstIndexOnPage + numbersPerPage - 1,
data.length - 1);
printPageHeader(pageHeader, pageNumber);
printPage(firstIndexOnPage, lastIndexOnPage, data);
printStream.println("\f");
pageNumber++;
}
}
private void printPage(int firstIndexOnPage,
int lastIndexOnPage,
int[] data) {
int firstIndexOfLastRowOnPage =
firstIndexOnPage + rowsPerPage - 1;
for (int firstIndexInRow = firstIndexOnPage;
firstIndexInRow <= firstIndexOfLastRowOnPage;
firstIndexInRow++) {
printRow(firstIndexInRow, lastIndexOnPage, data);
printStream.println("");
}
}
private void printRow(int firstIndexInRow,
int lastIndexOnPage,
int[] data) {
for (int column = 0; column < columnsPerPage; column++) {
int index = firstIndexInRow + column * rowsPerPage;
if (index <= lastIndexOnPage)
printStream.format("%10d", data[index]);
}
}
private void printPageHeader(String pageHeader,
int pageNumber) {
printStream.println(pageHeader + " --- Page " + pageNumber);
printStream.println("");
}
public void setOutput(PrintStream printStream) {
this.printStream = printStream;
}
}
package literatePrimes;
import java.util.ArrayList;
public class PrimeGenerator {
private static int[] primes;
private static ArrayList<Integer> multiplesOfPrimeFactors;
protected static int[] generate(int n) {
primes = new int[n];
multiplesOfPrimeFactors = new ArrayList<Integer>();
set2AsFirstPrime();
checkOddNumbersForSubsequentPrimes();
return primes;
}
private static void set2AsFirstPrime() {
primes[0] = 2;
multiplesOfPrimeFactors.add(2);
}
private static void checkOddNumbersForSubsequentPrimes() {
int primeIndex = 1;
for (int candidate = 3;
primeIndex < primes.length;
candidate += 2) {
if (isPrime(candidate))
primes[primeIndex++] = candidate;
}
}
private static boolean isPrime(int candidate) {
if (isLeastRelevantMultipleOfNextLargerPrimeFactor(candidate)) {
multiplesOfPrimeFactors.add(candidate);
return false;
}
return isNotMultipleOfAnyPreviousPrimeFactor(candidate);
}
private static boolean
isLeastRelevantMultipleOfNextLargerPrimeFactor(int candidate) {
int nextLargerPrimeFactor = primes[multiplesOfPrimeFactors.size()];
int leastRelevantMultiple = nextLargerPrimeFactor * nextLargerPrimeFactor;
return candidate == leastRelevantMultiple;
}
private static boolean
isNotMultipleOfAnyPreviousPrimeFactor(int candidate) {
for (int n = 1; n < multiplesOfPrimeFactors.size(); n++) {
if (isMultipleOfNthPrimeFactor(candidate, n))
return false;
}
return true;
}
private static boolean
isMultipleOfNthPrimeFactor(int candidate, int n) {
return
candidate == smallestOddNthMultipleNotLessThanCandidate(candidate, n);
}
private static int
smallestOddNthMultipleNotLessThanCandidate(int candidate, int n) {
int multiple = multiplesOfPrimeFactors.get(n);
while (multiple < candidate)
multiple += 2 * primes[n];
multiplesOfPrimeFactors.set(n, multiple);
return multiple;
}
}
Прежде всего бросается в глаза, что программа стала значительно длиннее. От одной с небольшим страницы она разрослась почти до трех страниц. Это объясняется несколькими причинами. Во-первых, в переработанной программе используются более длинные, более содержательные имена переменных. Во-вторых, объявления функций и классов в переработанной версии используются для комментирования кода. В третьих, пробелы и дополнительное форматирование обеспечивают удобочитаемость программы.
Обратите внимание на логическое разбиение программы в соответствии с тремя основными видами ответственности. Основной код программы содержится в классе PrimePrinter; он отвечает за управлении средой выполнения. Именно этот код изменится в случае смены механизма вызова. Например, если в будущем программа будет преобразована в службу SOAP, то изменения будут внесены в код PrimePrinter.
Класс RowColumnPagePrinter специализируется на форматировании списка чисел в страницы с определенным количеством строк и столбцов. Если потребуется изменить формат вывода, то изменения затронут только этот класс.
Класс PrimeGenerator специализируется на построении списка простых чисел. Создание экземпляров этого класса не предполагается. Класс всего лишь определяет удобную область видимости, в которой можно объявлять и скрывать переменные. Он изменится при изменении алгоритма вычисления простых чисел.
При этом программа не была переписана! Мы не начинали работу «с нуля» и не писали код заново. В самом деле, внимательно присмотревшись к двум программам, вы увидите, что они используют одинаковые алгоритмы и одинаковую механику для решения своих задач.
Модификация началась с написания тестового пакета, досконально проверявшего поведение первой программы. Далее в код последовательно вносились многочисленные мелкие изменения. После каждого изменения проводились тесты, которые подтверждали, что поведение программы не изменилось. Так, шаг за шагом, первая программа очищалась и трансформировалась во вторую.
public class Sql {
public Sql(String table, Column[] columns)
public String create()
public String insert(Object[] fields)
public String selectAll()
public String findByKey(String keyColumn, String keyValue)
public String select(Column column, String pattern)
public String select(Criteria criteria)
public String preparedInsert()
private String columnList(Column[] columns)
private String valuesList(Object[] fields, final Column[] columns)
private String selectWithCriteria(String criteria)
private String placeholderList(Column[] columns)
}
Класс Sql изменяется при добавлении нового типа команды. Кроме того, он будет изменяться при изменении подробностей реализации уже существующего типа команды — скажем, если нам понадобится изменить функциональность select для поддержки подчиненной выборки. Две причины для изменения означают, что класс Sql нарушает принцип единой ответственности.
Нарушение принципа единой ответственности проявляется и в структуре кода. Из набора методов Sql видно, что класс содержит приватные методы (например, selectWithCriteria), относящиеся только к командам select.
Приватные методы, действие которых распространяется только на небольшое подмножество класса, — хороший признак для поиска потенциальных усовершенствований. Тем не менее основные усилия следует направить на изменение самой системы. Если бы класс Sql выглядел логически завершенным, то нам не пришлось бы беспокоиться о разделении ответственности. Если бы в обозримом будущем функциональность update не понадобилась, Sql можно было бы оставить в покое. Но как только выясняется, что класс необходимо открыть, нужно рассмотреть возможность усовершенствования его структуры.
Почему бы не воспользоваться решением, представленным в листинге 10.10? Для каждого метода открытого интерфейса, определенного в предыдущей версии Sql из листинга 10.9, создается соответствующий класс, производный от Sql. При этом приватные методы (такие, как valuesList) перемещаются непосредственно туда, где они понадобятся. Общее приватное поведение изолируется в паре вспомогательных классов, Where и ColumnList.
abstract public class Sql {
public Sql(String table, Column[] columns)
abstract public String generate();
}
public class CreateSql extends Sql {
public CreateSql(String table, Column[] columns)
@Override public String generate()
}
public class SelectSql extends Sql {
public SelectSql(String table, Column[] columns)
@Override public String generate()
}
public class InsertSql extends Sql {
public InsertSql(String table, Column[] columns, Object[] fields)
@Override public String generate()
private String valuesList(Object[] fields, final Column[] columns)
}
public class SelectWithCriteriaSql extends Sql {
public SelectWithCriteriaSql(
String table, Column[] columns, Criteria criteria)
@Override public String generate()
}
public class SelectWithMatchSql extends Sql {
public SelectWithMatchSql(
String table, Column[] columns, Column column, String pattern)
@Override public String generate()
}
public class FindByKeySql extends Sql
public FindByKeySql(
String table, Column[] columns, String keyColumn, String keyValue)
@Override public String generate()
}
public class PreparedInsertSql extends Sql {
public PreparedInsertSql(String table, Column[] columns)
@Override public String generate() {
private String placeholderList(Column[] columns)
}
public class Where {
public Where(String criteria)
public String generate()
}
Код каждого класса становится до смешного простым. Время, необходимое для понимания класса, падает почти до нуля. Вероятность того, что одна из функций нарушит работу другой, ничтожно мала. С точки зрения тестирования проверка всех фрагментов логики в этом решении упрощается, поскольку все классы изолированы друг от друга.
Что не менее важно, когда придет время добавления update, вам не придется изменять ни один из существующих классов! Логика построения команды update реализуется в новом субклассе Sql с именем UpdateSql. Это изменение не нарушит работу другого кода в системе.
Переработанная логика Sql положительна во всех отношениях. Она поддерживает принцип единой ответственности. Она также поддерживает другой ключевой принцип проектирования классов в ООП, называемый принципом открытости/закрытости [PPP]: классы должны быть открыты для расширений, но закрыты для модификации. Наш переработанный класс Sql открыт для добавления новой функциональности посредством создания производных классов, но при внесении этого изменения все остальные классы остаются закрытыми. Новый класс UpdateSql просто размещается в положенном месте.
Структура системы должна быть такой, чтобы обновление системы (с добавлением новых или изменением существующих аспектов) создавало как можно меньше проблем. В идеале новая функциональность должна реализовываться расширением системы, а не внесением изменений в существующий код.
public interface StockExchange {
Money currentPrice(String symbol);
}
Класс TokyoStockExchange проектируется с расчетом на реализацию этого интерфейса. При ссылке на StockExchange передается в аргументе конструктора Portfolio:
public Portfolio {
private StockExchange exchange;
public Portfolio(StockExchange exchange) {
this.exchange = exchange;
}
// ...
}
Теперь наш тест может создать пригодную для тестирования реализацию интерфейса StockExchange, эмулирующую реальный API TokyoStockExchange. Тестовая реализация задает текущую стоимость каждого вида акций, используемых при тестировании. Если тест демонстрирует приобретение пяти акций Microsoft, мы кодируем тестовую реализацию так, чтобы для Microsoft всегда возвращалась стоимость $100 за акцию. Тестовая реализация интерфейса StockExchange сводится к простому поиску по таблице. После этого пишется тест, который должен вернуть общую стоимость портфеля в $500:
public class PortfolioTest {
private FixedStockExchangeStub exchange;
private Portfolio portfolio;
@Before
protected void setUp() throws Exception {
exchange = new FixedStockExchangeStub();
exchange.fix("MSFT", 100);
portfolio = new Portfolio(exchange);
}
@Test
public void GivenFiveMSFTTotalShouldBe500() throws Exception {
portfolio.add(5, "MSFT");
Assert.assertEquals(500, portfolio.value());
}
}
Если система обладает достаточной логической изоляцией для подобного тестирования, она также становится более гибкой и более подходящей для повторного использования. Отсутствие жестких привязок означает, что элементы системы лучше изолируются друг от друга и от изменений. Изоляция упрощает понимание каждого элемента системы.
Сведение к минимуму логических привязок соответствует другому принципу проектирования классов, известному как принцип обращения зависимостей (DIP, Dependency Inversion Principle). По сути DIP гласит, что классы системы должны зависеть от абстракций, а не от конкретных подробностей.
Вместо того чтобы зависеть от подробностей реализации класса TokyoStockExchange, наш класс Portfolio теперь зависит от интерфейса StockExchange. Интерфейс StockExchange представляет абстрактную концепцию запроса текущей стоимости акций. Эта абстракция изолирует класс от конкретных подробностей получения такой цены — в том числе и от источника, из которого берется реальная информация.
Кевин Дин Уомплер
Сложность убивает. Она вытягивает жизненные силы из разработчиков, затрудняя планирование, построение и тестирование продуктов.Рэй Оззи, технический директор Microsoft Corporation
public Service getService() {
if (service == null)
service = new MyServiceImpl(...); // Инициализация по умолчанию,
// подходящая для большинства случаев?
return service;
}
Идиома ОТЛОЖЕННОЙ ИНИЦИАЛИЗАЦИИ обладает определенными достоинствами. Приложение не тратит времени на конструирование объекта до момента его фактического использования, а это может ускорить процесс инициализации. Кроме того, мы следим за тем, чтобы функция никогда не возвращала null.
Однако в программе появляется жестко закодированная зависимость от класса MyServiceImpl и всего, что необходимо для его конструктора (который я не привел). Программа не компилируется без разрешения этих зависимостей, даже если объект этого типа ни разу не используется во время выполнения!
Проблемы могут возникнуть и при тестировании. Если MyServiceImpl представляет собой тяжеловесный объект, нам придется позаботиться о том, чтобы перед вызовом метода в ходе модульного тестирования в поле service был сохранен соответствующий ТЕСТОВЫЙ ДУБЛЕР [Mezzaros07] или ФИКТИВНЫЙ ОБЪЕКТ. А поскольку логика конструирования смешана с логикой нормальной обработки, мы должны протестировать все пути выполнения (в частности, проверку null и ее блок). Наличие обеих обязанностей означает, что метод выполняет более одной операции, а это указывает на некоторое нарушение принципа единой ответственности.
Но хуже всего другое — мы не знаем, является ли MyServiceImpl правильным объектом во всех случаях. Я намекнул на это в комментарии. Почему класс с этим методом должен знать глобальный контекст? Можем ли мы вообще определить, какой объект должен здесь использоваться? И вообще, может ли один тип быть подходящим для всех возможных контекстов?
Конечно, одно вхождение ОТЛОЖЕННОЙ ИНИЦИАЛИЗАЦИИ не создает серьезных проблем. Однако в приложениях идиомы инициализации обычно встречаются во множество экземпляров. Таким образом, глобальная стратегия инициализации (если она здесь вообще присутствует) распределяется по всему приложению, с минимальной модульностью и значительным дублированием кода.
Если вы действительно стремитесь к созданию хорошо структурированных, надежных систем, никогда не допускайте, чтобы удобные идиомы вели к нарушению модульности. Процесс конструирования объектов и установления связей не является исключением. Этот процесс должен быть отделен от нормальной логики времени выполнения, а вы должны позаботиться о выработке глобальной, последовательной стратегии разрешения основных зависимостей.
MyService myService = (MyService)(jndiContext.lookup("NameOfMyService"));
Вызывающий объект не управляет тем, какой именно объект будет возвращен (конечно, при условии, что этот объект реализует положенный интерфейс), но при этом происходит активное разрешение зависимости.
Истинное внедрение зависимостей идет еще на один шаг вперед. Класс не предпринимает непосредственных действий по разрешению своих зависимостей; он остается абсолютно пассивным. Вместо этого он предоставляет set-методы и/или аргументы конструктора, используемые для внедрения зависимостей. В процессе конструирования контейнер DI создает экземпляры необходимых объектов (обычно по требованию) и использует аргументы конструктора или set-методы для скрепления зависимостей. Фактически используемые зависимые объекты задаются в конфигурационном файле или на программном уровне в специализированном конструирующем модуле.
Самый известный DI-контейнер для Java присутствует в Spring Framework[35]. Подключаемые объекты перечисляются в конфигурационном файле XML, после чего конкретный объект запрашивается по имени в коде Java. Пример будет рассмотрен ниже.
Но как же преимущества ОТЛОЖЕННОЙ ИНИЦИАЛИЗАЦИИ? Эта идиома иногда бывает полезной и при внедрении зависимостей. Во-первых, большинство DI-контейнеров не конструирует объекты до того момента, когда это станет необходимо. Во-вторых, многие из этих контейнеров предоставляют механизмы использования фабрик или конструирования посредников (proxies), которые могут использоваться для ОТЛОЖЕННОЙ ИНИЦИАЛИЗАЦИИ и других аналогичных оптимизаций[36].
В этом проявляется важнейшее отличие программных систем от физических. Архитектура программных систем может развиваться последовательно, если обеспечить правильное разделение ответственности.Как вы вскоре убедитесь, нематериальная природа программных систем делает это возможным. Но давайте начнем с контрпримера архитектуры, в которой нормальное разделение ответственности отсутствует. Исходные архитектуры EJB1 и EJB2 не обеспечивали должного разделения областей ответственности и поэтому создавали лишние барьеры для естественного роста. Возьмем хотя бы компонент-сущность (Entity Bean) для постоянного (persistent) класса. Компонентом-сущностью называется представление реляционных данных (иначе говоря, записи таблицы) в памяти. Для начала необходимо определить локальный (внутрипроцессный) или удаленный (на отдельной JVM) интерфейс, который будет использоваться клиентами. Возможный локальный интерфейс представлен в листинге 11.1.
package com.example.banking;
import java.util.Collections;
import javax.ejb.*;
public interface BankLocal extends java.ejb.EJBLocalObject {
String getStreetAddr1() throws EJBException;
String getStreetAddr2() throws EJBException;
String getCity() throws EJBException;
String getState() throws EJBException;
String getZipCode() throws EJBException;
void setStreetAddr1(String street1) throws EJBException;
void setStreetAddr2(String street2) throws EJBException;
void setCity(String city) throws EJBException;
void setState(String state) throws EJBException;
void setZipCode(String zip) throws EJBException;
Collection getAccounts() throws EJBException;
void setAccounts(Collection accounts) throws EJBException;
void addAccount(AccountDTO accountDTO) throws EJBException;
}
В интерфейс включены некоторые атрибуты адреса Bank, а также коллекция счетов, принадлежащих банку; данные каждого счета представляются отдельным EJB Account. В листинге 11.2 приведен соответствующий класс реализации компонента Bank.
package com.example.banking;
import java.util.Collections;
import javax.ejb.*;
public abstract class Bank implements javax.ejb.EntityBean {
// Бизнес-логика...
public abstract String getStreetAddr1();
public abstract String getStreetAddr2();
public abstract String getCity();
public abstract String getState();
public abstract String getZipCode();
public abstract void setStreetAddr1(String street1);
public abstract void setStreetAddr2(String street2);
public abstract void setCity(String city);
public abstract void setState(String state);
public abstract void setZipCode(String zip);
public abstract Collection getAccounts();
public abstract void setAccounts(Collection accounts);
public void addAccount(AccountDTO accountDTO) {
InitialContext context = new InitialContext();
AccountHomeLocal accountHome = context.lookup("AccountHomeLocal");
AccountLocal account = accountHome.create(accountDTO);
Collection accounts = getAccounts();
accounts.add(account);
}
// Логика контейнера EJB
public abstract void setId(Integer id);
public abstract Integer getId();
public Integer ejbCreate(Integer id) { ... }
public void ejbPostCreate(Integer id) { ... }
// Остальные методы должны быть реализованы, но обычно остаются пустыми:
public void setEntityContext(EntityContext ctx) {}
public void unsetEntityContext() {}
public void ejbActivate() {}
public void ejbPassivate() {}
public void ejbLoad() {}
public void ejbStore() {}
public void ejbRemove() {}
}
В листинге не приведен ни соответствующий интерфейс LocalHome (по сути — фабрика, используемая для создания объектов), ни один из возможных методов поиска Bank, которые вы можете добавить.
Наконец, вы должны написать один или несколько дескрипторов в формате XML, которые определяют подробности соответствия между объектом и реляционными данными, желаемое транзакционное поведение, ограничения безопасности и т.д.
Бизнес-логика тесно привязана к «контейнеру» приложения EJB2. Вы должны субклассировать контейнерные типы, а также предоставить многие методы жизненного цикла, необходимые для контейнера.
Привязка к тяжеловесному контейнеру затрудняет изолированное модульное тестирование. Приходится либо имитировать контейнер, что непросто, либо тратить много времени на развертывание EJB и тестов на реальном сервере. Повторное использование за пределами архитектуры EJB2 практически невозможно из-за жесткой привязки.
Наконец, такое решение противоречит принципам объектно-ориентированного программирования. Один компонент не может наследовать от другого компонента. Обратите внимание на логику добавления нового счета. В EJB2 компоненты часто определяют «объекты передачи данных» (DTO), которые фактически представляют собой «структуры без поведения». Обычно это приводит к появлению избыточных типов, содержащих по сути одинаковые данные, и необходимости использования стереотипного кода для копирования данных между объектами.
// Bank.java (подавление имен пакетов...)
import java.utils.*;
// Абстрактное представление банка.
public interface Bank {
Collection<Account> getAccounts();
void setAccounts(Collection<Account> accounts);
}
// BankImpl.java
import java.utils.*;
// POJO-объект ("Plain Old Java Object"), реализующий абстракцию.
public class BankImpl implements Bank {
private List<Account> accounts;
public Collection<Account> getAccounts() {
return accounts;
}
public void setAccounts(Collection<Account> accounts) {
this.accounts = new ArrayList<Account>();
for (Account account: accounts) {
this.accounts.add(account);
}
}
}
// BankProxyHandler.java
import java.lang.reflect.*;
import java.util.*;
// Реализация InvocationHandler, необходимая для API посредника.
public class BankProxyHandler implements InvocationHandler {
private Bank bank;
public BankHandler (Bank bank) {
this.bank = bank;
}
// Метод, определенный в InvocationHandler
public Object invoke(Object proxy, Method method, Object[] args)
throws Throwable {
String methodName = method.getName();
if (methodName.equals("getAccounts")) {
bank.setAccounts(getAccountsFromDatabase());
return bank.getAccounts();
} else if (methodName.equals("setAccounts")) {
bank.setAccounts((Collection<Account>) args[0]);
setAccountsToDatabase(bank.getAccounts());
return null;
} else {
...
}
}
// Подробности:
protected Collection<Account> getAccountsFromDatabase() { ... }
protected void setAccountsToDatabase(Collection<Account> accounts) { ... }
}
// В другом месте...
Bank bank = (Bank) Proxy.newProxyInstance(
Bank.class.getClassLoader(),
new Class[] { Bank.class },
new BankProxyHandler(new BankImpl()));
Мы определили интерфейс Bank, который будет инкапсулироваться посредником, и POJO-объект («Plain Old Java Object», то есть «обычный Java-объект») BankImpl, реализующий бизнес-логику. (Вскоре мы вернемся к теме POJO-объектов).
Для работы посредника необходим объект InvocationHandler, который вызывается для реализации всех вызовов методов Bank, обращенных к посреднику. Наша реализация BankProxyHandler использует механизм рефлексии Java для отображения вызовов обобщенных методов на соответствующие методы BankImpl.
Код получается весьма объемистым и относительно сложным, даже в этом простом случае[41]. Не меньше проблем создает и использование библиотек для манипуляций с байт-кодом. Объем и сложность кода — два основных недостатка посредников. Эти два фактора усложняют создание чистого кода! Кроме того, у посредников не существует механизма определения «точек интереса» общесистемного уровня, необходимых для полноценного АОП-решения[42].
<beans>
...
<bean id="appDataSource"
class="org.apache.commons.dbcp.BasicDataSource"
destroy-method="close"
p:driverClassName="com.mysql.jdbc.Driver"
p:url="jdbc:mysql://localhost:3306/mydb"
p:username="me"/>
<bean id="bankDataAccessObject"
class="com.example.banking.persistence.BankDataAccessObject"
p:dataSource-ref="appDataSource"/>
<bean id="bank"
class="com.example.banking.model.Bank"
p:dataAccessObject-ref="bankDataAccessObject"/>
...
</beans>
Каждый компонент напоминает одну из частей русской «матрешки»: объект предметной области Bank «упаковывается» в объект доступа к данным DAO (Data Accessor Object), который, в свою очередь, упаковывается в объект источника данных JDBC (рис. 11.3).
XmlBeanFactory bf =
new XmlBeanFactory(new ClassPathResource("app.xml", getClass()));
Bank bank = (Bank) bf.getBean("bank");
Так как объем кода, специфического для Spring, минимален, приложение почти полностью изолировано от Spring. Тем самым устраняются все проблемы жесткой привязки, характерные для таких систем, как EJB2.
Хотя код XML занимает много места и плохо читается[45], определяемая в этих конфигурационных файлах «политика» все же проще сложной логики посредников и аспектов, скрытой от наших глаз и создаваемой автоматически. Архитектура выглядит настолько заманчиво, что инфраструктуры вроде Spring привели к полной переработке стандарта EJB для версии 3. EJB3 в значительной мере следует характерной для Spring модели декларативной поддержки поперечных областей ответственности с использованием конфигурационных файлов XML и/или аннотаций Java 5.
В листинге 11.5 приведен объект Bank, переписанный для EJB3[46].
package com.example.banking.model;
import javax.persistence.*;
import java.util.ArrayList;
import java.util.Collection;
@Entity
@Table(name = "BANKS")
public class Bank implements java.io.Serializable {
@Id @GeneratedValue(strategy=GenerationType.AUTO)
private int id;
@Embeddable // Объект "встраивается" в запись базы данных Bank
public class Address {
protected String streetAddr1;
protected String streetAddr2;
protected String city;
protected String state;
protected String zipCode;
}
@Embedded
private Address address;
@OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER,
mappedBy="bank")
private Collection<Account> accounts = new ArrayList<Account>();
public int getId() {
return id;
}
public void setId(int id) {
this.id = id;
}
public void addAccount(Account account) {
account.setBank(this);
accounts.add(account);
}
public Collection<Account> getAccounts() {
return accounts;
}
public void setAccounts(Collection<Account> accounts) {
this.accounts = accounts;
}
}
Этот вариант кода намного чище исходного кода EJB2. Некоторые подробности о сущностях все еще присутствуют в аннотациях. Тем не менее, поскольку эта информация не выходит за пределы аннотаций, код остается чистым, понятным, а следовательно, простым в тестировании, сопровождении и т.д.
Часть информации о сохранении объектов, содержащейся в аннотациях, можно при желании переместить в дескрипторы XML, оставив действительно чистый POJO-объект. Если детали сохранения объектов изменяются относительно редко, многие группы отдадут предпочтение аннотациям, но с гораздо меньшими отрицательными последствиями по сравнению с EJB2.
Оптимальная архитектура системы состоит из модульных областей ответственности, каждая из которых реализуется на базе POJO-объектов. Области интегрируются между собой при помощи аспектов или аналогичных средств, минимальным образом вмешивающихся в их работу. Такая архитектура может строиться на базе методологии разработки через тестирование, как и программный код.
Гибкость POJO-системы с модульными областями ответственности позволяет принимать оптимальные, своевременные решения на базе новейшей информации. Кроме того, она способствует снижению сложности таких решений.
Стандарты упрощают повторное использование идей и компонентов, привлечение людей с необходимым опытом, воплощение удачных идей и связывание компонентов. Тем не менее, процесс создания стандарта иногда занимает слишком много времени (а отрасль не стоит на месте), в результате чего стандарты теряют связь с реальными потребностями тех людей, которым они должны служить.
Предметно-ориентированные языки позволяют выразить в форме POJO-объектов все уровни абстракции и все предметные области приложения, от высокоуровневых политик до низкоуровневых технических подробностей.
Джефф Лангр
int size() {}
boolean isEmpty() {}
Методы могут иметь разные реализации. Допустим, метод isEmpty может использовать логический флаг, а size — счетчик элементов. Однако мы можем устранить дублирование, связав isEmpty с определением size:
boolean isEmpty() {
return 0 == size();
}
Чтобы создать чистую систему, необходимо сознательно стремиться к устранению дубликатов, пусть даже всего в нескольких строках кода. Для примера рассмотрим следующий код:
public void scaleToOneDimension(
float desiredDimension, float imageDimension) {
if (Math.abs(desiredDimension - imageDimension) < errorThreshold)
return;
float scalingFactor = desiredDimension / imageDimension;
scalingFactor = (float)(Math.floor(scalingFactor * 100) * 0.01f);
RenderedOp newImage = ImageUtilities.getScaledImage(
image, scalingFactor, scalingFactor);
image.dispose();
System.gc();
image = newImage;
}
public synchronized void rotate(int degrees) {
RenderedOp newImage = ImageUtilities.getRotatedImage(
image, degrees);
image.dispose();
System.gc();
image = newImage;
}
Чтобы обеспечить чистоту системы, следует устранить незначительное дублирование между методами scaleToOneDimension и rotate:
public void scaleToOneDimension(
float desiredDimension, float imageDimension) {
if (Math.abs(desiredDimension - imageDimension) < errorThreshold)
return;
float scalingFactor = desiredDimension / imageDimension;
scalingFactor = (float)(Math.floor(scalingFactor * 100) * 0.01f);
replaceImage(ImageUtilities.getScaledImage(
image, scalingFactor, scalingFactor));
}
public synchronized void rotate(int degrees) {
replaceImage(ImageUtilities.getRotatedImage(image, degrees));
}
private void replaceImage(RenderedOp newImage) {
image.dispose();
System.gc();
image = newImage;
}
В ходе выделения общности конструкций на этом микроскопическом уровне начинают проявляться нарушения принципа SRP. Таким образом, только что сформированный метод можно переместить в другой класс. Это расширяет видимость метода. Другой участник группы может найти возможность дальнейшего абстрагирования нового метода и его использования в другом контексте. Таким образом, принцип «повторного использования даже в мелочах» может привести к значительному сокращению сложности системы. Понимание того, как обеспечить повторное использование в мелочах, абсолютно необходимо для его обеспечения в большом масштабе.
Паттерн ШАБЛОННЫЙ МЕТОД [GOF] относится к числу стандартных приемов устранения высокоуровневого дублирования. Пример:
public class VacationPolicy {
public void accrueUSDivisionVacation() {
// Код вычисления продолжительности отпуска
// по количеству отработанных часов
// ...
// Код проверки минимальной продолжительности отпуска
// по стандартам США
// ...
// Код внесения отпуска в платежную ведомость
// ...
}
public void accrueEUDivisionVacation() {
// Код вычисления продолжительности отпуска
// по количеству отработанных часов
// ...
// Код проверки минимальной продолжительности отпуска
// по европейским стандартам
// ...
// Код внесения отпуска в платежную ведомость
// ...
}
}
Код accrueUSDivisionVacation и accrueEuropeanDivisionVacation в основном совпадает, если не считать проверки минимальной продолжительности. Этот фрагмент алгоритма изменяется в зависимости от типа работника.
Для устранения этого очевидного дублирования можно воспользоваться паттерном ШАБЛОННЫЙ МЕТОД:
abstract public class VacationPolicy {
public void accrueVacation() {
calculateBaseVacationHours();
alterForLegalMinimums();
applyToPayroll();
}
private void calculateBaseVacationHours() { /* ... */ };
abstract protected void alterForLegalMinimums();
private void applyToPayroll() { /* ... */ };
}
public class USVacationPolicy extends VacationPolicy {
@Override protected void alterForLegalMinimums() {
// Логика для США
}
}
public class EUVacationPolicy extends VacationPolicy {
@Override protected void alterForLegalMinimums() {
// Логика для Европы
}
}
Субклассы «заполняют пробел» в обобщенном алгоритме accrueVacation; они предоставляют только ту информацию, которая различается в специализированных версиях алгоритма.
Бретт Л. Шухерт
Объекты — абстракции для обработки данных. Программные потоки — абстракции для планирования.Джеймс О. Коплиен
public class X {
private int lastIdUsed;
public int getNextId() {
return ++lastIdUsed;
}
}
Допустим, мы создаем экземпляр X, присваиваем полю lastIdUsed значение 42, а затем используем созданный экземпляр в двух программных потоках. В обоих потоках вызывается метод getNextId(); возможны три исхода:
• Первый поток получает значение 43, второй получает значение 44, в поле lastIdUsed сохраняется 44.
• Первый поток получает значение 44, второй получает значение 43, в поле lastIdUsed сохраняется 44.
• Первый поток получает значение 43, второй получает значение 43, поле lastIdUsed содержит 43.
Удивительный третий результат[54] встречается тогда, когда два потока «перебивают» друг друга. Это происходит из-за того, что выполнение одной строки кода Java в двух потоках может пойти по разным путям, и некоторые из этих путей порождают неверные результаты. Сколько существует разных путей? Чтобы ответить на этот вопрос, необходимо понимать, как JIT-компилятор обрабатывает сгенерированный байт-код, и разбираться в том, какие операции рассматриваются моделью памяти Java как атомарные.
В двух словах скажу, что в сгенерированном байт-коде приведенного фрагмента существует 12 870 разных путей выполнения[55] метода getNextId в двух программных потоках. Если изменить тип lastIdUsed c int на long, то количество возможных путей возрастет до 2 704 156. Конечно, на большинстве путей выполнения вычисляются правильные результаты. Проблема в том, что на некоторых путях результаты будут неправильными.
ReentrantLock | Блокировка, которая может устанавливаться и освобождаться в разных методах |
Semaphore | Реализация классического семафора (блокировка со счетчиком) |
CountDownLatch | Блокировка, которая ожидает заданного количества событий до освобождения всех ожидающих потоков. Позволяет организовать более или менее одновременный запуск нескольких потоков |
Связанные ресурсы | Ресурсы с фиксированным размером или количеством, существующие в многопоточной среде, например подключения к базе данных или буферы чтения/записи |
Взаимное исключение | В любой момент времени с общими данными или с общим ресурсом может работать только один поток |
Зависание | Работа одного или нескольких потоков приостанавливается на слишком долгое время (или навсегда). Например, если высокоприоритетным потокам всегда предоставляется возможность отработать первыми, то низкоприоритетные потоки зависнут (при условии, что в системе постоянно появляются новые высокоприоритетные потоки) |
Взаимная блокировка (deadlock) | Два и более потока бесконечно ожидают завершения друг друга. Каждый поток захватил ресурс, необходимый для продолжения работы другого потока, и ни один поток не может завершиться без получения захваченного другим потоком ресурса |
Обратимая блокировка[57] (livelock) | Потоки не могут «разойтись» — каждый из потоков пытается выполнять свою работу, но обнаруживает, что другой поток стоит у него на пути. Потоки постоянно пытаются продолжить выполнение, но им это не удается в течение слишком долгого времени (или вообще не удается) |
public synchronized String nextUrlOrNull() {
if(hasNext()) {
String url = urlGenerator.next();
Thread.yield(); // Вставлено для тестирования
updateHasNext();
return url;
}
return null;
}
Добавленный вызов yield() изменяет путь выполнения кода. В результате в программе может произойти сбой там, где раньше его не было. Если работа программы действительно нарушается, то это произошло не из-за того, что вы добавили вызов yield()[66]. Просто ваш код содержал скрытые ошибки, а в результате вызова yield() они стали очевидными.
Ручная инструментовка имеет много недостатков:
• Разработчик должен каким-то образом найти подходящие места для вставки вызовов.
• Как узнать, где и какой именно вызов следует вставить?
• Если вставленные вызовы останутся в окончательной версии кода, это приведет к замедлению его работы.
• Вам приходится действовать «наобум»: вы либо находите скрытые дефекты, либо не находите их. Вообще говоря, шансы не в вашу пользу.
Отладочные вызовы должны присутствовать только на стадии тестирования, но не в окончательной версии кода. Кроме того, вам понадобятся средства для простого переключения конфигураций между запусками, повышающего вероятность обнаружения ошибок в общей кодовой базе.
Конечно, разделение системы на POJO-объекты, ничего не знающие о многопоточности, и классы, управляющие многопоточностью, упрощает поиск подходящих мест для инструментовки кода. Кроме того, такое разделение позволит нам создать целый набор «испытательных пакетов», активизирующих POJO-объекты с разными режимами вызова sleep, yield и т.д.
public class ThreadJigglePoint {
public static void jiggle() {
}
}
Вызовы этого метода размещаются в разных позициях кода:
public synchronized String nextUrlOrNull() {
if(hasNext()) {
ThreadJiglePoint.jiggle();
String url = urlGenerator.next();
ThreadJiglePoint.jiggle();
updateHasNext();
ThreadJiglePoint.jiggle();
return url;
}
return null;
}
Теперь в вашем распоряжении появился простой аспект, случайным образом выбирающий между обычным продолжением работы, приостановкой и передачей управления.
Или представьте, что класс ThreadJigglePoint имеет две реализации. В первой реализации jiggle не делает ничего; эта реализация используется в окончательной версии кода. Вторая реализация генерирует случайное число для выбора между приостановкой, передачей управления и обычным выполнением. Если теперь повторить тестирование тысячу раз со случайным выбором, возможно, вам удастся выявить некоторые дефекты. Даже если тестирование пройдет успешно, по крайней мере вы сможете сказать, что приложили должные усилия для выявления недостатков. Такой подход выглядит несколько упрощенно, но и он может оказаться разумной альтернативой для применения более сложных инструментов.
Программа ConTest[67], разработанная фирмой IBM, работает по аналогичному принципу, но предоставляет расширенные возможности.
Впрочем, суть тестирования остается неизменной: вы ломаете предсказуемость пути выполнения, чтобы при разных запусках код проходил по разным путям. Комбинация хорошо написанных тестов и случайного выбора пути может радикально повысить вероятность поиска ошибок.
Рекомендация: используйте стратегию случайного выбора пути выполнения для выявления ошибок.
Дело о разборе аргументов командной строки
public static void main(String[] args) {
try {
Args arg = new Args("l,p#,d*", args);
boolean logging = arg.getBoolean('l');
int port = arg.getInt('p');
String directory = arg.getString('d');
executeApplication(logging, port, directory);
} catch (ArgsException e) {
System.out.printf("Argument error: %s\n", e.errorMessage());
}
}
Вы и сами видите, что все действительно просто. Мы создаем экземпляр класса Args с двумя параметрами. Первый параметр задает форматную строку: "l,p#,d*.". Эта строка определяет три аргумента командной строки. Первый аргумент, –l, относится к логическому (булевскому) типу. Второй аргумент, -p, относится к целочисленному типу. Третий аргумент, -d, является строковым. Во втором параметре конструктора Args содержится массив аргументов командной строки, полученный main.
Если конструктор возвращает управление без выдачи исключения ArgsException, значит, разбор входной командной строки прошел успешно, и экземпляр Args готов к приему запросов. Методы getBoolean, getInteger, getString и т.д. используются для получения значений аргументов по именам.
При возникновении проблем (в форматной строке или в самих аргументах командной строки) инициируется исключение ArgsException. Для получения текстового описания проблемы следует вызвать метод errorMessage объекта исключения.
package com.objectmentor.utilities.args;
import static com.objectmentor.utilities.args.ArgsException.ErrorCode.*;
import java.util.*;
public class Args {
private Map<Character, ArgumentMarshaler> marshalers;
private Set<Character> argsFound;
private ListIterator<String> currentArgument;
public Args(String schema, String[] args) throws ArgsException {
marshalers = new HashMap<Character, ArgumentMarshaler>();
argsFound = new HashSet<Character>();
parseSchema(schema);
parseArgumentStrings(Arrays.asList(args));
}
private void parseSchema(String schema) throws ArgsException {
for (String element : schema.split(","))
if (element.length() > 0)
parseSchemaElement(element.trim());
}
private void parseSchemaElement(String element) throws ArgsException {
char elementId = element.charAt(0);
String elementTail = element.substring(1);
validateSchemaElementId(elementId);
if (elementTail.length() == 0)
marshalers.put(elementId, new BooleanArgumentMarshaler());
else if (elementTail.equals("*"))
marshalers.put(elementId, new StringArgumentMarshaler());
else if (elementTail.equals("#"))
marshalers.put(elementId, new IntegerArgumentMarshaler());
else if (elementTail.equals("##"))
marshalers.put(elementId, new DoubleArgumentMarshaler());
else if (elementTail.equals("[*]"))
marshalers.put(elementId, new StringArrayArgumentMarshaler());
else
throw new ArgsException(INVALID_ARGUMENT_FORMAT, elementId, elementTail);
}
private void validateSchemaElementId(char elementId) throws ArgsException {
if (!Character.isLetter(elementId))
throw new ArgsException(INVALID_ARGUMENT_NAME, elementId, null);
}
private void parseArgumentStrings(List<String> argsList) throws ArgsException
{
for (currentArgument = argsList.listIterator(); currentArgument.hasNext();)
{
String argString = currentArgument.next();
if (argString.startsWith("-")) {
parseArgumentCharacters(argString.substring(1));
} else {
currentArgument.previous();
break;
}
}
}
private void parseArgumentCharacters(String argChars) throws ArgsException {
for (int i = 0; i < argChars.length(); i++)
parseArgumentCharacter(argChars.charAt(i));
}
private void parseArgumentCharacter(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
if (m == null) {
throw new ArgsException(UNEXPECTED_ARGUMENT, argChar, null);
} else {
argsFound.add(argChar);
try {
m.set(currentArgument);
} catch (ArgsException e) {
e.setErrorArgumentId(argChar);
throw e;
}
}
}
public boolean has(char arg) {
return argsFound.contains(arg);
}
public int nextArgument() {
return currentArgument.nextIndex();
}
public boolean getBoolean(char arg) {
return BooleanArgumentMarshaler.getValue(marshalers.get(arg));
}
public String getString(char arg) {
return StringArgumentMarshaler.getValue(marshalers.get(arg));
}
public int getInt(char arg) {
return IntegerArgumentMarshaler.getValue(marshalers.get(arg));
}
public double getDouble(char arg) {
return DoubleArgumentMarshaler.getValue(marshalers.get(arg));
}
public String[] getStringArray(char arg) {
return StringArrayArgumentMarshaler.getValue(marshalers.get(arg));
}
}
Обратите внимание: код читается сверху вниз, и вам не приходится постоянно переходить туда-сюда или заглядывать вперед. Единственное место, где все же необходимо заглянуть вперед, — это определение ArgumentMarshaler, но и это было сделано намеренно. Внимательно прочитав этот код, вы поймете, что собой представляет интерфейс ArgumentMarshaler и что делают производные классы. Примеры таких классов приведены в листингах 14.3–14.6.
public interface ArgumentMarshaler {
void set(Iterator<String> currentArgument) throws ArgsException;
}
public class BooleanArgumentMarshaler implements ArgumentMarshaler {
private boolean booleanValue = false;
public void set(Iterator<String> currentArgument) throws ArgsException {
booleanValue = true;
}
public static boolean getValue(ArgumentMarshaler am) {
if (am != null && am instanceof BooleanArgumentMarshaler)
return ((BooleanArgumentMarshaler) am).booleanValue;
else
return false;
}
}
import static com.objectmentor.utilities.args.ArgsException.ErrorCode.*;
public class StringArgumentMarshaler implements ArgumentMarshaler {
private String stringValue = "";
public void set(Iterator<String> currentArgument) throws ArgsException {
try {
stringValue = currentArgument.next();
} catch (NoSuchElementException e) {
throw new ArgsException(MISSING_STRING);
}
}
public static String getValue(ArgumentMarshaler am) {
if (am != null && am instanceof StringArgumentMarshaler)
return ((StringArgumentMarshaler) am).stringValue;
else
return "";
}
}
import static com.objectmentor.utilities.args.ArgsException.ErrorCode.*;
public class IntegerArgumentMarshaler implements ArgumentMarshaler {
private int intValue = 0;
public void set(Iterator<String> currentArgument) throws ArgsException {
String parameter = null;
try {
parameter = currentArgument.next();
intValue = Integer.parseInt(parameter);
} catch (NoSuchElementException e) {
throw new ArgsException(MISSING_INTEGER);
} catch (NumberFormatException e) {
throw new ArgsException(INVALID_INTEGER, parameter);
}
}
public static int getValue(ArgumentMarshaler am) {
if (am != null && am instanceof IntegerArgumentMarshaler)
return ((IntegerArgumentMarshaler) am).intValue;
else
return 0;
}
}
Другие классы, производные от ArgumentMarshaler, строятся по тому же шаблону, что и классы для массивов double и String. Здесь они не приводятся для экономии места. Оставляю их вам для самостоятельной работы.
Возможно, вы заметили еще одно обстоятельство: где определяются константы для кодов ошибок? Они находятся в классе ArgsException (листинг 14.7).
import static com.objectmentor.utilities.args.ArgsException.ErrorCode.*;
public class ArgsException extends Exception {
private char errorArgumentId = '\0';
private String errorParameter = null;
private ErrorCode errorCode = OK;
public ArgsException() {}
public ArgsException(String message) {super(message);}
public ArgsException(ErrorCode errorCode) {
this.errorCode = errorCode;
}
public ArgsException(ErrorCode errorCode, String errorParameter) {
this.errorCode = errorCode;
this.errorParameter = errorParameter;
}
public ArgsException(ErrorCode errorCode,
char errorArgumentId, String errorParameter) {
this.errorCode = errorCode;
this.errorParameter = errorParameter;
this.errorArgumentId = errorArgumentId;
}
public char getErrorArgumentId() {
return errorArgumentId;
}
public void setErrorArgumentId(char errorArgumentId) {
this.errorArgumentId = errorArgumentId;
}
public String getErrorParameter() {
return errorParameter;
}
public void setErrorParameter(String errorParameter) {
this.errorParameter = errorParameter;
}
public ErrorCode getErrorCode() {
return errorCode;
}
public void setErrorCode(ErrorCode errorCode) {
this.errorCode = errorCode;
}
public String errorMessage() {
switch (errorCode) {
case OK:
return "TILT: Should not get here.";
case UNEXPECTED_ARGUMENT:
return String.format("Argument -%c unexpected.", errorArgumentId);
case MISSING_STRING:
return String.format("Could not find string parameter for -%c.",
errorArgumentId);
case INVALID_INTEGER:
return String.format("Argument -%c expects an integer but was '%s'.",
errorArgumentId, errorParameter);
case MISSING_INTEGER:
return String.format("Could not find integer parameter for -%c.",
errorArgumentId);
case INVALID_DOUBLE:
return String.format("Argument -%c expects a double but was '%s'.",
errorArgumentId, errorParameter);
case MISSING_DOUBLE:
return String.format("Could not find double parameter for -%c.",
errorArgumentId);
case INVALID_ARGUMENT_NAME:
return String.format("'%c' is not a valid argument name.",
errorArgumentId);
case INVALID_ARGUMENT_FORMAT:
return String.format("'%s' is not a valid argument format.",
errorParameter);
}
return "";
}
public enum ErrorCode {
OK, INVALID_ARGUMENT_FORMAT, UNEXPECTED_ARGUMENT, INVALID_ARGUMENT_NAME,
MISSING_STRING,
MISSING_INTEGER, INVALID_INTEGER,
MISSING_DOUBLE, INVALID_DOUBLE}
}
Удивительно, какой объем кода понадобился для воплощения всех подробностей этой простой концепции. Одна из причин заключается в том, что мы используем весьма «многословный» язык. Поскольку Java относится к числу языков со статической типизацией, для удовлетворения требований системы типов в нем используется немалый объем кода. На таких языках, как Ruby, Python или Smalltalk, программа получится гораздо короче[68].
Пожалуйста, перечитайте код еще раз. Обратите особое внимание на выбор имен, размеры функций и форматирование кода. Возможно, опытные программисты найдут отдельные недочеты в стиле или структуре кода. Но я надеюсь, что в целом вы согласитесь с тем, что код хорошо написан, а его структура чиста и логична.
Скажем, после чтения кода вам должно быть очевидно, как добавить поддержку нового типа аргументов (например, дат или комплексных чисел), и это потребует относительно небольших усилий с вашей стороны. Для этого достаточно создать новый класс, производный от ArgumentMarshaler, новую функцию getXXX и включить новое условие case в функцию parseSchemaElement. Вероятно, также потребуется новое значение ArgsException.ErrorCode и новое сообщение об ошибке.
import java.text.ParseException;
import java.util.*;
public class Args {
private String schema;
private String[] args;
private boolean valid = true;
private Set<Character> unexpectedArguments = new TreeSet<Character>();
private Map<Character, Boolean> booleanArgs =
new HashMap<Character, Boolean>();
private Map<Character, String> stringArgs = new HashMap<Character, String>();
private Map<Character, Integer> intArgs = new HashMap<Character, Integer>();
private Set<Character> argsFound = new HashSet<Character>();
private int currentArgument;
private char errorArgumentId = '\0';
private String errorParameter = "TILT";
private ErrorCode errorCode = ErrorCode.OK;
private enum ErrorCode {
OK, MISSING_STRING, MISSING_INTEGER, INVALID_INTEGER, UNEXPECTED_ARGUMENT}
public Args(String schema, String[] args) throws ParseException {
this.schema = schema;
this.args = args;
valid = parse();
}
private boolean parse() throws ParseException {
if (schema.length() == 0 && args.length == 0)
return true;
parseSchema();
try {
parseArguments();
} catch (ArgsException e) {
}
return valid;
}
private boolean parseSchema() throws ParseException {
for (String element : schema.split(",")) {
if (element.length() > 0) {
String trimmedElement = element.trim();
parseSchemaElement(trimmedElement);
}
}
return true;
}
private void parseSchemaElement(String element) throws ParseException {
char elementId = element.charAt(0);
String elementTail = element.substring(1);
validateSchemaElementId(elementId);
if (isBooleanSchemaElement(elementTail))
parseBooleanSchemaElement(elementId);
else if (isStringSchemaElement(elementTail))
parseStringSchemaElement(elementId);
else if (isIntegerSchemaElement(elementTail)) {
parseIntegerSchemaElement(elementId);
} else {
throw new ParseException(
String.format("Argument: %c has invalid format: %s.",
elementId, elementTail), 0);
}
}
private void validateSchemaElementId(char elementId) throws ParseException {
if (!Character.isLetter(elementId)) {
throw new ParseException(
"Bad character:" + elementId + "in Args format: " + schema, 0);
}
}
private void parseBooleanSchemaElement(char elementId) {
booleanArgs.put(elementId, false);
}
private void parseIntegerSchemaElement(char elementId) {
intArgs.put(elementId, 0);
}
private void parseStringSchemaElement(char elementId) {
stringArgs.put(elementId, "");
}
private boolean isStringSchemaElement(String elementTail) {
return elementTail.equals("*");
}
private boolean isBooleanSchemaElement(String elementTail) {
return elementTail.length() == 0;
}
private boolean isIntegerSchemaElement(String elementTail) {
return elementTail.equals("#");
}
private boolean parseArguments() throws ArgsException {
for (currentArgument = 0; currentArgument < args.length; currentArgument++)
{
String arg = args[currentArgument];
parseArgument(arg);
}
return true;
}
private void parseArgument(String arg) throws ArgsException {
if (arg.startsWith("-"))
parseElements(arg);
}
private void parseElements(String arg) throws ArgsException {
for (int i = 1; i < arg.length(); i++)
parseElement(arg.charAt(i));
}
private void parseElement(char argChar) throws ArgsException {
if (setArgument(argChar))
argsFound.add(argChar);
else {
unexpectedArguments.add(argChar);
errorCode = ErrorCode.UNEXPECTED_ARGUMENT;
valid = false;
}
}
private boolean setArgument(char argChar) throws ArgsException {
if (isBooleanArg(argChar))
setBooleanArg(argChar, true);
else if (isStringArg(argChar))
setStringArg(argChar);
else if (isIntArg(argChar))
setIntArg(argChar);
else
return false;
return true;
}
private boolean isIntArg(char argChar) {return intArgs.containsKey(argChar);}
private void setIntArg(char argChar) throws ArgsException {
currentArgument++;
String parameter = null;
try {
parameter = args[currentArgument];
intArgs.put(argChar, new Integer(parameter));
} catch (ArrayIndexOutOfBoundsException e) {
valid = false;
errorArgumentId = argChar;
errorCode = ErrorCode.MISSING_INTEGER;
throw new ArgsException();
} catch (NumberFormatException e) {
valid = false;
errorArgumentId = argChar;
errorParameter = parameter;
errorCode = ErrorCode.INVALID_INTEGER;
throw new ArgsException();
}
}
private void setStringArg(char argChar) throws ArgsException {
currentArgument++;
try {
stringArgs.put(argChar, args[currentArgument]);
} catch (ArrayIndexOutOfBoundsException e) {
valid = false;
errorArgumentId = argChar;
errorCode = ErrorCode.MISSING_STRING;
throw new ArgsException();
}
}
private boolean isStringArg(char argChar) {
return stringArgs.containsKey(argChar);
}
private void setBooleanArg(char argChar, boolean value) {
booleanArgs.put(argChar, value);
}
private boolean isBooleanArg(char argChar) {
return booleanArgs.containsKey(argChar);
}
public int cardinality() {
return argsFound.size();
}
public String usage() {
if (schema.length() > 0)
return "-[" + schema + "]";
else
return "";
}
public String errorMessage() throws Exception {
switch (errorCode) {
case OK:
throw new Exception("TILT: Should not get here.");
case UNEXPECTED_ARGUMENT:
return unexpectedArgumentMessage();
case MISSING_STRING:
return String.format("Could not find string parameter for -%c.",
errorArgumentId);
case INVALID_INTEGER:
return String.format("Argument -%c expects an integer but was '%s'.",
errorArgumentId, errorParameter);
case MISSING_INTEGER:
return String.format("Could not find integer parameter for -%c.",
errorArgumentId);
}
return "";
}
private String unexpectedArgumentMessage() {
StringBuffer message = new StringBuffer("Argument(s) -");
for (char c : unexpectedArguments) {
message.append(c);
}
message.append(" unexpected.");
return message.toString();
}
private boolean falseIfNull(Boolean b) {
return b != null && b;
}
private int zeroIfNull(Integer i) {
return i == null ? 0 : i;
}
private String blankIfNull(String s) {
return s == null ? "" : s;
}
public String getString(char arg) {
return blankIfNull(stringArgs.get(arg));
}
public int getInt(char arg) {
return zeroIfNull(intArgs.get(arg));
}
public boolean getBoolean(char arg) {
return falseIfNull(booleanArgs.get(arg));
}
public boolean has(char arg) {
return argsFound.contains(arg);
}
public boolean isValid() {
return valid;
}
private class ArgsException extends Exception {
}
}
Надеюсь, при виде этой глыбы кода вам захотелось сказать: «Как хорошо, что она не осталась в таком виде!» Если вы почувствовали нечто подобное, вспомните, что другие люди чувствуют то же самое при виде вашего кода, оставшегося на стадии «черновика».
Вообще говоря, «черновик» — самое мягкое, что можно сказать об этом коде. Очевидно, что перед нами незавершенная работа. От одного количества переменных экземпляров можно прийти в ужас. Загадочные строки вроде "TILT”, контейнеры HashSet и TreeSet, конструкции try-catch-catch только увеличивают масштабы этого беспорядочного месива.
Я вовсе не собирался писать беспорядочное месиво. В самом деле, я постарался сохранить более или менее разумную организацию кода. Об этом свидетельствует хотя бы выбор имен функций и переменных, а также наличие у программы примитивной структуры. Но совершенно очевидно, что проблемы вышли из-под моего контроля.
Неразбериха накапливалась постепенно. Ранние версии выглядели вовсе не так отвратительно. Для примера в листинге 14.9 приведена начальная версия, поддерживающая только логические аргументы.
package com.objectmentor.utilities.getopts;
import java.util.*;
public class Args {
private String schema;
private String[] args;
private boolean valid;
private Set<Character> unexpectedArguments = new TreeSet<Character>();
private Map<Character, Boolean> booleanArgs =
new HashMap<Character, Boolean>();
private int numberOfArguments = 0;
public Args(String schema, String[] args) {
this.schema = schema;
this.args = args;
valid = parse();
}
public boolean isValid() {
return valid;
}
private boolean parse() {
if (schema.length() == 0 && args.length == 0)
return true;
parseSchema();
parseArguments();
return unexpectedArguments.size() == 0;
}
private boolean parseSchema() {
for (String element : schema.split(",")) {
parseSchemaElement(element);
}
return true;
}
private void parseSchemaElement(String element) {
if (element.length() == 1) {
parseBooleanSchemaElement(element);
}
}
private void parseBooleanSchemaElement(String element) {
char c = element.charAt(0);
if (Character.isLetter(c)) {
booleanArgs.put(c, false);
}
}
private boolean parseArguments() {
for (String arg : args)
parseArgument(arg);
return true;
}
private void parseArgument(String arg) {
if (arg.startsWith("-"))
parseElements(arg);
}
private void parseElements(String arg) {
for (int i = 1; i < arg.length(); i++)
parseElement(arg.charAt(i));
}
private void parseElement(char argChar) {
if (isBoolean(argChar)) {
numberOfArguments++;
setBooleanArg(argChar, true);
} else
unexpectedArguments.add(argChar);
}
private void setBooleanArg(char argChar, boolean value) {
booleanArgs.put(argChar, value);
}
private boolean isBoolean(char argChar) {
return booleanArgs.containsKey(argChar);
}
public int cardinality() {
return numberOfArguments;
}
public String usage() {
if (schema.length() > 0)
return "-["+schema+"]";
else
return "";
}
public String errorMessage() {
if (unexpectedArguments.size() > 0) {
return unexpectedArgumentMessage();
} else
return "";
}
private String unexpectedArgumentMessage() {
StringBuffer message = new StringBuffer("Argument(s) -");
for (char c : unexpectedArguments) {
message.append(c);
}
message.append(" unexpected.");
return message.toString();
}
public boolean getBoolean(char arg) {
return booleanArgs.get(arg);
}
}
В этом коде можно найти множество недостатков, однако в целом он не так уж плох. Код компактен и прост, в нем легко разобраться. Тем не менее в этом коде легко прослеживаются зачатки будущего беспорядочного месива. Нетрудно понять, как из него выросла вся последующая неразбериха.
Обратите внимание: в последующей неразберихе добавились всего два новых типа аргументов, String и integer. Добавление всего двух типов аргументов имело огромные отрицательные последствия для кода. Более или менее понятный код превратился в запутанный клубок, наверняка кишащий множеством ошибок и недочетов.
Два новых типа аргументов добавлялись последовательно. Сначала я добавил поддержку String, что привело к следующему результату.
package com.objectmentor.utilities.getopts;
import java.text.ParseException;
import java.util.*;
public class Args {
private String schema;
private String[] args;
private boolean valid = true;
private Set<Character> unexpectedArguments = new TreeSet<Character>();
private Map<Character, Boolean> booleanArgs =
new HashMap<Character, Boolean>();
private Map<Character, String> stringArgs =
new HashMap<Character, String>();
private Set<Character> argsFound = new HashSet<Character>();
private int currentArgument;
private char errorArgument = '\0';
enum ErrorCode {
OK, MISSING_STRING}
private ErrorCode errorCode = ErrorCode.OK;
public Args(String schema, String[] args) throws ParseException {
this.schema = schema;
this.args = args;
valid = parse();
}
private boolean parse() throws ParseException {
if (schema.length() == 0 && args.length == 0)
return true;
parseSchema();
parseArguments();
return valid;
}
private boolean parseSchema() throws ParseException {
for (String element : schema.split(",")) {
if (element.length() > 0) {
String trimmedElement = element.trim();
parseSchemaElement(trimmedElement);
}
}
return true;
}
private void parseSchemaElement(String element) throws ParseException {
char elementId = element.charAt(0);
String elementTail = element.substring(1);
validateSchemaElementId(elementId);
if (isBooleanSchemaElement(elementTail))
parseBooleanSchemaElement(elementId);
else if (isStringSchemaElement(elementTail))
parseStringSchemaElement(elementId);
}
private void validateSchemaElementId(char elementId) throws ParseException {
if (!Character.isLetter(elementId)) {
throw new ParseException(
"Bad character:" + elementId + "in Args format: " + schema, 0);
}
}
private void parseStringSchemaElement(char elementId) {
stringArgs.put(elementId, "");
}
private boolean isStringSchemaElement(String elementTail) {
return elementTail.equals("*");
}
private boolean isBooleanSchemaElement(String elementTail) {
return elementTail.length() == 0;
}
private void parseBooleanSchemaElement(char elementId) {
booleanArgs.put(elementId, false);
}
private boolean parseArguments() {
for (currentArgument = 0; currentArgument < args.length; currentArgument++)
{
String arg = args[currentArgument];
parseArgument(arg);
}
return true;
}
private void parseArgument(String arg) {
if (arg.startsWith("-"))
parseElements(arg);
}
private void parseElements(String arg) {
for (int i = 1; i < arg.length(); i++)
parseElement(arg.charAt(i));
}
private void parseElement(char argChar) {
if (setArgument(argChar))
argsFound.add(argChar);
else {
unexpectedArguments.add(argChar);
valid = false;
}
}
private boolean setArgument(char argChar) {
boolean set = true;
if (isBoolean(argChar))
setBooleanArg(argChar, true);
else if (isString(argChar))
setStringArg(argChar, "");
else
set = false;
return set;
}
private void setStringArg(char argChar, String s) {
currentArgument++;
try {
stringArgs.put(argChar, args[currentArgument]);
} catch (ArrayIndexOutOfBoundsException e) {
valid = false;
errorArgument = argChar;
errorCode = ErrorCode.MISSING_STRING;
}
}
private boolean isString(char argChar) {
return stringArgs.containsKey(argChar);
}
private void setBooleanArg(char argChar, boolean value) {
booleanArgs.put(argChar, value);
}
private boolean isBoolean(char argChar) {
return booleanArgs.containsKey(argChar);
}
public int cardinality() {
return argsFound.size();
}
public String usage() {
if (schema.length() > 0)
return "-[" + schema + "]";
else
return "";
}
public String errorMessage() throws Exception {
if (unexpectedArguments.size() > 0) {
return unexpectedArgumentMessage();
} else
switch (errorCode) {
case MISSING_STRING:
return String.format("Could not find string parameter for -%c.",
errorArgument);
case OK:
throw new Exception("TILT: Should not get here.");
}
return "";
}
private String unexpectedArgumentMessage() {
StringBuffer message = new StringBuffer("Argument(s) -");
for (char c : unexpectedArguments) {
message.append(c);
}
message.append(" unexpected.");
return message.toString();
}
public boolean getBoolean(char arg) {
return falseIfNull(booleanArgs.get(arg));
}
private boolean falseIfNull(Boolean b) {
return b == null ? false : b;
}
public String getString(char arg) {
return blankIfNull(stringArgs.get(arg));
}
private String blankIfNull(String s) {
return s == null ? "" : s;
}
public boolean has(char arg) {
return argsFound.contains(arg);
}
public boolean isValid() {
return valid;
}
}
Ситуация явно выходит из-под контроля. Код все еще не ужасен, но путаница очевидно растет. Это уже клубок, хотя и не беспорядочное месиво. А чтобы месиво забродило и стало подниматься, хватило простого добавления целочисленных аргументов.
private class ArgumentMarshaler {
private boolean booleanValue = false;
public void setBoolean(boolean value) {
booleanValue = value;
}
public boolean getBoolean() {return booleanValue;}
}
private class BooleanArgumentMarshaler extends ArgumentMarshaler {
}
private class StringArgumentMarshaler extends ArgumentMarshaler {
}
private class IntegerArgumentMarshaler extends ArgumentMarshaler {
}
}
Понятно, что добавление класса ничего не нарушит. Поэтому я внес самое простейшее из всех возможных изменений — изменил контейнер HashMap для логических аргументов так, чтобы при конструировании передавался тип ArgumentMarshaler:
private Map<Character, ArgumentMarshaler> booleanArgs =
new HashMap<Character, ArgumentMarshaler>();
Это нарушило работу нескольких команд, которые я быстро исправил.
...
private void parseBooleanSchemaElement(char elementId) {
booleanArgs.put(elementId, new BooleanArgumentMarshaler());
}
..
private void setBooleanArg(char argChar, boolean value) {
booleanArgs.get(argChar).setBoolean(value);
}
...
public boolean getBoolean(char arg) {
return falseIfNull(booleanArgs.get(arg).getBoolean());
}
Изменения вносятся в тех местах, о которых я упоминал ранее: методы parse, set и get для типа аргумента. К сожалению, при всей незначительности изменений некоторые тесты стали завершаться неудачей. Внимательно присмотревшись к getBoolean, вы увидите, что если при вызове метода с 'y' аргумента y не существует, вызов booleanArgs.get(‘y’) вернет null, а функция выдаст исключение NullPointerException. Функция falseIfNull защищала от подобных ситуаций, но в результате внесенных изменений она перестала работать.
Стратегия постепенных изменений требовала, чтобы я немедленно наладил работу программы, прежде чем вносить какие-либо дополнительные изменения. Действительно, проблема решалась просто: нужно было добавить проверку null. Но на этот раз проверять нужно было не логическое значение, а ArgumentMarshaller.
Сначала я убрал вызов falseIfNull из getBoolean. Функция falseIfNull стала бесполезной, поэтому я убрал и саму функцию. Тесты все равно не проходили, поэтому я был уверен, что новых ошибок от этого уже не прибавится.
public boolean getBoolean(char arg) {
return booleanArgs.get(arg).getBoolean();
}
Затем я разбил функцию getBoolean надвое и разместил ArgumentMarshaller в собственной переменной с именем argumentMarshaller. Длинное имя мне не понравилось; во-первых, оно было избыточным, а во-вторых, загромождало функцию. Соответственно я сократил его до am [N5].
public boolean getBoolean(char arg) {
Args.ArgumentMarshaler am = booleanArgs.get(arg);
return am.getBoolean();
}
Наконец, я добавил логику проверки null:
public boolean getBoolean(char arg) {
Args.ArgumentMarshaler am = booleanArgs.get(arg);
return am != null && am.getBoolean();
}
private Map<Character, ArgumentMarshaler> stringArgs =
new HashMap<Character, ArgumentMarshaler>();
...
private void parseStringSchemaElement(char elementId) {
stringArgs.put(elementId, new StringArgumentMarshaler());
}
...
private void setStringArg(char argChar) throws ArgsException {
currentArgument++;
try {
stringArgs.get(argChar).setString(args[currentArgument]);
} catch (ArrayIndexOutOfBoundsException e) {
valid = false;
errorArgumentId = argChar;
errorCode = ErrorCode.MISSING_STRING;
throw new ArgsException();
}
}
...
public String getString(char arg) {
Args.ArgumentMarshaler am = stringArgs.get(arg);
return am == null ? "" : am.getString();
}
...
private class ArgumentMarshaler {
private boolean booleanValue = false;
private String stringValue;
public void setBoolean(boolean value) {
booleanValue = value;
}
public boolean getBoolean() {
return booleanValue;
}
public void setString(String s) {
stringValue = s;
}
public String getString() {
return stringValue == null ? "" : stringValue;
}
}
И снова изменения вносились последовательно и только так, чтобы тесты по крайней мере хотя бы запускались (даже если и не проходили). Если работоспособность теста была нарушена, я сначала добивался того, чтобы он работал, и только потом переходил к следующему изменению.
Вероятно, вы уже поняли, что я собираюсь сделать. Собрав все текущее поведение компоновки аргументов в базовом классе ArgumentMarshaler, я намерен перемещать его вниз в производные классы. Это позволит мне сохранить работоспособность программы в ходе постепенного изменения ее структуры.
Очевидным следующим шагом стало перемещение функциональности аргумента int в ArgumentMarshaler. И снова все обошлось без сюрпризов:
private Map<Character, ArgumentMarshaler> intArgs =
new HashMap<Character, ArgumentMarshaler>();
...
private void parseIntegerSchemaElement(char elementId) {
intArgs.put(elementId, new IntegerArgumentMarshaler());
}
...
private void setIntArg(char argChar) throws ArgsException {
currentArgument++;
String parameter = null;
try {
parameter = args[currentArgument];
intArgs.get(argChar).setInteger(Integer.parseInt(parameter));
} catch (ArrayIndexOutOfBoundsException e) {
valid = false;
errorArgumentId = argChar;
errorCode = ErrorCode.MISSING_INTEGER;
throw new ArgsException();
} catch (NumberFormatException e) {
valid = false;
errorArgumentId = argChar;
errorParameter = parameter;
errorCode = ErrorCode.INVALID_INTEGER;
throw new ArgsException();
}
}
...
public int getInt(char arg) {
Args.ArgumentMarshaler am = intArgs.get(arg);
return am == null ? 0 : am.getInteger();
}
...
private class ArgumentMarshaler {
private boolean booleanValue = false;
private String stringValue;
private int integerValue;
public void setBoolean(boolean value) {
booleanValue = value;
}
public boolean getBoolean() {
return booleanValue;
}
public void setString(String s) {
stringValue = s;
}
public String getString() {
return stringValue == null ? "" : stringValue;
}
public void setInteger(int i) {
integerValue = i;
}
public int getInteger() {
return integerValue;
}
}
Переместив всю логику компоновки аргументов в ArgumentMarshaler, я занялся перемещением функциональности в производные классы. На первом этапе я должен был переместить функцию setBoolean в BooleanArgumentMarshaller и позаботиться о том, чтобы она правильно вызывалась. Для этого был создан абстрактный метод set.
private abstract class ArgumentMarshaler {
protected boolean booleanValue = false;
private String stringValue;
private int integerValue;
public void setBoolean(boolean value) {
booleanValue = value;
}
public boolean getBoolean() {
return booleanValue;
}
public void setString(String s) {
stringValue = s;
}
public String getString() {
return stringValue == null ? "" : stringValue;
}
public void setInteger(int i) {
integerValue = i;
}
public int getInteger() {
return integerValue;
}
public abstract void set(String s);
}
Затем метод set был реализован в BooleanArgumentMarshaller.
private class BooleanArgumentMarshaler extends ArgumentMarshaler {
public void set(String s) {
booleanValue = true;
}
}
Наконец, вызов setBoolean был заменен вызовом set.
private void setBooleanArg(char argChar, boolean value) {
booleanArgs.get(argChar).set("true");
}
Все тесты прошли успешно. Так как изменения привели к перемещению set в BooleanArgumentMarshaler, я удалил метод setBoolean из базового класса ArgumentMarshaler.
Обратите внимание: абстрактная функция set получает аргумент String, но реализация в классе BooleanArgumentMarshaller его не использует. Я добавил этот аргумент, потому что знал, что он будет использоваться классами StringArgumentMarshaller и IntegerArgumentMarshaller.
На следующем шаге я решил разместить метод get в BooleanArgumentMarshaler. Подобные размещения get всегда выглядят уродливо, потому что фактически возвращается тип Object, который в данном случае приходится преобразовывать в Boolean.
public boolean getBoolean(char arg) {
Args.ArgumentMarshaler am = booleanArgs.get(arg);
return am != null && (Boolean)am.get();
}
Просто для того, чтобы программа компилировалась, я добавил в ArgumentMarshaler функцию get.
private abstract class ArgumentMarshaler {
...
public Object get() {
return null;
}
}
Программа компилировалась, а тесты, разумеется, не проходили. Чтобы тесты снова заработали, достаточно объявить метод get абстрактным и реализовать его в BooleanAgumentMarshaler.
private abstract class ArgumentMarshaler {
protected boolean booleanValue = false;
...
public abstract Object get();
}
private class BooleanArgumentMarshaler extends ArgumentMarshaler {
public void set(String s) {
booleanValue = true;
}
public Object get() {
return booleanValue;
}
}
Итак, тесты снова проходят успешно. Теперь оба метода get и set размещаются в BooleanArgumentMarshaler! Это позволило мне удалить старую функцию getBoolean из ArgumentMarshaler, переместить защищенную переменную booleanValue в BooleanArgumentMarshaler и объявить ее приватной.
Аналогичные изменения были внесены для типа String. Я реализовал методы set и get, удалил ненужные функции и переместил переменные.
private void setStringArg(char argChar) throws ArgsException {
currentArgument++;
try {
stringArgs.get(argChar).set(args[currentArgument]);
} catch (ArrayIndexOutOfBoundsException e) {
valid = false;
errorArgumentId = argChar;
errorCode = ErrorCode.MISSING_STRING;
throw new ArgsException();
}
}
...
public String getString(char arg) {
Args.ArgumentMarshaler am = stringArgs.get(arg);
return am == null ? "" : (String) am.get();
}
...
private abstract class ArgumentMarshaler {
private int integerValue;
public void setInteger(int i) {
integerValue = i;
}
public int getInteger() {
return integerValue;
}
public abstract void set(String s);
public abstract Object get();
}
private class BooleanArgumentMarshaler extends ArgumentMarshaler {
private boolean booleanValue = false;
public void set(String s) {
booleanValue = true;
}
public Object get() {
return booleanValue;
}
}
private class StringArgumentMarshaler extends ArgumentMarshaler {
private String stringValue = "";
public void set(String s) {
stringValue = s;
}
public Object get() {
return stringValue;
}
}
private class IntegerArgumentMarshaler extends ArgumentMarshaler {
public void set(String s) {
}
public Object get() {
return null;
}
}
}
Осталось лишь повторить этот процесс для integer. На этот раз задача немного усложняется: целые числа необходимо разбирать, а в ходе разбора возможны исключения. Но внешний вид кода улучшается тем, что вся концепция NumberFormatException скрыта в классе IntegerArgumentMarshaler.
private boolean isIntArg(char argChar) {return intArgs.containsKey(argChar);}
private void setIntArg(char argChar) throws ArgsException {
currentArgument++;
String parameter = null;
try {
parameter = args[currentArgument];
intArgs.get(argChar).set(parameter);
} catch (ArrayIndexOutOfBoundsException e) {
valid = false;
errorArgumentId = argChar;
errorCode = ErrorCode.MISSING_INTEGER;
throw new ArgsException();
} catch (ArgsException e) {
valid = false;
errorArgumentId = argChar;
errorParameter = parameter;
errorCode = ErrorCode.INVALID_INTEGER;
throw e;
}
}
...
private void setBooleanArg(char argChar) {
try {
booleanArgs.get(argChar).set("true");
} catch (ArgsException e) {
}
}
...
public int getInt(char arg) {
Args.ArgumentMarshaler am = intArgs.get(arg);
return am == null ? 0 : (Integer) am.get();
}
...
private abstract class ArgumentMarshaler {
public abstract void set(String s) throws ArgsException;
public abstract Object get();
}
...
private class IntegerArgumentMarshaler extends ArgumentMarshaler {
private int intValue = 0;
public void set(String s) throws ArgsException {
try {
intValue = Integer.parseInt(s);
} catch (NumberFormatException e) {
throw new ArgsException();
}
}
public Object get() {
return intValue;
}
}
Конечно, тесты по-прежнему проходили. Далее я избавился от трех разновидностей Map в начале алгоритма, отчего система стала намного более универсальной. Впрочем, я не мог их просто удалить, поскольку это нарушило бы работу системы. Вместо этого я добавил новый объект Map для ArgumentMarshaler, а затем последовательно изменял методы, чтобы они использовали этот объект вместо трех исходных.
public class Args {
...
private Map<Character, ArgumentMarshaler> booleanArgs =
new HashMap<Character, ArgumentMarshaler>();
private Map<Character, ArgumentMarshaler> stringArgs =
new HashMap<Character, ArgumentMarshaler>();
private Map<Character, ArgumentMarshaler> intArgs =
new HashMap<Character, ArgumentMarshaler>();
private Map<Character, ArgumentMarshaler> marshalers =
new HashMap<Character, ArgumentMarshaler>();
...
private void parseBooleanSchemaElement(char elementId) {
ArgumentMarshaler m = new BooleanArgumentMarshaler();
booleanArgs.put(elementId, m);
marshalers.put(elementId, m);
}
private void parseIntegerSchemaElement(char elementId) {
ArgumentMarshaler m = new IntegerArgumentMarshaler();
intArgs.put(elementId, m);
marshalers.put(elementId, m);
}
private void parseStringSchemaElement(char elementId) {
ArgumentMarshaler m = new StringArgumentMarshaler();
stringArgs.put(elementId, m);
marshalers.put(elementId, m);
}
Разумеется, тесты проходили успешно. Далее я привел метод isBooleanArg:
private boolean isBooleanArg(char argChar) {
return booleanArgs.containsKey(argChar);
}
к следующему виду:
private boolean isBooleanArg(char argChar) {
ArgumentMarshaler m = marshalers.get(argChar);
return m instanceof BooleanArgumentMarshaler;
}
Тесты по-прежнему проходят. Я внес аналогичные изменения в isIntArg и isStringArg.
private boolean isIntArg(char argChar) {
ArgumentMarshaler m = marshalers.get(argChar);
return m instanceof IntegerArgumentMarshaler;
}
private boolean isStringArg(char argChar) {
ArgumentMarshaler m = marshalers.get(argChar);
return m instanceof StringArgumentMarshaler;
}
Тесты проходят. Я удалил все повторяющиеся вызовы marshalers.get:
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
if (isBooleanArg(m))
setBooleanArg(argChar);
else if (isStringArg(m))
setStringArg(argChar);
else if (isIntArg(m))
setIntArg(argChar);
else
return false;
return true;
}
private boolean isIntArg(ArgumentMarshaler m) {
return m instanceof IntegerArgumentMarshaler;
}
private boolean isStringArg(ArgumentMarshaler m) {
return m instanceof StringArgumentMarshaler;
}
private boolean isBooleanArg(ArgumentMarshaler m) {
return m instanceof BooleanArgumentMarshaler;
}
Причин для существования трех методов isxxxArg не осталось. Я оформил их в виде встроенного кода:
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
if (m instanceof BooleanArgumentMarshaler)
setBooleanArg(argChar);
else if (m instanceof StringArgumentMarshaler)
setStringArg(argChar);
else if (m instanceof IntegerArgumentMarshaler)
setIntArg(argChar);
else
return false;
return true;
}
На следующем шаге я перешел на использование ассоциативного массива marshalers в функциях set, отказываясь от использования трех старых контейнеров. Преобразование началось с Boolean:
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
if (m instanceof BooleanArgumentMarshaler)
setBooleanArg(m);
else if (m instanceof StringArgumentMarshaler)
setStringArg(argChar);
else if (m instanceof IntegerArgumentMarshaler)
setIntArg(argChar);
else
return false;
return true;
}
...
private void setBooleanArg(ArgumentMarshaler m) {
try {
m.set(“true”); // было: booleanArgs.get(argChar).set(«true»);
} catch (ArgsException e) {
}
}
Тесты проходили успешно, и я сделал то же самое для типов String и Integer. Это позволило мне интегрировать часть некрасивого кода обработки исключений в функцию setArgument.
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
try {
if (m instanceof BooleanArgumentMarshaler)
setBooleanArg(m);
else if (m instanceof StringArgumentMarshaler)
setStringArg(m);
else if (m instanceof IntegerArgumentMarshaler)
setIntArg(m);
else
return false;
} catch (ArgsException e) {
valid = false;
errorArgumentId = argChar;
throw e;
}
return true;
}
private void setIntArg(ArgumentMarshaler m) throws ArgsException {
currentArgument++;
String parameter = null;
try {
parameter = args[currentArgument];
m.set(parameter);
} catch (ArrayIndexOutOfBoundsException e) {
errorCode = ErrorCode.MISSING_INTEGER;
throw new ArgsException();
} catch (ArgsException e) {
errorParameter = parameter;
errorCode = ErrorCode.INVALID_INTEGER;
throw e;
}
}
private void setStringArg(ArgumentMarshaler m) throws ArgsException {
currentArgument++;
try {
m.set(args[currentArgument]);
} catch (ArrayIndexOutOfBoundsException e) {
errorCode = ErrorCode.MISSING_STRING;
throw new ArgsException();
}
}
Я вплотную подошел к удалению трех старых объектов Map. Прежде всего было необходимо привести функцию getBoolean:
public boolean getBoolean(char arg) {
Args.ArgumentMarshaler am = booleanArgs.get(arg);
return am != null && (Boolean) am.get();
}
к следующему виду:
public boolean getBoolean(char arg) {
Args.ArgumentMarshaler am = marshalers.get(arg);
boolean b = false;
try {
b = am != null && (Boolean) am.get();
} catch (ClassCastException e) {
b = false;
}
return b;
}
Возможно, последнее изменение вас удивило. Почему я вдруг решил обрабатывать ClassCastException? Дело в том, что наряду с набором модульных тестов у меня был отдельный набор приемочных тестов, написанных для FitNesse. Оказалось, что тесты FitNesse проверяли, что при вызове getBoolean для аргумента с типом, отличным от Boolean, возвращается false. Модульные тесты этого не делали. До этого момента я запускал только модульные тесты[69].
Последнее изменение позволило исключить еще одну точку использования объекта Map для типа Boolean:
private void parseBooleanSchemaElement(char elementId) {
ArgumentMarshaler m = new BooleanArgumentMarshaler();
booleanArgs.put(elementId, m);
marshalers.put(elementId, m);
}
Теперь объект Map для типа Boolean можно было удалить:
public class Args {
...
private Map<Character, ArgumentMarshaler> booleanArgs =
new HashMap<Character, ArgumentMarshaler>();
private Map<Character, ArgumentMarshaler> stringArgs =
new HashMap<Character, ArgumentMarshaler>();
private Map<Character, ArgumentMarshaler> intArgs =
new HashMap<Character, ArgumentMarshaler>();
private Map<Character, ArgumentMarshaler> marshalers =
new HashMap<Character, ArgumentMarshaler>();
...
Далее я проделал аналогичную процедуру для аргументов String и Integer и немного подчистил код:
private void parseBooleanSchemaElement(char elementId) {
marshalers.put(elementId, new BooleanArgumentMarshaler());
}
private void parseIntegerSchemaElement(char elementId) {
marshalers.put(elementId, new IntegerArgumentMarshaler());
}
private void parseStringSchemaElement(char elementId) {
marshalers.put(elementId, new StringArgumentMarshaler());
}
...
public String getString(char arg) {
Args.ArgumentMarshaler am = marshalers.get(arg);
try {
return am == null ? "" : (String) am.get();
} catch (ClassCastException e) {
return "";
}
}
...
public class Args {
...
private Map<Character, ArgumentMarshaler> stringArgs =
new HashMap<Character, ArgumentMarshaler>();
private Map<Character, ArgumentMarshaler> intArgs =
new HashMap<Character, ArgumentMarshaler>();
private Map<Character, ArgumentMarshaler> marshalers =
new HashMap<Character, ArgumentMarshaler>();
...
Затем я подставил в parseSchemaElement код трех методов parse, сократившихся до одной команды:
private void parseSchemaElement(String element) throws ParseException {
char elementId = element.charAt(0);
String elementTail = element.substring(1);
validateSchemaElementId(elementId);
if (isBooleanSchemaElement(elementTail))
marshalers.put(elementId, new BooleanArgumentMarshaler());
else if (isStringSchemaElement(elementTail))
marshalers.put(elementId, new StringArgumentMarshaler());
else if (isIntegerSchemaElement(elementTail)) {
marshalers.put(elementId, new IntegerArgumentMarshaler());
} else {
throw new ParseException(String.format(
"Argument: %c has invalid format: %s.", elementId, elementTail), 0);
}
}
Давайте взглянем на общую картину. В листинге 14.12 представлена текущая форма класса Args.
package com.objectmentor.utilities.getopts;
import java.text.ParseException;
import java.util.*;
public class Args {
private String schema;
private String[] args;
private boolean valid = true;
private Set<Character> unexpectedArguments = new TreeSet<Character>();
private Map<Character, ArgumentMarshaler> marshalers =
new HashMap<Character, ArgumentMarshaler>();
private Set<Character> argsFound = new HashSet<Character>();
private int currentArgument;
private char errorArgumentId = '\0';
private String errorParameter = "TILT";
private ErrorCode errorCode = ErrorCode.OK;
private enum ErrorCode {
OK, MISSING_STRING, MISSING_INTEGER, INVALID_INTEGER, UNEXPECTED_ARGUMENT}
public Args(String schema, String[] args) throws ParseException {
this.schema = schema;
this.args = args;
valid = parse();
}
private boolean parse() throws ParseException {
if (schema.length() == 0 && args.length == 0)
return true;
parseSchema();
try {
parseArguments();
} catch (ArgsException e) {
}
return valid;
}
private boolean parseSchema() throws ParseException {
for (String element : schema.split(",")) {
if (element.length() > 0) {
String trimmedElement = element.trim();
parseSchemaElement(trimmedElement);
}
}
return true;
}
private void parseSchemaElement(String element) throws ParseException {
char elementId = element.charAt(0);
String elementTail = element.substring(1);
validateSchemaElementId(elementId);
if (isBooleanSchemaElement(elementTail))
marshalers.put(elementId, new BooleanArgumentMarshaler());
else if (isStringSchemaElement(elementTail))
marshalers.put(elementId, new StringArgumentMarshaler());
else if (isIntegerSchemaElement(elementTail)) {
marshalers.put(elementId, new IntegerArgumentMarshaler());
} else {
throw new ParseException(String.format(
"Argument: %c has invalid format: %s.", elementId, elementTail), 0);
}
}
private void validateSchemaElementId(char elementId) throws ParseException {
if (!Character.isLetter(elementId)) {
throw new ParseException(
"Bad character:" + elementId + "in Args format: " + schema, 0);
}
}
private boolean isStringSchemaElement(String elementTail) {
return elementTail.equals("*");
}
private boolean isBooleanSchemaElement(String elementTail) {
return elementTail.length() == 0;
}
private boolean isIntegerSchemaElement(String elementTail) {
return elementTail.equals("#");
}
private boolean parseArguments() throws ArgsException {
for (currentArgument=0; currentArgument<args.length; currentArgument++) {
String arg = args[currentArgument];
parseArgument(arg);
}
return true;
}
private void parseArgument(String arg) throws ArgsException {
if (arg.startsWith("-"))
parseElements(arg);
}
private void parseElements(String arg) throws ArgsException {
for (int i = 1; i < arg.length(); i++)
parseElement(arg.charAt(i));
}
private void parseElement(char argChar) throws ArgsException {
if (setArgument(argChar))
argsFound.add(argChar);
else {
unexpectedArguments.add(argChar);
errorCode = ErrorCode.UNEXPECTED_ARGUMENT;
valid = false;
}
}
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
try {
if (m instanceof BooleanArgumentMarshaler)
setBooleanArg(m);
else if (m instanceof StringArgumentMarshaler)
setStringArg(m);
else if (m instanceof IntegerArgumentMarshaler)
setIntArg(m);
else
return false;
} catch (ArgsException e) {
valid = false;
errorArgumentId = argChar;
throw e;
}
return true;
}
private void setIntArg(ArgumentMarshaler m) throws ArgsException {
currentArgument++;
String parameter = null;
try {
parameter = args[currentArgument];
m.set(parameter);
} catch (ArrayIndexOutOfBoundsException e) {
errorCode = ErrorCode.MISSING_INTEGER;
throw new ArgsException();
} catch (ArgsException e) {
errorParameter = parameter;
errorCode = ErrorCode.INVALID_INTEGER;
throw e;
}
}
private void setStringArg(ArgumentMarshaler m) throws ArgsException {
currentArgument++;
try {
m.set(args[currentArgument]);
} catch (ArrayIndexOutOfBoundsException e) {
errorCode = ErrorCode.MISSING_STRING;
throw new ArgsException();
}
}
private void setBooleanArg(ArgumentMarshaler m) {
try {
m.set("true");
} catch (ArgsException e) {
}
}
public int cardinality() {
return argsFound.size();
}
public String usage() {
if (schema.length() > 0)
return "-[" + schema + "]";
else
return "";
}
public String errorMessage() throws Exception {
switch (errorCode) {
case OK:
throw new Exception("TILT: Should not get here.");
case UNEXPECTED_ARGUMENT:
return unexpectedArgumentMessage();
case MISSING_STRING:
return String.format("Could not find string parameter for -%c.",
errorArgumentId);
case INVALID_INTEGER:
return String.format("Argument -%c expects an integer but was '%s'.",
errorArgumentId, errorParameter);
case MISSING_INTEGER:
return String.format("Could not find integer parameter for -%c.",
errorArgumentId);
}
return "";
}
private String unexpectedArgumentMessage() {
StringBuffer message = new StringBuffer("Argument(s) -");
for (char c : unexpectedArguments) {
message.append(c);
}
message.append(" unexpected.");
return message.toString();
}
public boolean getBoolean(char arg) {
Args.ArgumentMarshaler am = marshalers.get(arg);
boolean b = false;
try {
b = am != null && (Boolean) am.get();
} catch (ClassCastException e) {
b = false;
}
return b;
}
public String getString(char arg) {
Args.ArgumentMarshaler am = marshalers.get(arg);
try {
return am == null ? "" : (String) am.get();
} catch (ClassCastException e) {
return "";
}
}
public int getInt(char arg) {
Args.ArgumentMarshaler am = marshalers.get(arg);
try {
return am == null ? 0 : (Integer) am.get();
} catch (Exception e) {
return 0;
}
}
public boolean has(char arg) {
return argsFound.contains(arg);
}
public boolean isValid() {
return valid;
}
private class ArgsException extends Exception {
}
private abstract class ArgumentMarshaler {
public abstract void set(String s) throws ArgsException;
public abstract Object get();
}
private class BooleanArgumentMarshaler extends ArgumentMarshaler {
private boolean booleanValue = false;
public void set(String s) {
booleanValue = true;
}
public Object get() {
return booleanValue;
}
}
private class StringArgumentMarshaler extends ArgumentMarshaler {
private String stringValue = "";
public void set(String s) {
stringValue = s;
}
public Object get() {
return stringValue;
}
}
private class IntegerArgumentMarshaler extends ArgumentMarshaler {
private int intValue = 0;
public void set(String s) throws ArgsException {
try {
intValue = Integer.parseInt(s);
} catch (NumberFormatException e) {
throw new ArgsException();
}
}
public Object get() {
return intValue;
}
}
}
Вроде бы проделана большая работа, а результат не впечатляет. Структура кода немного улучшилась, но в начале листинга по-прежнему объявляются многочисленные переменные; в setArgument осталась кошмарная конструкция проверки типа; функции set выглядят просто ужасно. Я уже не говорю об обработке ошибок… Нам еще предстоит большая работа.
Прежде всего хотелось бы избавиться от конструкции выбора в setArgument [G23]. В идеале она должна быть заменена единственным вызовом ArgumentMarshaler.set. Это означает, что код setIntArg, setStringArg и setBooleanArg должен быть перемещен в соответствующие классы, производные от ArgumentMarshaler. Однако при этом возникает одна проблема.
Внимательно присмотревшись к функции setIntArg, можно заметить, что в ней используются две переменные экземпляров: args и currentArg. Чтобы переместить setIntArg в BooleanArgumentMarshaler, мне придется передать args и currentArgs в аргументах при вызове. Решение получается «грязным» [F1]. Я бы предпочел передать один аргумент вместо двух. К счастью, у проблемы существует простое решение: мы можем преобразовать массив args в list и передать Iterator функциям set. Следующее преобразование было проведено за десять шагов, с обязательным выполнением всех тестов после каждого шага. Здесь я приведу только конечный результат, но вы легко сможете опознать большинство промежуточных шагов по этому листингу.
public class Args {
private String schema;
private String[] args;
private boolean valid = true;
private Set<Character> unexpectedArguments = new TreeSet<Character>();
private Map<Character, ArgumentMarshaler> marshalers =
new HashMap<Character, ArgumentMarshaler>();
private Set<Character> argsFound = new HashSet<Character>();
private Iterator<String> currentArgument;
private char errorArgumentId = '\0';
private String errorParameter = "TILT";
private ErrorCode errorCode = ErrorCode.OK;
private List<String> argsList;
private enum ErrorCode {
OK, MISSING_STRING, MISSING_INTEGER, INVALID_INTEGER, UNEXPECTED_ARGUMENT}
public Args(String schema, String[] args) throws ParseException {
this.schema = schema;
argsList = Arrays.asList(args);
valid = parse();
}
private boolean parse() throws ParseException {
if (schema.length() == 0 && argsList.size() == 0)
return true;
parseSchema();
try {
parseArguments();
} catch (ArgsException e) {
}
return valid;
}
---
private boolean parseArguments() throws ArgsException {
for (currentArgument = argsList.iterator(); currentArgument.hasNext();) {
String arg = currentArgument.next();
parseArgument(arg);
}
return true;
}
---
private void setIntArg(ArgumentMarshaler m) throws ArgsException {
String parameter = null;
try {
parameter = currentArgument.next();
m.set(parameter);
} catch (NoSuchElementException e) {
errorCode = ErrorCode.MISSING_INTEGER;
throw new ArgsException();
} catch (ArgsException e) {
errorParameter = parameter;
errorCode = ErrorCode.INVALID_INTEGER;
throw e;
}
}
private void setStringArg(ArgumentMarshaler m) throws ArgsException {
try {
m.set(currentArgument.next());
} catch (NoSuchElementException e) {
errorCode = ErrorCode.MISSING_STRING;
throw new ArgsException();
}
}
Все изменения были простыми и не нарушали работы тестов. Теперь можно заняться перемещением функций в соответствующие производные классы. Начнем с внесения изменений в setArgument:
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
if (m == null)
return false;
try {
if (m instanceof BooleanArgumentMarshaler)
setBooleanArg(m);
else if (m instanceof StringArgumentMarshaler)
setStringArg(m);
else if (m instanceof IntegerArgumentMarshaler)
setIntArg(m);
else
return false;
} catch (ArgsException e) {
valid = false;
errorArgumentId = argChar;
throw e;
}
return true;
}
Это изменение важно, потому что мы хотим полностью устранить цепочку if-else. Для этого из нее необходимо вывести состояние ошибки.
Теперь можно переходить к перемещению функций set. Функция setBooleanArg тривиальна, поэтому начнем с нее. Наша задача — изменить функцию setBooleanArg так, чтобы она просто передавала управление BooleanArgumentMarshaler.
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
if (m == null)
return false;
try {
if (m instanceof BooleanArgumentMarshaler)
setBooleanArg(m, currentArgument);
else if (m instanceof StringArgumentMarshaler)
setStringArg(m);
else if (m instanceof IntegerArgumentMarshaler)
setIntArg(m);
} catch (ArgsException e) {
valid = false;
errorArgumentId = argChar;
throw e;
}
return true;
}
---
private void setBooleanArg(ArgumentMarshaler m,
Iterator<String> currentArgument)
throws ArgsException {
try {
m.set("true");
catch (ArgsException e) {
}
}
Но ведь мы только что перенесли обработку исключения в функцию? Ситуация с включением того, что вы намереваетесь вскоре исключить, весьма часто встречается при переработке кода. Малый размер шагов и необходимость прохождения тестов означает, что вам придется часто перемещать туда-сюда фрагменты кода. Переработка кода напоминает кубик Рубика: чтобы добиться большой цели, необходимо выполнить множество мелких операций. Каждая операция делает возможной следующую.
Зачем передавать итератор, если setBooleanArg он не нужен? Потому что он нужен setIntArg и setStringArg! И если я хочу организовать доступ ко всем трем функциям через абстрактный метод в ArgumentMarshaller, мне не обойтись без его передачи setBooleanArg.
Итак, функция setBooleanArg стала бесполезной. Если бы в ArgumentMarshaler присутствовала функция set, то мы могли бы вызвать ее напрямую. Значит, нужно создать такую функцию! Первым шагом станет включение нового абстрактного метода в ArgumentMarshaler.
private abstract class ArgumentMarshaler {
public abstract void set(Iterator<String> currentArgument)
throws ArgsException;
public abstract void set(String s) throws ArgsException;
public abstract Object get();
}
Конечно, это нарушает работу всех производных классов, поэтому мы добавим реализацию нового метода в каждый из них.
private class BooleanArgumentMarshaler extends ArgumentMarshaler {
private boolean booleanValue = false;
public void set(Iterator<String> currentArgument) throws ArgsException {
booleanValue = true;
}
public void set(String s) {
booleanValue = true;
}
public Object get() {
return booleanValue;
}
}
private class StringArgumentMarshaler extends ArgumentMarshaler {
private String stringValue = "";
public void set(Iterator<String> currentArgument) throws ArgsException {
}
public void set(String s) {
stringValue = s;
}
public Object get() {
return stringValue;
}
}
private class IntegerArgumentMarshaler extends ArgumentMarshaler {
private int intValue = 0;
public void set(Iterator<String> currentArgument) throws ArgsException {
}
public void set(String s) throws ArgsException {
try {
intValue = Integer.parseInt(s);
} catch (NumberFormatException e) {
throw new ArgsException();
}
}
public Object get() {
return intValue;
}
}
А теперь setBooleanArg можно удалить!
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
if (m == null)
return false;
try {
if (m instanceof BooleanArgumentMarshaler)
m.set(currentArgument);
else if (m instanceof StringArgumentMarshaler)
setStringArg(m);
else if (m instanceof IntegerArgumentMarshaler)
setIntArg(m);
} catch (ArgsException e) {
valid = false;
errorArgumentId = argChar;
throw e;
}
return true;
}
Все тесты проходят, а функция set размещается в BooleanArgumentMarshaler! Теперь можно сделать то же самое для String and Integer.
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
if (m == null)
return false;
try {
if (m instanceof BooleanArgumentMarshaler)
m.set(currentArgument);
else if (m instanceof StringArgumentMarshaler)
m.set(currentArgument);
else if (m instanceof IntegerArgumentMarshaler)
m.set(currentArgument);
} catch (ArgsException e) {
valid = false;
errorArgumentId = argChar;
throw e;
}
return true;
}
---
private class StringArgumentMarshaler extends ArgumentMarshaler {
private String stringValue = "";
public void set(Iterator<String> currentArgument) throws ArgsException {
try {
stringValue = currentArgument.next();
} catch (NoSuchElementException e) {
errorCode = ErrorCode.MISSING_STRING;
throw new ArgsException();
}
}
public void set(String s) {
}
public Object get() {
return stringValue;
}
}
private class IntegerArgumentMarshaler extends ArgumentMarshaler {
private int intValue = 0;
public void set(Iterator<String> currentArgument) throws ArgsException {
String parameter = null;
try {
parameter = currentArgument.next();
set(parameter);
} catch (NoSuchElementException e) {
errorCode = ErrorCode.MISSING_INTEGER;
throw new ArgsException();
} catch (ArgsException e) {
errorParameter = parameter;
errorCode = ErrorCode.INVALID_INTEGER;
throw e;
}
}
public void set(String s) throws ArgsException {
try {
intValue = Integer.parseInt(s);
} catch (NumberFormatException e) {
throw new ArgsException();
}
}
public Object get() {
return intValue;
}
}
А теперь завершающий штрих: убираем цепочку if-else!
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
if (m == null)
return false;
try {
m.set(currentArgument);
return true;
} catch (ArgsException e) {
valid = false;
errorArgumentId = argChar;
throw e;
}
}
Избавляемся от лишних функций в IntegerArgumentMarshaler и слегка чистим код:
private class IntegerArgumentMarshaler extends ArgumentMarshaler {
private int intValue = 0
public void set(Iterator<String> currentArgument) throws ArgsException {
String parameter = null;
try {
parameter = currentArgument.next();
intValue = Integer.parseInt(parameter);
} catch (NoSuchElementException e) {
errorCode = ErrorCode.MISSING_INTEGER;
throw new ArgsException();
} catch (NumberFormatException e) {
errorParameter = parameter;
errorCode = ErrorCode.INVALID_INTEGER;
throw new ArgsException();
}
}
public Object get() {
return intValue;
}
}
}
ArgumentMarshaler преобразуется в интерфейс:
private interface ArgumentMarshaler {
void set(Iterator<String> currentArgument) throws ArgsException;
Object get();
}
А теперь посмотрите, как легко добавлять новые типы аргументов в эту структуру. Количество изменений минимально, а все изменения логически изолированы. Начнем с добавления нового тестового сценария, проверяющего правильность работы аргументов double:
public void testSimpleDoublePresent() throws Exception {
Args args = new Args("x##", new String[] {"-x","42.3"});
assertTrue(args.isValid());
assertEquals(1, args.cardinality());
assertTrue(args.has('x'));
assertEquals(42.3, args.getDouble('x'), .001);
}
Чистим код разбора форматной строки и добавляем обнаружение ## для аргументов типа double.
private void parseSchemaElement(String element) throws ParseException {
char elementId = element.charAt(0);
String elementTail = element.substring(1);
validateSchemaElementId(elementId);
if (elementTail.length() == 0)
marshalers.put(elementId, new BooleanArgumentMarshaler());
else if (elementTail.equals("*"))
marshalers.put(elementId, new StringArgumentMarshaler());
else if (elementTail.equals("#"))
marshalers.put(elementId, new IntegerArgumentMarshaler());
else if (elementTail.equals("##"))
marshalers.put(elementId, new DoubleArgumentMarshaler());
else
throw new ParseException(String.format(
"Argument: %c has invalid format: %s.", elementId, elementTail), 0);
}
Затем пишется класс DoubleArgumentMarshaler.
private class DoubleArgumentMarshaler implements ArgumentMarshaler {
private double doubleValue = 0;
public void set(Iterator<String> currentArgument) throws ArgsException {
String parameter = null;
try {
parameter = currentArgument.next();
doubleValue = Double.parseDouble(parameter);
} catch (NoSuchElementException e) {
errorCode = ErrorCode.MISSING_DOUBLE;
throw new ArgsException();
} catch (NumberFormatException e) {
errorParameter = parameter;
errorCode = ErrorCode.INVALID_DOUBLE;
throw new ArgsException();
}
}
public Object get() {
return doubleValue;
}
}
Для нового типа добавляются новые коды ошибок:
private enum ErrorCode {
OK, MISSING_STRING, MISSING_INTEGER, INVALID_INTEGER, UNEXPECTED_ARGUMENT,
MISSING_DOUBLE, INVALID_DOUBLE}
А еще понадобится функция getDouble:
public double getDouble(char arg) {
Args.ArgumentMarshaler am = marshalers.get(arg);
try {
return am == null ? 0 : (Double) am.get();
} catch (Exception e) {
return 0.0;
}
}
И все тесты успешно проходят! Добавление нового типа прошло в целом безболезненно. Теперь давайте убедимся в том, что обработка ошибок работает правильно. Следующий тестовый сценарий проверяет, что при передаче неразбираемой строки с аргументом ## выдается соответствующая ошибка:
public void testInvalidDouble() throws Exception {
Args args = new Args("x##", new String[] {"-x","Forty two"});
assertFalse(args.isValid());
assertEquals(0, args.cardinality());
assertFalse(args.has('x'));
assertEquals(0, args.getInt('x'));
assertEquals("Argument -x expects a double but was 'Forty two'.",
args.errorMessage());
}
---
public String errorMessage() throws Exception {
switch (errorCode) {
case OK:
throw new Exception("TILT: Should not get here.");
case UNEXPECTED_ARGUMENT:
return unexpectedArgumentMessage();
case MISSING_STRING:
return String.format("Could not find string parameter for -%c.",
errorArgumentId);
case INVALID_INTEGER:
return String.format("Argument -%c expects an integer but was '%s'.",
errorArgumentId, errorParameter);
case MISSING_INTEGER:
return String.format("Could not find integer parameter for -%c.",
errorArgumentId);
case INVALID_DOUBLE:
return String.format("Argument -%c expects a double but was '%s'.",
errorArgumentId, errorParameter);
case MISSING_DOUBLE:
return String.format("Could not find double parameter for -%c.",
errorArgumentId);
}
return "";
}
Тесты успешно проходят. Следующий тест проверяет, что ошибка с отсутствующим аргументом double будет успешно обнаружена:
public void testMissingDouble() throws Exception {
Args args = new Args("x##", new String[]{"-x"});
assertFalse(args.isValid());
assertEquals(0, args.cardinality());
assertFalse(args.has('x'));
assertEquals(0.0, args.getDouble('x'), 0.01);
assertEquals("Could not find double parameter for -x.",
args.errorMessage());
}
Как и ожидалось, все проходит успешно. Этот тест был написан просто для полноты картины.
Код исключения некрасив, и в классе Args ему не место. Также в коде инициируется исключение ParseException, которое на самом деле нам не принадлежит. Давайте объединим все исключения в один класс ArgsException и переместим его в отдельный модуль.
public class ArgsException extends Exception {
private char errorArgumentId = '\0';
private String errorParameter = "TILT";
private ErrorCode errorCode = ErrorCode.OK;
public ArgsException() {}
public ArgsException(String message) {super(message);}
public enum ErrorCode {
OK, MISSING_STRING, MISSING_INTEGER, INVALID_INTEGER, UNEXPECTED_ARGUMENT,
MISSING_DOUBLE, INVALID_DOUBLE}
}
---
public class Args {
...
private char errorArgumentId = '\0';
private String errorParameter = "TILT";
private ArgsException.ErrorCode errorCode = ArgsException.ErrorCode.OK;
private List<String> argsList;
public Args(String schema, String[] args) throws ArgsException {
this.schema = schema;
argsList = Arrays.asList(args);
valid = parse();
}
private boolean parse() throws ArgsException {
if (schema.length() == 0 && argsList.size() == 0)
return true;
parseSchema();
try {
parseArguments();
} catch (ArgsException e) {
}
return valid;
}
private boolean parseSchema() throws ArgsException {
...
}
private void parseSchemaElement(String element) throws ArgsException {
...
else
throw new ArgsException(
String.format("Argument: %c has invalid format: %s.",
elementId,elementTail));
}
private void validateSchemaElementId(char elementId) throws ArgsException {
if (!Character.isLetter(elementId)) {
throw new ArgsException(
"Bad character:" + elementId + "in Args format: " + schema);
}
}
...
private void parseElement(char argChar) throws ArgsException {
if (setArgument(argChar))
argsFound.add(argChar);
else {
unexpectedArguments.add(argChar);
errorCode = ArgsException.ErrorCode.UNEXPECTED_ARGUMENT;
valid = false;
}
}
...
private class StringArgumentMarshaler implements ArgumentMarshaler {
private String stringValue = "";
public void set(Iterator<String> currentArgument) throws ArgsException {
try {
stringValue = currentArgument.next();
} catch (NoSuchElementException e) {
errorCode = ArgsException.ErrorCode.MISSING_STRING;
throw new ArgsException();
}
}
public Object get() {
return stringValue;
}
}
private class IntegerArgumentMarshaler implements ArgumentMarshaler {
private int intValue = 0;
public void set(Iterator<String> currentArgument) throws ArgsException {
String parameter = null;
try {
parameter = currentArgument.next();
intValue = Integer.parseInt(parameter);
} catch (NoSuchElementException e) {
errorCode = ArgsException.ErrorCode.MISSING_INTEGER;
throw new ArgsException();
} catch (NumberFormatException e) {
errorParameter = parameter;
errorCode = ArgsException.ErrorCode.INVALID_INTEGER;
throw new ArgsException();
}
}
public Object get() {
return intValue;
}
}
private class DoubleArgumentMarshaler implements ArgumentMarshaler {
private double doubleValue = 0;
public void set(Iterator<String> currentArgument) throws ArgsException {
String parameter = null;
try {
parameter = currentArgument.next();
doubleValue = Double.parseDouble(parameter);
} catch (NoSuchElementException e) {
errorCode = ArgsException.ErrorCode.MISSING_DOUBLE;
throw new ArgsException();
} catch (NumberFormatException e) {
errorParameter = parameter;
errorCode = ArgsException.ErrorCode.INVALID_DOUBLE;
throw new ArgsException();
}
}
public Object get() {
return doubleValue;
}
}
}
Хорошо — теперь Args выдает единственное исключение ArgsException. Выделение ArgsException в отдельный модуль приведет к тому, что большой объем вспомогательного кода обработки ошибок переместится из модуля Args в этот модуль. Это наиболее естественное и очевидное место для размещения этого кода, вдобавок перемещение поможет очистить перерабатываемый модуль Args.
Итак, нам удалось полностью отделить код исключений и ошибок от модуля Args (листинги 14.13–14.16). Для решения этой задачи понадобилось примерно 30 промежуточных шагов, и после каждого шага проверялось прохождение всех тестов.
package com.objectmentor.utilities.args;
import junit.framework.TestCase;
public class ArgsTest extends TestCase {
public void testCreateWithNoSchemaOrArguments() throws Exception {
Args args = new Args("", new String[0]);
assertEquals(0, args.cardinality());
}
public void testWithNoSchemaButWithOneArgument() throws Exception {
try {
new Args("", new String[]{"-x"});
fail();
} catch (ArgsException e) {
assertEquals(ArgsException.ErrorCode.UNEXPECTED_ARGUMENT,
e.getErrorCode());
assertEquals('x', e.getErrorArgumentId());
}
}
public void testWithNoSchemaButWithMultipleArguments() throws Exception {
try {
new Args("", new String[]{"-x", "-y"});
fail();
} catch (ArgsException e) {
assertEquals(ArgsException.ErrorCode.UNEXPECTED_ARGUMENT,
e.getErrorCode());
assertEquals('x', e.getErrorArgumentId());
}
}
public void testNonLetterSchema() throws Exception {
try {
new Args("*", new String[]{});
fail("Args constructor should have thrown exception");
} catch (ArgsException e) {
assertEquals(ArgsException.ErrorCode.INVALID_ARGUMENT_NAME,
e.getErrorCode());
assertEquals('*', e.getErrorArgumentId());
}
}
public void testInvalidArgumentFormat() throws Exception {
try {
new Args("f~", new String[]{});
fail("Args constructor should have throws exception");
} catch (ArgsException e) {
assertEquals(ArgsException.ErrorCode.INVALID_FORMAT, e.getErrorCode());
assertEquals('f', e.getErrorArgumentId());
}
}
public void testSimpleBooleanPresent() throws Exception {
Args args = new Args("x", new String[]{"-x"});
assertEquals(1, args.cardinality());
assertEquals(true, args.getBoolean('x'));
}
public void testSimpleStringPresent() throws Exception {
Args args = new Args("x*", new String[]{"-x", "param"});
assertEquals(1, args.cardinality());
assertTrue(args.has('x'));
assertEquals("param", args.getString('x'));
}
public void testMissingStringArgument() throws Exception {
try {
new Args("x*", new String[]{"-x"});
fail();
} catch (ArgsException e) {
assertEquals(ArgsException.ErrorCode.MISSING_STRING, e.getErrorCode());
assertEquals('x', e.getErrorArgumentId());
}
}
public void testSpacesInFormat() throws Exception {
Args args = new Args("x, y", new String[]{"-xy"});
assertEquals(2, args.cardinality());
assertTrue(args.has('x'));
assertTrue(args.has('y'));
}
public void testSimpleIntPresent() throws Exception {
Args args = new Args("x#", new String[]{"-x", "42"});
assertEquals(1, args.cardinality());
assertTrue(args.has('x'));
assertEquals(42, args.getInt('x'));
}
public void testInvalidInteger() throws Exception {
try {
new Args("x#", new String[]{"-x", "Forty two"});
fail();
} catch (ArgsException e) {
assertEquals(ArgsException.ErrorCode.INVALID_INTEGER, e.getErrorCode());
assertEquals('x', e.getErrorArgumentId());
assertEquals("Forty two", e.getErrorParameter());
}
}
public void testMissingInteger() throws Exception {
try {
new Args("x#", new String[]{"-x"});
fail();
} catch (ArgsException e) {
assertEquals(ArgsException.ErrorCode.MISSING_INTEGER, e.getErrorCode());
assertEquals('x', e.getErrorArgumentId());
}
}
public void testSimpleDoublePresent() throws Exception {
Args args = new Args("x##", new String[]{"-x", "42.3"});
assertEquals(1, args.cardinality());
assertTrue(args.has('x'));
assertEquals(42.3, args.getDouble('x'), .001);
}
public void testInvalidDouble() throws Exception {
try {
new Args("x##", new String[]{"-x", "Forty two"});
fail();
} catch (ArgsException e) {
assertEquals(ArgsException.ErrorCode.INVALID_DOUBLE, e.getErrorCode());
assertEquals('x', e.getErrorArgumentId());
assertEquals("Forty two", e.getErrorParameter());
}
}
public void testMissingDouble() throws Exception {
try {
new Args("x##", new String[]{"-x"});
fail();
} catch (ArgsException e) {
assertEquals(ArgsException.ErrorCode.MISSING_DOUBLE, e.getErrorCode());
assertEquals('x', e.getErrorArgumentId());
}
}
}
public class ArgsExceptionTest extends TestCase {
public void testUnexpectedMessage() throws Exception {
ArgsException e =
new ArgsException(ArgsException.ErrorCode.UNEXPECTED_ARGUMENT,
'x', null);
assertEquals("Argument -x unexpected.", e.errorMessage());
}
public void testMissingStringMessage() throws Exception {
ArgsException e = new ArgsException(ArgsException.ErrorCode.MISSING_STRING,
'x', null);
assertEquals("Could not find string parameter for -x.", e.errorMessage());
}
public void testInvalidIntegerMessage() throws Exception {
ArgsException e =
new ArgsException(ArgsException.ErrorCode.INVALID_INTEGER,
'x', "Forty two");
assertEquals("Argument -x expects an integer but was 'Forty two'.",
e.errorMessage());
}
public void testMissingIntegerMessage() throws Exception {
ArgsException e =
new ArgsException(ArgsException.ErrorCode.MISSING_INTEGER, 'x', null);
assertEquals("Could not find integer parameter for -x.", e.errorMessage());
}
public void testInvalidDoubleMessage() throws Exception {
ArgsException e = new ArgsException(ArgsException.ErrorCode.INVALID_DOUBLE,
'x', "Forty two");
assertEquals("Argument -x expects a double but was 'Forty two'.",
e.errorMessage());
}
public void testMissingDoubleMessage() throws Exception {
ArgsException e = new ArgsException(ArgsException.ErrorCode.MISSING_DOUBLE,
'x', null);
assertEquals("Could not find double parameter for -x.", e.errorMessage());
}
}
public class ArgsException extends Exception {
private char errorArgumentId = '\0';
private String errorParameter = "TILT";
private ErrorCode errorCode = ErrorCode.OK;
public ArgsException() {}
public ArgsException(String message) {super(message);}
public ArgsException(ErrorCode errorCode) {
this.errorCode = errorCode;
}
public ArgsException(ErrorCode errorCode, String errorParameter) {
this.errorCode = errorCode;
this.errorParameter = errorParameter;
}
public ArgsException(ErrorCode errorCode, char errorArgumentId,
String errorParameter) {
this.errorCode = errorCode;
this.errorParameter = errorParameter;
this.errorArgumentId = errorArgumentId;
}
public char getErrorArgumentId() {
return errorArgumentId;
}
public void setErrorArgumentId(char errorArgumentId) {
this.errorArgumentId = errorArgumentId;
}
public String getErrorParameter() {
return errorParameter;
}
public void setErrorParameter(String errorParameter) {
this.errorParameter = errorParameter;
}
public ErrorCode getErrorCode() {
return errorCode;
}
public void setErrorCode(ErrorCode errorCode) {
this.errorCode = errorCode;
}
public String errorMessage() throws Exception {
switch (errorCode) {
case OK:
throw new Exception("TILT: Should not get here.");
case UNEXPECTED_ARGUMENT:
return String.format("Argument -%c unexpected.", errorArgumentId);
case MISSING_STRING:
return String.format("Could not find string parameter for -%c.",
errorArgumentId);
case INVALID_INTEGER:
return String.format("Argument -%c expects an integer but was '%s'.",
errorArgumentId, errorParameter);
case MISSING_INTEGER:
return String.format("Could not find integer parameter for -%c.",
errorArgumentId);
case INVALID_DOUBLE:
return String.format("Argument -%c expects a double but was '%s'.",
errorArgumentId, errorParameter);
case MISSING_DOUBLE:
return String.format("Could not find double parameter for -%c.",
errorArgumentId);
}
return "";
}
public enum ErrorCode {
OK, INVALID_FORMAT, UNEXPECTED_ARGUMENT, INVALID_ARGUMENT_NAME,
MISSING_STRING,
MISSING_INTEGER, INVALID_INTEGER,
MISSING_DOUBLE, INVALID_DOUBLE}
}
public class Args {
private String schema;
private Map<Character, ArgumentMarshaler> marshalers =
new HashMap<Character, ArgumentMarshaler>();
private Set<Character> argsFound = new HashSet<Character>();
private Iterator<String> currentArgument;
private List<String> argsList;
public Args(String schema, String[] args) throws ArgsException {
this.schema = schema;
argsList = Arrays.asList(args);
parse();
}
private void parse() throws ArgsException {
parseSchema();
parseArguments();
}
private boolean parseSchema() throws ArgsException {
for (String element : schema.split(",")) {
if (element.length() > 0) {
parseSchemaElement(element.trim());
}
}
return true;
}
private void parseSchemaElement(String element) throws ArgsException {
char elementId = element.charAt(0);
String elementTail = element.substring(1);
validateSchemaElementId(elementId);
if (elementTail.length() == 0)
marshalers.put(elementId, new BooleanArgumentMarshaler());
else if (elementTail.equals("*"))
marshalers.put(elementId, new StringArgumentMarshaler());
else if (elementTail.equals("#"))
marshalers.put(elementId, new IntegerArgumentMarshaler());
else if (elementTail.equals("##"))
marshalers.put(elementId, new DoubleArgumentMarshaler());
else
throw new ArgsException(ArgsException.ErrorCode.INVALID_FORMAT,
elementId, elementTail);
}
private void validateSchemaElementId(char elementId) throws ArgsException {
if (!Character.isLetter(elementId)) {
throw new ArgsException(ArgsException.ErrorCode.INVALID_ARGUMENT_NAME,
elementId, null);
}
}
private void parseArguments() throws ArgsException {
for (currentArgument = argsList.iterator(); currentArgument.hasNext();) {
String arg = currentArgument.next();
parseArgument(arg);
}
}
private void parseArgument(String arg) throws ArgsException {
if (arg.startsWith("-"))
parseElements(arg);
}
private void parseElements(String arg) throws ArgsException {
for (int i = 1; i < arg.length(); i++)
parseElement(arg.charAt(i));
}
private void parseElement(char argChar) throws ArgsException {
if (setArgument(argChar))
argsFound.add(argChar);
else {
throw new ArgsException(ArgsException.ErrorCode.UNEXPECTED_ARGUMENT,
argChar, null);
}
}
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
if (m == null)
return false;
try {
m.set(currentArgument);
return true;
} catch (ArgsException e) {
e.setErrorArgumentId(argChar);
throw e;
}
}
public int cardinality() {
return argsFound.size();
}
public String usage() {
if (schema.length() > 0)
return "-[" + schema + "]";
else
return "";
}
public boolean getBoolean(char arg) {
ArgumentMarshaler am = marshalers.get(arg);
boolean b = false;
try {
b = am != null && (Boolean) am.get();
} catch (ClassCastException e) {
b = false;
}
return b;
}
public String getString(char arg) {
ArgumentMarshaler am = marshalers.get(arg);
try {
return am == null ? "" : (String) am.get();
} catch (ClassCastException e) {
return "";
}
}
public int getInt(char arg) {
ArgumentMarshaler am = marshalers.get(arg);
try {
return am == null ? 0 : (Integer) am.get();
} catch (Exception e) {
return 0;
}
}
public double getDouble(char arg) {
ArgumentMarshaler am = marshalers.get(arg);
try {
return am == null ? 0 : (Double) am.get();
} catch (Exception e) {
return 0.0;
}
}
public boolean has(char arg) {
return argsFound.contains(arg);
}
}
Основные изменения в классе Args свелись к удалениям. Большая часть кода ушла из Args в ArgsException. Хорошо. Мы также переместили все разновидности ArgumentMarshaller в отдельные файлы. Еще лучше!
Одним из важнейших аспектов хорошей программной архитектуры является логическое разбиение кода — создание подходящих мест для размещения разных кодовых блоков. Разделение ответственности заметно упрощает понимание и сопровождение кода.
Обратите внимание на метод errorMessage класса ArgsException. Очевидно, размещение форматирования сообщения об ошибках нарушает принцип единой ответственности. Класс Args должен заниматься обработкой аргументов, а не форматом сообщений об ошибках. Но насколько логично размещать код форматирования сообщений в ArgsException?
Откровенно говоря, это компромиссное решение. Пользователям, которым не нравится, что сообщения об ошибках поставляет класс ArgsException, придется написать собственную реализацию.
К этому моменту мы уже вплотную подошли к окончательному решению, приведенному в начале этой главы. Завершающие преобразования остаются читателю для самостоятельных упражнений.
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);
}
}
Я провел для ComparisonCompactor анализ покрытия кода на основе этих тестов. В ходе тестирования обеспечивалось 100%-ное покрытие: была выполнена каждая строка кода, каждая команда if и цикл for. Я удостоверился в том, что код работает правильно, а также преисполнился уважения к мастерству его авторов.
Код ComparisonCompactor приведен в листинге 15.2. Не жалейте времени и как следует разберитесь в нем. Вероятно, вы согласитесь с тем, что код достаточно выразителен, обладает логичным разбиением и простой структурой. А когда вы закончите, мы вместе начнем придираться к мелочам.
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);
}
}
Вероятно, вы найдете в этом модуле некоторые недочеты. В нем встречаются длинные выражения, какие-то малопонятные +1 и т.д. Но в целом модуль весьма хорош. В конце концов, он мог бы выглядеть и так, как показано в листинге 15.3.
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;
}
}
Авторы оставили эту модуль в очень хорошей форме. И все же «правило бойскаута[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 в аргументе.
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.
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].
private String compactString(String source) {
return
computeCommonPrefix() +
DELTA_START +
source.substring(prefixLength, source.length() - suffixLength) +
DELTA_END +
computeCommonSuffix();
}
Стало гораздо лучше! Теперь мы видим, что функция compactString просто соединяет фрагменты строки. Вероятно, этот факт можно сделать еще более очевидным. Осталось еще много мелких улучшений, которые можно было бы внести в код. Но я не стану мучить вас подробными описаниями остальных изменений и просто приведу окончательный результат в листинге 15.5.
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 : "");
}
}
Результат выглядит вполне симпатично. Модуль делится на группы: первую группу составляют функции анализа, а вторую — функции синтеза. Функции топологически отсортированы таким образом, что определение каждой функции размещается перед ее первым использованием. Сначала определяются все функции анализа, а за ними следуют функции синтеза.
Внимательно присмотревшись, можно заметить, что я отменил некоторые решения, принятые ранее в этой главе. Например, некоторые извлеченные методы были снова встроены в formatCompactedComparison, а смысл выражения shouldNotBeCompacted снова изменился. Это типичная ситуация. Одна переработка часто приводит к другой, отменяющей первую. Переработка представляет собой итеративный процесс, полный проб и ошибок, но этот процесс неизбежно приводит к формированию кода, достойного настоящего профессионала.
457 if ((result < 1) || (result > 12)) {
result = -1;
458 for (int i = 0; i < monthNames.length; i++) {
459 if (s.equalsIgnoreCase(shortMonthNames[i])) {
460 result = i + 1;
461 break;
462 }
463 if (s.equalsIgnoreCase(monthNames[i])) {
464 result = i + 1;
465 break;
466 }
467 }
468 }
Закомментированный тест в строке 318 выявляет ошибку в методе getFollowingDayOfWeek (строка 672). 25 декабря 2004 года было субботой. Следующей субботой было 1 января 2005 года. Тем не менее при запуске теста getFollowingDayOfWeek утверждает, что первой субботой, предшествующей 25 декабря, было 25 декабря. Разумеется, это неверно [G3],[T1]. Проблема — типичная ошибка граничного условия [T5] — кроется в строке 685. Строка должна читаться следующим образом:
685 if (baseDOW >= targetWeekday) {
Интересно, что проблемы с этой функцией возникали и раньше. Из истории изменений (строка 43) видно, что в функциях getPreviousDayOfWeek, getFollowingDayOfWeek и getNearestDayOfWeek [T6] «исправлялись ошибки».
Модульный тест testGetNearestDayOfWeek (строка 329), проверяющий работу метода getNearestDayOfWeek (строка 705), изначально был не таким длинным и исчерпывающим, как в окончательной версии. Я включил в него много дополнительных тестовых сценариев, потому что не все исходные тесты проходили успешно [T6]. Посмотрите, какие тестовые сценарии были закомментированы — закономерность проявляется достаточно очевидно [T7]. Сбой в алгоритме происходит в том случае, если ближайший день находится в будущем. Очевидно, и здесь происходит какая-то ошибка граничного условия [T5].
Результаты тестового покрытия кода, полученные от Clover, тоже весьма интересны [T8]. Строка 719 никогда не выполняется! Следовательно, условие if в строке 718 всегда ложно. С первого взгляда на код понятно, что это действительно так. Переменная adjust всегда отрицательна, она не может быть больше либо равна 4. Значит, алгоритм попросту неверен.
Правильный алгоритм выглядит так:
int delta = targetDOW - base.getDayOfWeek();
int positiveDelta = delta + 7;
int adjust = positiveDelta % 7;
if (adjust > 3)
adjust -= 7;
return SerialDate.addDays(adjust, base);
Наконец, для прохождения тестов в строках 417 и 429 достаточно инициировать исключение IllegalArgumentException вместо возвращения строки ошибки в функциях weekInMonthToString и relativeToString.
После таких изменений все модульные тесты проходят. Вероятно, класс SerialDate теперь действительно работает. Теперь пришло время «довести его до ума».
public abstract class DayDate implements Comparable,
Serializable {
public static enum Month {
JANUARY(1),
FEBRUARY(2),
MARCH(3),
APRIL(4),
MAY(5),
JUNE(6),
JULY(7),
AUGUST(8),
SEPTEMBER(9),
OCTOBER(10),
NOVEMBER(11),
DECEMBER(12);
Month(int index) {
this.index = index;
}
public static Month make(int monthIndex) {
for (Month m : Month.values()) {
if (m.index == monthIndex)
return m;
}
throw new IllegalArgumentException("Invalid month index " + monthIndex);
}
public final int index;
}
Преобразование MonthConstants в enum инициирует ряд изменений в классе DayDate и всех его пользователях. На внесение всех изменений мне потребовалось около часа. Однако теперь любая функция, прежде получавшая int вместо месяца, теперь получает значение из перечисления Month. Это означает, что мы можем удалить метод isValidMonthCode (строка 326), а также все проверки ошибок кодов месяцев — например, monthCodeToQuarter (строка 356) [G5].
Далее возьмем строку 91, serialVersionUID. Переменная используется для управления сериализацией данных. Если изменить ее, то данные DayDate, записанные старой версией программы, перестанут читаться, а попытки приведут к исключению InvalidClassException. Если вы не объявите переменную serialVersionUID, компилятор автоматически сгенерирует ее за вас, причем значение переменной будет различаться при каждом внесении изменений в модуль. Я знаю, что во всей документации рекомендуется управлять этой переменной вручную, но мне кажется, что автоматическое управление сериализацией надежнее [G4]. В конце концов, я предпочитаю отлаживать исключение InvalidClassException, чем необъяснимое поведение программы в результате того, что я забыл изменить serialVersionUID. Итак, я собираюсь удалить эту переменную — по крайней мере пока.
Комментарий в строке 93 выглядит избыточным. Избыточные комментарии только распространяют лживую и недостоверную информацию [C2]. Соответственно, я удаляю его вместе со всеми аналогами.
В комментариях в строках 97 и 100 упоминаются порядковые номера, о которых говорилось ранее [C1]. Комментарии описывают самую раннюю и самую позднюю дату, представляемую классом DayDate. Их можно сделать более понятными [N1].
public static final int EARLIEST_DATE_ORDINAL = 2; // 1/1/1900
public static final int LATEST_DATE_ORDINAL = 2958465; // 12/31/9999
Мне неясно, почему значение EARLIEST_DATE_ORDINAL равно 2, а не 0. Комментарий в строке 829 подсказывает, что это как-то связано с представлением дат в Microsoft Excel. Более подробное объяснение содержится в производном от DayDate классе с именем SpreadsheetDate (листинг Б.5, с. 428). Комментарий в строке 71 хорошо объясняет суть дела.
Проблема в том, что такой выбор относится к реализации SpreadsheetDate и не имеет ничего общего с DayDate. Из этого я заключаю, что EARLIEST_DATE_ORDINAL и LATEST_DATE_ORDINAL реально не относятся к DayDate и их следует переместить в SpreadsheetDate [G6].
Поиск по коду показывает, что эти переменные используются только в SpreadsheetDate. Они не используются ни в DayDate, ни в других классах JCommon. Соответственно, я перемещаю их в SpreadsheetDate.
Со следующими переменными, MINIMUM_YEAR_SUPPORTED и MAXIMUM_YEAR_SUPPORTED (строки 104 и 107), возникает дилемма. Вроде бы понятно, что если DayDate является абстрактным классом, то он не должен содержать информации о минимальном или максимальном годе. У меня снова возникло искушение переместить эти переменные в SpreadsheetDate [G6]. Тем не менее поиск показал, что эти переменные используются еще в одном классе: RelativeDayOfWeekRule (листинг Б.6, с. 438). В строках 177 и 178 функция getDate проверяет, что в ее аргументе передается действительный год. Дилемма состоит в том, что пользователю абстрактного класса необходима информация о его реализации.
Наша задача — предоставить эту информацию, не загрязняя самого класса DayDate. В общем случае мы могли бы получить данные реализации из экземпляра производного класса, однако функция getDate не получает экземпляр DayDate. С другой стороны, она возвращает такой экземпляр, а это означает, что она его где-то создает. Из строк 187–205 можно заключить, что экземпляр DayDate создается при вызове одной из трех функций: getPreviousDayOfWeek, getNearestDayOfWeek или getFollowingDayOfWeek. Обратившись к листингу DayDate, мы видим, что все эти функции (строки 638–724) возвращают дату, созданную функцией addDays (строка 571), которая вызывает createInstance (строка 808), которая создает SpreadsheetDate! [G7].
В общем случае базовые классы не должны располагать информацией о своих производных классах. Проблема решается применением паттерна АБСТРАКТНАЯ ФАБРИКА [GOF] и созданием класса DayDateFactory. Фабрика создает экземпляры DayDate, а также предоставляет информацию по поводу реализации — в частности, минимальное и максимальное значение даты.
public abstract class DayDateFactory {
private static DayDateFactory factory = new SpreadsheetDateFactory();
public static void setInstance(DayDateFactory factory) {
DayDateFactory.factory = factory;
}
protected abstract DayDate _makeDate(int ordinal);
protected abstract DayDate _makeDate(int day, DayDate.Month month, int year);
protected abstract DayDate _makeDate(int day, int month, int year);
protected abstract DayDate _makeDate(java.util.Date date);
protected abstract int _getMinimumYear();
protected abstract int _getMaximumYear();
public static DayDate makeDate(int ordinal) {
return factory._makeDate(ordinal);
}
public static DayDate makeDate(int day, DayDate.Month month, int year) {
return factory._makeDate(day, month, year);
}
public static DayDate makeDate(int day, int month, int year) {
return factory._makeDate(day, month, year);
}
public static DayDate makeDate(java.util.Date date) {
return factory._makeDate(date);
}
public static int getMinimumYear() {
return factory._getMinimumYear();
}
public static int getMaximumYear() {
return factory._getMaximumYear();
}
}
Фабрика заменяет методы createInstance методами makeDate, в результате чего имена выглядят гораздо лучше [N1]. По умолчанию используется SpreadsheetDateFactory, но этот класс можно в любой момент заменить другой фабрикой. Статические методы, делегирующие выполнение операций абстрактным методам, используют комбинацию паттернов СИНГЛЕТ [GOF], ДЕКОРАТОР [GOF] и АБСТРАКТНАЯ ФАБРИКА.
Класс SpreadsheetDateFactory выглядит так:
public class SpreadsheetDateFactory extends DayDateFactory {
public DayDate _makeDate(int ordinal) {
return new SpreadsheetDate(ordinal);
}
public DayDate _makeDate(int day, DayDate.Month month, int year) {
return new SpreadsheetDate(day, month, year);
}
public DayDate _makeDate(int day, int month, int year) {
return new SpreadsheetDate(day, month, year);
}
public DayDate _makeDate(Date date) {
final GregorianCalendar calendar = new GregorianCalendar();
calendar.setTime(date);
return new SpreadsheetDate(
calendar.get(Calendar.DATE),
DayDate.Month.make(calendar.get(Calendar.MONTH) + 1),
calendar.get(Calendar.YEAR));
}
protected int _getMinimumYear() {
return SpreadsheetDate.MINIMUM_YEAR_SUPPORTED;
}
protected int _getMaximumYear() {
return SpreadsheetDate.MAXIMUM_YEAR_SUPPORTED;
}
}
Как видите, я уже переместил переменные MINIMUM_YEAR_SUPPORTED и MAXIMUM_YEAR_SUPPORTED в класс SpreadsheetDate, в котором им положено находиться [G6].
Следующая проблема DayDate — константы дней, начинающиеся со строки 109. Их следует оформить в виде другого перечисления [J3]. Мы уже видели, как это делается, поэтому я не буду повторяться. При желании посмотрите в итоговом листинге.
Далее мы видим серию таблиц, начинающуюся с LAST_DAY_OF_MONTH в строке 140. Моя первая претензия к этим таблицам состоит в том, что описывающие их комментарии избыточны [C3]. Одних имен вполне достаточно, поэтому я собираюсь удалить комментарии.
Также неясно, почему эта таблица не объявлена приватной [G8], потому что в классе имеется статическая функция lastDayOfMonth, предоставляющая те же данные.
Следующая таблица, AGGREGATE_DAYS_TO_END_OF_MONTH, выглядит загадочно — она ни разу не используется в JCommon [G9]. Я удалил ее.
То же произошло с LEAP_YEAR_AGGREGATE_DAYS_TO_END_OF_MONTH.
Следующая таблица, AGGREGATE_DAYS_TO_END_OF_PRECEDING_MONTH, используется только в SpreadsheetDate (строки 434 и 473). Так почему бы не переместить ее в SpreadsheetDate? Против перемещения говорит тот факт, что таблица не привязана ни к какой конкретной реализации [G6]. С другой стороны, никаких реализаций, кроме SpreadsheetDate, фактически не существует, поэтому таблицу следует переместить ближе к месту ее использования [G10].
Для меня решающим обстоятельством является то, что для обеспечения логической согласованности [G11] таблицу следует объявить приватной и предоставить доступ к ней через функцию вида julianDateOfLastDayOfMonth. Но похоже, такая функция никому не нужна. Более того, если этого потребует новая реализация DayDate, таблицу можно будет легко вернуть на место. Поэтому я ее переместил.
Далее следуют три группы констант, которые можно преобразовать в перечисления (строки 162–205).
Первая из трех групп предназначена для выбора недели в месяце. Я преобразовал ее в перечисление с именем WeekInMonth.
public enum WeekInMonth {
FIRST(1), SECOND(2), THIRD(3), FOURTH(4), LAST(0);
public final int index;
WeekInMonth(int index) {
this.index = index;
}
}
Со второй группой констант (строки 177–187) дело обстоит сложнее. Константы INCLUDE_NONE, INCLUDE_FIRST, INCLUDE_SECOND и INCLUDE_BOTH определяют, должны ли включаться в диапазон конечные даты. В математике в подобных случаях используются термины «открытый интервал», «полуоткрытый интервал» и «замкнутый интервал». Мне кажется, что математические названия выглядят более понятно [N3], поэтому я преобразовал группу в перечисление DateInterval с элементами CLOSED, CLOSED_LEFT, CLOSED_RIGHT и OPEN.
Третья группа констант (строки 18–205) определяет, должно ли в результате поиска конкретного дня