Создание, анализ ирефакторинг
 Скачать 3.16 Mb.
|
|
312 Глава 16 . Переработка SerialDate Первая из трех групп предназначена для выбора недели в месяце . Я преобразовал ее в перечисление с именем 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) определяет, должно ли в результа- те по иска конкретного дня недели возвращаться последнее, предыдущее или ближайшее вхождение . Выбрать подходящее имя для такого перечисления не просто . В итоге я остановился на имени WeekdayRange с элементами LAST , NEXT и NEAREST Возможно, вы не согласитесь с выбранными мной именами . Мне они кажутся логичными, но у вас может быть свое мнение . Однако сейчас константы при- ведены к форме, которая позволяет легко изменить их в случае необходимости [J3] . Они передаются не в виде целых чисел, а в виде символических имен . Я могу воспользоваться функцией переименования своей рабочей среды для изменения имен или типов, не беспокоясь о том, что я пропустил где-то в коде -1 или 2 или объявление аргумента int осталось плохо описанным . Поле description в строке 208 нигде не используется . Я удалил его вместе с ме- тодами доступа [G9] . Также был удален вырожденный конструктор по умолчанию в строке 213 [G12] . Компилятор сгенерирует его за нас . Метод isValidWeekdayCode (строки 216–238) пропускаем — мы удалили его при создании перечисления Day Мы подходим к методу stringToWeekdayCode (строки 242–270) . Комментарии Javadoc, не добавляющие полезной информации к сигнатуре метода, только загромождают код [C3],[G12] . В комментарии есть всего один содержатель- ный момент — он описывает возвращаемое значение -1 . Но после перехода на пе речисление Day этот комментарий стал неверным [C2] . Сейчас метод сообща- ет об ошибке, выдавая исключение IllegalArgumentException . Я удалил коммен- тарий . 312 …Потом очистить код 313 Также я удалил все ключевые слова final в объявлениях аргументов и переменных . На мой взгляд, реальной пользы от них не было, а программу они загромождают [G12] . Удаление final противоречит мнению некоторых экспертов . Например, Роберт Симмонс (Robert Simmons) [Simmons04, p . 73] настоятельно рекомендует «…почаще вставлять final в своем коде» . Разумеется, я с этим не согласен . У final имеются свои полезные применения (например, при объявлении отдельных кон- стант), но в остальных случаях это ключевое слово не приносит реальной поль- зы . Возможно, я так считаю еще и потому, что типичные ошибки, выявляемые при помощи final , уже выявляются написанными мной модульными тестами . Мне не понравились повторяющиеся команды if [G5] внутри цикла for (строки 259 и 263); они были объединены в одну команду if при помощи оператора || Также я использовал перечисление Day для управления циклом for и внес ряд других косметических изменений . Мне пришло в голову, что метод в действительности не принадлежит DayDate Фактически это функция разбора Day , поэтому я переместил ее в перечисление Day . Но после этого перечисление Day стало занимать довольно много места . По- скольку концепция Day не зависит от DayDate , я вывел перечисление Day из класса DayDate в собственный исходный файл [G13] . Кроме того, я переместил следующую функцию weekdayCodeToString (строки 272– 286) в перечисление Day и переименовал ее в toString public enum Day { MONDAY(Calendar.MONDAY), TUESDAY(Calendar.TUESDAY), WEDNESDAY(Calendar.WEDNESDAY),s THURSDAY(Calendar.THURSDAY), FRIDAY(Calendar.FRIDAY), SATURDAY(Calendar.SATURDAY), SUNDAY(Calendar.SUNDAY); public final int index; private static DateFormatSymbols dateSymbols = new DateFormatSymbols(); Day(int day) { index = day; } public static Day make(int index) throws IllegalArgumentException { for (Day d : Day.values()) if (d.index == index) return d; throw new IllegalArgumentException( String.format("Illegal day index: %d.", index)); } public static Day parse(String s) throws IllegalArgumentException { String[] shortWeekdayNames = 313 314 Глава 16 . Переработка SerialDate dateSymbols.getShortWeekdays(); String[] weekDayNames = dateSymbols.getWeekdays(); s = s.trim(); for (Day day : Day.values()) { if (s.equalsIgnoreCase(shortWeekdayNames[day.index]) || s.equalsIgnoreCase(weekDayNames[day.index])) { return day; } } throw new IllegalArgumentException( String.format("%s is not a valid weekday string", s)); } public String toString() { return dateSymbols.getWeekdays()[index]; } } В программе две функции getMonths (строки 288–316); первая функция вызы- вает вторую . Вторая функция не вызывается никем, кроме первой функцией . Я свернул две функции в одну, что привело к значительному упрощению кода [G9],[G12],[F4] . В завершение я переименовал итоговую функцию, присвоив ей более содержательное имя [N1] . public static String[] getMonthNames() { return dateFormatSymbols.getMonths(); } Функция isValidMonthCode (строки 326–346) потеряла актуальность после введе- ния перечисления Month , поэтому я ее удалил [G9] . Функция monthCodeToQuarter (строки 356–375) отдает ФУНКЦИОНАЛЬНОЙ ЗАВИСТЬЮ [Refactoring]; вероятно, ее логичнее включить в перечисление Month в виде метода с именем quarter . Я выполнил замену . public int quarter() { return 1 + (index-1)/3; } В результате перечисление Month стало достаточно большим для выделения в от- дельный класс . Я убрал его из DayDate по образцу перечисления Day [G11],[G13] . Следующие два метода называются monthCodeToString (строки 377–426) . И сно- ва мы видим, как один метод вызывает своего «двойника» с передачей флага . Обычно передавать флаг в аргументе не рекомендуется, особенно если он просто выбирает формат вывода [G15] . Я переименовал, упростил и реструктурировал эти функции и переместил их в перечисление Month [N1],[N3],[C3],[G14] . public String toString() { return dateFormatSymbols.getMonths()[index - 1]; } 314 …Потом очистить код 315 public String toShortString() { return dateFormatSymbols.getShortMonths()[index - 1]; } Далее в листинге идет метод stringToMonthCode (строки 428–472) . Я пере именовал его, переместил в перечисление Month и упростил [N1],[N3],[C3],[G14],[G12] . public static Month parse(String s) { s = s.trim(); for (Month m : Month.values()) if (m.matches(s)) return m; try { return make(Integer.parseInt(s)); } catch (NumberFormatException e) {} throw new IllegalArgumentException("Invalid month " + s); } private boolean matches(String s) { return s.equalsIgnoreCase(toString()) || s.equalsIgnoreCase(toShortString()); } Метод isLeapYear (строки 495–517) можно сделать более выразительным [G16] . public static boolean isLeapYear(int year) { boolean fourth = year % 4 == 0; boolean hundredth = year % 100 == 0; boolean fourHundredth = year % 400 == 0; return fourth && (!hundredth || fourHundredth); } Следующая функция, leapYearCount (строки 519–536), не принадлежит DayDate Она не вызывается никем, кроме двух методов SpreadsheetDate . Я переместил ее в производный класс [G6] . Функция lastDayOfMonth (строки 538–560) использует массив LAST_DAY_OF_MONTH Этот массив принадлежит перечислению Month [G17], поэтому функция бы- ла перемещена . Заодно я упростил ее код и сделал его более выразительным [G16] . public static int lastDayOfMonth(Month month, int year) { if (month == Month.FEBRUARY && isLeapYear(year)) return month.lastDay() + 1; else return month.lastDay(); } Начинается самое интересное . Далее в листинге идет функция addDays (строки 562–576) . Прежде всего, поскольку эта функция работает с переменными DayDate , она не должна быть статической [G18] . Соответственно, я преобразовал ее в ме- 315 316 Глава 16 . Переработка SerialDate тод экземпляра . Также она вызывает функцию toSerial , которую правильнее называть toOrdinal [N1] . Наконец, метод можно несколько упростить . public DayDate addDays(int days) { return DayDateFactory.makeDate(toOrdinal() + days); } Сказанное относится и к функции addMonths (строки 578–602) . Она должна быть оформлена в виде метода экземпляра [G18] . Алгоритм относительно сложен, поэтому я воспользовался ПОЯСНИТЕЛЬНЫМИ ВРЕМЕННЫМИ ПЕРЕ- МЕННЫМИ [Beck97] [G19], чтобы сделать его смысл более прозрачным . Заодно метод getYYY был переименован в getYear [N1] . public DayDate addMonths(int months) { int thisMonthAsOrdinal = 12 * getYear() + getMonth().index - 1; int resultMonthAsOrdinal = thisMonthAsOrdinal + months; int resultYear = resultMonthAsOrdinal / 12; Month resultMonth = Month.make(resultMonthAsOrdinal % 12 + 1); int lastDayOfResultMonth = lastDayOfMonth(resultMonth, resultYear); int resultDay = Math.min(getDayOfMonth(), lastDayOfResultMonth); return DayDateFactory.makeDate(resultDay, resultMonth, resultYear); } Функция addYears (строки 604–626) преобразуется по тем же принципам, что и ее аналоги . public DayDate plusYears(int years) { int resultYear = getYear() + years; int lastDayOfMonthInResultYear = lastDayOfMonth(getMonth(), resultYear); int resultDay = Math.min(getDayOfMonth(), lastDayOfMonthInResultYear); return DayDateFactory.makeDate(resultDay, getMonth(), resultYear); } Преобразование статических методов в методы экземпляров вызвало у меня не- которое беспокойство . Поймет ли читатель при виде выражения date.addDays(5) , что объект date не изменяется, а вместо этого возвращается новый экземпляр DayDate ? Или он ошибочно решит, что к объекту date прибавляются пять дней? Казалось бы, проблема не столь серьезна, но конструкции вроде следующей могут оказаться очень коварными [G20] . DayDate date = DateFactory.makeDate(5, Month.DECEMBER, 1952); date.addDays(7); // Смещение date на одну неделю. Скорее всего, читатель кода предположит, что вызов addDays изменяет объект date . Значит, нам понадобится имя, разрушающее эту двусмысленность [N4] . Я переименовал методы в plusDays и plusMonths . Мне кажется, что предназначение данного метода отлично отражается конструкциями вида DayDate date = oldDate.plusDays(5); С другой стороны, следующая конструкция читается недостаточно бегло, чтобы читатель сразу предположил, что изменяется объект date : date.plusDays(5); 316 …Потом очистить код 317 Алгоритмы становятся все интереснее . Функция getPreviousDayOfWeek (стро- ки 628–660) работает, но выглядит слишком сложно . После некоторых размыш- лений относительно того, что же в действительности происходит в этой функции [G21], мне удалось упростить ее и воспользоваться ПОЯСНИТЕЛЬНЫМИ ВРЕМЕННЫМИ ПЕРЕМЕННЫМИ [G19], чтобы сделать код более понятным . Я также преобразовал статический метод в метод экземпляра [G18] и избавился от дублирующего метода экземпляра [G5] (строки 997–1008) . public DayDate getPreviousDayOfWeek(Day targetDayOfWeek) { int offsetToTarget = targetDayOfWeek.index - getDayOfWeek().index; if (offsetToTarget >= 0) offsetToTarget -= 7; return plusDays(offsetToTarget); } Абсолютно такой же анализ с тем же результатом был проведен для метода getFollowingDayOfWeek (строки 662–693) . public DayDate getFollowingDayOfWeek(Day targetDayOfWeek) { int offsetToTarget = targetDayOfWeek.index - getDayOfWeek().index; if (offsetToTarget <= 0) offsetToTarget += 7; return plusDays(offsetToTarget); } Далее идет функция getNearestDayOfWeek (строки 695–726), которую мы исправ- ляли на с . 309 . Внесенные тогда изменения не соответствуют тому шаблону, по которому были преобразованы две последние функции [G11] . Я преобразовал функцию по тем же правилам, а также воспользовался ПОЯСНИТЕЛЬНЫМИ ВРЕМЕННЫМИ ПЕРЕМЕННЫМИ [G19] для разъяснения алгоритма . public DayDate getNearestDayOfWeek(final Day targetDay) { int offsetToThisWeeksTarget = targetDay.index - getDayOfWeek().index; int offsetToFutureTarget = (offsetToThisWeeksTarget + 7) % 7; int offsetToPreviousTarget = offsetToFutureTarget - 7; if (offsetToFutureTarget > 3) return plusDays(offsetToPreviousTarget); else return plusDays(offsetToFutureTarget); } Метод getEndOfCurrentMonth (строки 728–740) выглядит немного странно — перед нами метод экземпляра, который «завидует» [G14] собственному классу, получая аргумент DayDate . Я преобразовал его в полноценный метод экземпляра, а также заменил несколько имен более содержательными . public DayDate getEndOfMonth() { Month month = getMonth(); int year = getYear(); 317 318 Глава 16 . Переработка SerialDate int lastDay = lastDayOfMonth(month, year); return DayDateFactory.makeDate(lastDay, month, year); } Переработка weekInMonthToString (строки 742–761) оказалась очень интересным делом . Используя средства рефакторинга своей IDE, я сначала переместил метод в перечисление WeekInMonth , созданное ранее на с . 312 . Затем я переименовал его в toString и преобразовал из статического метода в метод экземпляра . Все тесты прошли успешно . (Догадываетесь, к чему я клоню?) Затем я полностью удалил метод! Пять проверок завершились неудачей (строки 411–415, листинг Б .4, с . 417) . Я изменил эти строки так, чтобы в них исполь- зовались имена из перечисления ( FIRST , SECOND , . . .) . И все тесты прошли . А вы догадываетесь, почему? И понимаете ли вы, почему каждый из этих шагов был необходим? Функция рефакторинга проследила за тем, чтобы все предыдущие вызовы weekInMonthToString были заменены вызовами toString для перечисления weekInMonth , а во всех перечислениях реализация toString просто возвращает имена… К сожалению, мои ухищрения ни к чему не привели . Как бы элегантно ни вы- глядела эта замечательная цепочка рефакторинга, в итоге я понял, что единствен- ными пользователями этой функции были тесты, которые я только что изменил . И я удалил тесты . Стыдно дважды наступать на одни грабли! Определив, что функция relative- ToString (строки 765–781) не вызывается нигде, кроме тестов, я просто удалил функцию вместе с ее тестами . Наконец-то мы добрались до абстрактных методов абстрактного класса . Пер- вый метод выглядит знакомо: toSerial (строки 838–844) . На с . 316 я присвоил ему имя toOrdinal . Рассматривая его в новом контексте, я решил, что его лучше переименовать в getOrdinalDay Следующий абстрактный метод, toDate (строки 838–844), преобразует DayDate в java.util.Date . Почему метод объявлен абстрактным? Присмотревшись к его реализации в SpreadsheetDate (строки 198–207, листинг Б .5, с . 428), мы видим, что он не зависит ни от каких подробностей реализации класса [G6] . Поэтому я поднял его на более высокий уровень абстракции . Методы getYYYY , getMonth и getDayOfMonth абстрактны . Метод getDayOfWeek — еще один метод, который следовало бы извлечь из SpreadSheetDate — тоже не зависит от DayDate . Или все-таки зависит? Присмотревшись внимательно (строка 247, листинг Б .5, с . 428), мы видим, что алгоритм неявно зависит от «точки отсче- та» дней недели (иначе говоря, от того, какой день недели считается днем 0) . Таким образом, хотя функция не имеет физических зависимостей, которые нельзя было бы переместить в DayDate , у нее имеются логические зависимости . Подобные логические зависимости беспокоят меня [G22] . Если что-то зависит от реализации на логическом уровне, то что-то должно зависеть и на физическом уровне . Кроме того, мне кажется, что сам алгоритм можно было бы сделать более 318 …Потом очистить код 319 универсальным, чтобы существенно меньшая его часть зависела от реализа- ции [G6] . Я создал в DayDate абстрактный метод с именем getDayOfWeekForOrdinalZero и реа- лизовал его в SpreadsheetDate так, чтобы он возвращал Day.SATURDAY . Затем я пере- местил метод getDayOfWeek наверх по цепочке в DayDate и изменил его так, чтобы в нем вызывались методы getOrdinalDay и getDayOfWeekForOrdinalZero public Day getDayOfWeek() { Day startingDay = getDayOfWeekForOrdinalZero(); int startingOffset = startingDay.index - Day.SUNDAY.index; return Day.make((getOrdinalDay() + startingOffset) % 7 + 1); } Заодно присмотритесь к комментарию в строках с 895 по 899 . Так ли необходимо это повторение? Как и в предыдущих случаях, я удалил этот комментарий вместе со всеми остальными . Переходим к следующему методу compare (строки 902–913) . Уровень абстракции этого метода снова выбран неправильно [G6], поэтому я поднял его реализацию в DayDate . Кроме того, его имя недостаточно содержательно [N1] . В действитель- ности этот метод возвращает промежуток в днях, начиная с аргумента, поэтому я переименовал его в daysSince . Также я заметил, что для этого метода нет ни одного теста, и написал их . Следующие шесть функций (строки 915–980) представляют собой абстрактные методы, которые должны реализовываться в DayDate . Я извлек из SpreadsheetDate Последнюю функцию isInRange (строки 982–995) также необходимо извлечь и переработать . Команда switch выглядит некрасиво [G23]; ее можно заменить, переместив условия в перечисление DateInterval public enum DateInterval { OPEN { public boolean isIn(int d, int left, int right) { return d > left && d < right; } }, CLOSED_LEFT { public boolean isIn(int d, int left, int right) { return d >= left && d < right; } }, CLOSED_RIGHT { public boolean isIn(int d, int left, int right) { return d > left && d <= right; } }, CLOSED { public boolean isIn(int d, int left, int right) { return d >= left && d <= right; } }; 319 |