Создание, анализ ирефакторинг
 Скачать 3.16 Mb.
|
|
for (HourlyEmployee e : employees) { addLineItemToPage(e); if (page.size() == PAGE_SIZE) printAndClearItemList(); } if (page.size() > 0) printAndClearItemList(); } private void printAndClearItemList() { formatter.format(page); page.clear(); } private void addLineItemToPage(HourlyEmployee e) { LineItem item = new LineItem(); item.name = e.getName(); item.hours = e.getTenthsWorked() / 10; item.tenths = e.getTenthsWorked() % 10; page.add(item); } public class LineItem { public String name; public int hours; public int tenths; } } У этого кода имеется логическая зависимость, лишенная физического вопло- щения . Удастся ли вам найти ее? Речь идет о константе PAGE_SIZE . Почему класс HourlyReporter должен знать размер страницы? За размер страницы должен от- вечать класс HourlyReportFormatter Факт объявления PAGE_SIZE в HourlyReporter указывает на неверное размещение [G17] . Разработчик полагает, что классу HourlyReporter необходимо знать размер страницы . Такое предположение является логической зависимостью . Работа HourlyReporter зависит от того факта, что HourlyReportFormatter поддерживает раз- мер страницы до 55 . Если какая-то реализация HourlyReportFormatter не сможет работать с такими страницами, произойдет ошибка . 337 338 Глава 17 . Запахи и эвристические правила Чтобы создать физическое представление этой зависимости, мы можем включить в HourlyReportFormatter новый метод с именем getMaxPageSize() . В дальнейшем HourlyReporter вызывает эту функцию вместо того, чтобы использовать константу PAGE_SIZE g23: Используйте полиморфизм вместо if/else или switch/Case Эта рекомендация может показаться странной, если вспомнить главу 6 . Ведь в этой главе говорилось, что в тех частях системы, в которые с большей веро- ятностью будут добавляться новые функции, а не новые типы, команды switch вполне уместны . Во-первых, команды switch чаще всего используются только потому, что они представляют очевидное решение методом «грубой силы», а не самое уместное решение для конкретной ситуации . Таким образом, это эвристическое правило напоминает нам о том, что до применения switch следует рассмотреть возмож- ность применения полиморфизма . Во-вторых, ситуации, в которых состав функций менее стабилен, чем состав типов, встречаются относительно редко . Следовательно, к каждой конструкции switch следует относиться с подозрением . Я использую правило «ОДНОЙ КОМАНДЫ SWITCH»: для каждого типа выбо- ра программа не должна содержать более одной команды switch. Множественные конструкции switch следует заменять полиморфными объектами. g24: Соблюдайте стандартные конвенции Все рабочие группы должны соблюдать единые стандарты ко дирования, основан- ные на отраслевых нормах . Стандарт кодирования опреде ляет, где объявляются переменные экземпляров; как присваиваются имена классов, методов и пере- менных; где размещаются фигурные скобки и т . д . Документ с явным описанием этих правил не нужен — сам код служит примером оформления . Правила должны соблюдаться всеми участниками группы . Это означает, что каждый участник группы должен быть достаточно разумным, чтобы понимать: неважно, как именно размещаются фигурные скобки, если только все согласились размещать их одинаковым образом . Если вам захочется узнать, какие конвенции оформления использую я, обрати- тесь к переработанному коду в листингах Б .7 (с . 442) — Б .14 . 338 Разное 339 g25: Заменяйте «волшебные числа» именованными константами Вероятно, это одно из самых древних правил разработки . Помню, оно встречалось мне еще в 60-х годах, в учебниках COBOL, FORTRAN и PL/1 для начинающих . В общем случае присутствие «сырых» чисел в коде нежелательно . Числа следует скрыть в константах с содержательными именами . Например, число 86,400 следует скрыть в константе SECONDS_PER_DAY . Если в стра- нице отчета выводится 55 строк, число 55 следует скрыть в константе LINES_PER_ PAGE Некоторые числа так легко узнаются, что их не обязательно скрывать за име- нованными константами — при условии, что они используются в сочетании с предельно ясным кодом . Пример: double milesWalked = feetWalked/5280.0; int dailyPay = hourlyRate * 8; double circumference = radius * Math.PI * 2; Нужны ли константы FEET_PER_MILE , WORK_HOURS_PER_DAY и TWO в этих примерах? Разумеется, последний случай выглядит особенно абсурдно . В некоторых фор- мулах константы попросту лучше воспринимаются в числовой записи . По поводу WORK_HOURS_PER_DAY можно спорить, потому что законы и нормативы могут изме- няться . С другой стороны, формула с числом 8 читается настолько удобно, что мне просто не хочется нагружать читателя кода лишними 17 символами . А число 5280 — количество футов в миле — настолько хорошо известно и уникально, что читатель сразу узнает его, даже если оно будет располагаться вне какого-либо контекста . Такие константы, как 3 .141592653589793, тоже хорошо известны и легко узнавае- мы . Однако вероятность ошибки слишком велика, чтобы оставлять их в числовой форме . Встречая значение 3 .1415927535890793, вы сразу догадываетесь, что перед вами число π, и не проверяете его (а вы заметили ошибку в одной цифре?) . Также мы не хотим, чтобы в программах использовались сокращения 3 .14, 3 .14159, 3 .142 и т . д . К счастью, значение Math.PI уже определено за нас . Термин «волшебное число» относится не только к числам . Он распространяется на все лексемы, значения которых не являются самодокументирующими . При- мер: assertEquals(7777, Employee.find("John Doe").employeeNumber()); В этом проверочном условии задействованы два «волшебных числа» . Очевидно, первое — 7777, хотя его смысл далеко не так очевиден . Второе «волшебное чис- ло» — строка " John Doe " . Ее смысл тоже выглядит весьма загадочно . Оказывается, " John Doe " — имя работника с табельным номером 7777 в тестовой базе данных, созданной нашей группой . Все участники группы знают, как под- 339 340 Глава 17 . Запахи и эвристические правила ключаться к этой базе данных . В базе уже хранятся тестовые записи с заранее известными значениями и атрибутами . Также выясняется, что " John Doe " — един- ственный работник с почасовой оплатой в тестовой базе данных . Следовательно, эта проверка должна выглядеть так: assertEquals( HOURLY_EMPLOYEE_ID, Employee.find(HOURLY_EMPLOYEE_NAME).employeeNumber()); g26: Будьте точны Наивно ожидать, что первая запись, возвращаемая по запросу, является единственной . Использовать числа c плавающей точкой для представления де нежных сумм — почти преступление . Отсутствие блокировок и/или управ- ления транз акциями только потому, что вы думаете, что одновременное обнов- ление маловероятно — в лучшем случае халатность . Объявление переменной с типом ArrayList там, где более уместен тип List — чрезмерное ограничение . Объявление всех переменных защищенными по умолчанию — недостаточное ограничение . Принимая решение в своем коде, убедитесь в том, что вы действуете предельно точно и аккуратно . Знайте, почему принимается решение, и как вы собираетесь поступать с исключениями из правила . Не ленитесь обеспечивать точность своих решений . Если вы решили вызвать функцию, которая может вернуть null — про- верьте возвращаемое значение . Если вы запрашиваете из базы данных запись, которая, по вашему мнению, является единственной — проверьте, не вернул ли запрос дополнительные записи . Если вам нужно работать с денежными суммами, используйте целые числа и округляйте результат по действующим правилам . Если в программе существует возможность одновременного объявления, реали- зуйте ту или иную разновидность блокировки . Неоднозначности и неточности в коде объясняются либо недопониманием, либо ленью . В любом случае от них следует избавиться . g27: Структура важнее конвенций Воплощайте архитектурные решения на уровне структуры кода; она важнее стандартов и конвенций . Содержательные имена полезны, но структура, застав- ляющая пользователя соблюдать установленные правила, важнее . Например, конструкции switch / case с хорошо выбранными именами элементов перечисле- ния уступают базовым классам с абстрактными методами . Ничто не вынуждает пользователя применять одинаковую реализацию switch / case во всех случаях; с другой стороны, базовые классы заставляют его реализовать все абстрактные методы в конкретных классах . 340 Разное 341 g28: Инкапсулируйте условные конструкции В булевской логике достаточно трудно разобраться и вне контекста команд if или while . Выделите в программе функции, объясняющие намерения условной конструкции . Например, команда if (shouldBeDeleted(timer)) выразительнее команды if (timer.hasExpired() && !timer.isRecurrent()) g29: Избегайте отрицательных условий Отрицательные условия немного сложнее для понимания, чем положительные . Таким образом, по возможности старайтесь формулировать положительные условия . Например, запись if (buffer.shouldCompact()) предпочтительнее записи if (!buffer.shouldNotCompact()) g30: Функции должны выполнять одну операцию Часто возникает искушение разделить свою функцию на несколько секций для выполнения разных операций . Такие функции выполняют несколько операций; их следует преобразовать в группу меньших функций, каждая из которых вы- полняет только одну операцию . Пример: public void pay() { for (Employee e : employees) { if (e.isPayday()) { Money pay = e.calculatePay(); e.deliverPay(pay); } } } Эта функция выполняет сразу три операции: она перебирает всех работников; проверяет, начислены ли работнику какие-то выплаты; и наконец, производит оплату . Код лучше записать в следующем виде: public void pay() { for (Employee e : employees) payIfNecessary(e); } private void payIfNecessary(Employee e) { if (e.isPayday()) calculateAndDeliverPay(e); } 341 342 Глава 17 . Запахи и эвристические правила private void calculateAndDeliverPay(Employee e) { Money pay = e.calculatePay(); e.deliverPay(pay); } Каждая из этих функций выполняет только одну операцию (см . «Правило одной операции», с . 59) . g31: Скрытые временн *ые привязки Временн *ые привязки часто необходимы, но они не должны скрываться . Струк- тура аргументов функций должна быть такой, чтобы последовательность вызова была абсолютно очевидной . Рассмотрим следующий пример: public class MoogDiver { Gradient gradient; List public void dive(String reason) { saturateGradient(); reticulateSplines(); diveForMoog(reason); } } Порядок вызова трех функций важен . Сначала вызывается saturateGradient() , затем reticulateSplines() и только после этого diveForMoog() . К сожалению, код не обеспечивает принудительного соблюдения временной привязки . Ничто не мешает другому программисту вызвать reticulateSplines до saturateGradient , и все кончится исключением UnsaturatedGradientException Более правильное решение выглядит так: public class MoogDiver { Gradient gradient; List public void dive(String reason) { Gradient gradient = saturateGradient(); List diveForMoog(splines, reason); } } Временная привязка реализуется посредством создания «эстафеты» . Каждая функция выдает результат, необходимый для работы следующей функции, и вы- звать эти функции с нарушением порядка сколько-нибудь разумным способом уже не удастся . Пожалуй, кто-то сочтет, что это увеличивает сложность функций, и это действи- тельно так . Однако дополнительные синтаксические сложности лишь выявляют 342 Разное 343 реальную сложность ситуации, обусловленную необходимостью согласования по времени . Обратите внимание: переменные экземпляров остались на своих местах . Пред- полагается, что они используются приватными методами класса . Использование их в аргументах лишь явно выражает факт существования временной привязки . g32: Структура кода должна быть обоснована Структура кода должна выбираться не произвольно, а по строго определенным причинам . Позаботьтесь о том, чтобы эти причины были выражены в структуре кода . Если при чтении кода создается впечатление, что его структура выбрана произвольно, другим пользователям может показаться, что ее можно изменить . Если во всей системе последовательно используется единая структура кода, другие пользователи примут ее и сохранят действующие правила . Например, недавно я занимался объединением изменений в FitNesse и обнаружил, что один из наших авторов использовал следующую запись: public class AliasLinkWidget extends ParentWidget { public static class VariableExpandingWidgetRoot { } Проблема в том, что VariableExpandingWidgetRoot незачем находиться в облсти видимости AliasLinkWidget . Более того, класс AliasLinkWidget VariableExpanding- WidgetRoot использовался посторонними классами, которые не имели никакого отношения к AliasLinkWidget Возможно, программист разместил VariableExpandingWidgetRoot в AliasWidget по со- ображениям удобства, а может, он действительно полагал, что область видимости этого класса должна находиться внутри области видимости AliasWidget . Какими бы причинами он ни руководствовался, результат выглядит необоснованным . Открытые классы, не являющиеся вспомогательными по отношению к другому классу (то есть используемыми только в его внутренних операциях), не должны размещаться внутри других классов . По стандартным правилам такие классы объявляются на верхнем уровне своих пакетов . g33: Инкапсулируйте граничные условия Отслеживать граничные условия нелегко . Разместите их обработку в одном ме- сте . Не позволяйте им «растекаться» по всему коду . Не допускайте, чтобы в вашей программе кишели многочисленные +1 и –1 . Возьмем простой пример из FIT: if(level + 1 < tags.length) { parts = new Parse(body, tags, level + 1, offset + endTag); 343 344 Глава 17 . Запахи и эвристические правила body = null; } Обратите внимание: level+1 здесь встречается дважды . Это граничное усло- вие, которое следует инкапсулировать в переменной — например, с именем nextLevel : int nextLevel = level + 1; if(nextLevel < tags.length) { parts = new Parse(body, tags, nextLevel, offset + endTag); body = null; } g34: Функции должны быть написаны на одном уровне абстракции Все команды функции должны быть сформулированы на одном уровне абстрак- ции, который расположен одним уровнем ниже операции, описываемой именем функции . Возможно, это эвристическое правило сложнее всего правильно ин- терпретировать и соблюдать . Идея достаточно тривиальна, но люди слишком хорошо справляются со смешением разных уровней абстракции . Для примера возьмем следующий код из FitNesse: public String render() throws Exception { StringBuffer html = new StringBuffer(" |