Чистый код. Создание, анализ и рефакторинг — страница 58 из 94

Класс 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) определяет, должно ли в результате поиска конкретного дня недели возвращаться последнее, предыдущее или ближайшее вхождение. Выбрать подходящее имя для такого перечисления непросто. В итоге я остановился на имени 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. Я удалил комментарий.

Также я удалил все ключевые слова 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 =

      dateSymbols.getShortWeekdays();

    String[] weekDayNames =

      dateSymbols.getWeekdays();


    s = s.trim();

    for (Day day : Day.values()) {

      if (s.equalsIgnoreCase(shortWeekdayNames[day.index]) ||