[개발 도서] Clean Code :: 16장 - SerialDate 리팩터링

Jihyoung·2021년 8월 14일
0

Clean Code

목록 보기
10/11
post-thumbnail
post-custom-banner

📕 개요

http://www.jfree.org/jcommon/ 에서 제공하는 JCommon 라이브러리 중 org.jfree.date라는 패키지 속에 위치한 SerialDate라는 클래스에 대해 이야기 한다.

SerialDate는 날짜를 표현하는 자바 클래스다. 이미 자바에서는 다양한 클래스를 제공하지만, 하루 중 시각, 시간대에 무관하게 특정 날짜를 표기하기 위해, 즉 시간 기반 날짜 클래스가 아닌 순수 날짜 클래스를 만들고자 SerialDate를 만들었다고 한다.

/**
*  An abstract class that defines our requirements for manipulating dates,
*  without tying down a particular implementation.
*  <P>
*  Requirement 1 : match at least what Excel does for dates;
*  Requirement 2 : the date represented by the class is immutable;
*  <P>
*  Why not just use java.util.Date?  We will, when it makes sense.  At times,
*  java.util.Date can be *too* precise - it represents an instant in time,
*  accurate to 1/1000th of a second (with the date itself depending on the
*  time-zone).  Sometimes we just want to represent a particular day (e.g. 21
*  January 2015) without concerning ourselves about the time of day, or the
*  time-zone, or anything else.  That's what we've defined SerialDate for.
*  <P>
*  You can call getInstance() to get a concrete subclass of SerialDate,
*  without worrying about the exact implementation.
*
* @author David Gilbert
*/

📗 첫째, 돌려보자

SerialDateTests 라는 클래스는 단위 테스트 케이스 몇개를 포함한다. 해당 클래스를 실행하면 실패하는 경우는 없다. 하지만 코드를 살펴보면 모든 경우를 점검하지 않았다는 사실을 알 수 있다.
테스트 케이스가 확인하지 않는 조건이 있다면 해당 테스트는 불완전하다.
여기서는 생성한 함수를 호출하지 않는 경우가 생긴다.

그래서 클로버를 이용해 실행 코드와 실행하지 않는 코드를 확인한 결과 SerialDateTests는 50%의 경우만 테스트함을 확인할 수 있었다.

* 클로버 : 코드 커버리지 분석 도구

따라서 더 높은 코드 커버리지를 위해 단위 테스트 케이스를 구현하고, 해당 단위 테스트에서 실패한 경우(주석으로 처리한 부분)를 성공시킬 수 있도록 원본 클래스를 리팩터링한다.

    1. BobsSerialDateTest.java의 23행에서 63행 까지 주석

      todo assertEquals(MONDAY, stringToWeekdayCode("monday"));
      assertEquals(MONDAY, stringToWeekdayCode("MONDAY"));
    2. BobsSerialDateTest.java의 43행과 45행 주석

      • 설계 상으로는 통과하지 않는게 맞더라도, 당연히 통과해야 한다고 여겨지는 부분
      • 당연한 동작을 구현하지 않으면 코드를 읽거나 사용하는 사람이 함수 이름만으로 기능을 직관적으로 예상하기 어렵다.
      • 저자는 대소문자 구분없이 모두 통과해야한다고 여김 (1)
      • 'tues'와 'thurs'라는 약어 지원여부 불분명 (2)

  • SerialDate.java의 259행과 263행의 equals를 equalsIgnoreCase로 바꾸는 것으로 간단히 테스트 케이스를 통과하도록 수정
    • 사소한 테스트를 건너뛰면 안된다.

  • 163행에서 213행에 나오는 테스트
    • 대소문자, 약어 구분없이 월 구분하게 해주기

위를 고려하여 기존 stringTomonthCode 메서드를 아래와 같이 고친다

if  ((result < 1) || result > 12) {
    result = -1;
    for (int i = 0; i < maonthNames.length; i++) {
        if (s.equalsIgnoreCase(shortMonthNames[i])) {
            result = i + 1;
            break;
        }
        if (s.equalsIgnoreCase(monthNames[i])) {
            result = i + 1;
            break;
        }
    }
}

그리고 아래 테스트 케이스는 SerialDate의 getFollowingDayOfWeek 메서드에 버그를 드러낸다. 2004년 12월 25일은 토요일이었으므로 다음 토요일은 2005년 1월1일이다. 하지만 테스트를 돌려보면 12월 25일을 다음 토요일로 반환하는 버그를 보여준다.

assertEquals(d(1, JANUARY, 2005), getFollowingDayOfWeek(SATURDAY, d(25, DECEMBER, 2004)));

이는 경계 조건 오류이므로, 아래와 같이 메서드를 수정해준다.

if (baseDow >= targetWeekday) { //if (baseDOW > targetWeekday) {

이 부분은 이전에 수정을 거친 코드다.

 * 12-Nov-2001 : IBD requires setDescription() method, now that NotableDate 
 *               class is gone (DG);  Changed getPreviousDayOfWeek(), 
 *               getFollowingDayOfWeek() and getNearestDayOfWeek() to correct 
 *               bugs (DG);

버그는 서로 모이는 경향이 있기 때문에 해당 함수를 철저하게 테스트 할 필요가 있다.

또한, 잘 짜여진 테스트 케이스는 실패 패턴이 유사해질 수 있다. 앞선 경우와 같이 경계 조건 오류로 인한 부분을 수정해보자.


확인 결과 SerialDate.java의 719행은 실행 되지 않는다. ```java final int baseDOW = base.getDayOfWeek(); int adjust = -Math.abs(targetDOW - baseDOW); if (adjust >= 4) { // 718 adjust = 7 - adjust; // 719 } if (adjust <= -4) { adjust = 7 + adjust; } return SerialDate.addDays(adjust, base); ```
따라서, 테스트케이스를 만족할 수 있도록 올바르게 구현한 결과는 아래와 같다.
int delta = targetDOW = base.getDayOfWeek();
int positiveDelta = delta + 7;
int adjust = positiveDelta % 7;
if (adjust > 3)
    adjust -= 7;

return SerialDate.addDays(adjust, base);

위와 같이 변경한 SerialDate는 모든 테스트케이스를 통과했기 때문에 SerialDate 코드를 올바르게 고쳐보자!


📙 둘째, 고쳐보자

코드를 수정하면서 단위 테스트는 계속해서 수행한다!!

📍 448page 1행

법적인 정보는 필요하기 때문에 라이선스 정보와 저작권은 보존하고 변경 이력은 불필요하기 때문에 삭제한다.


📍 449page 61행

import 문은 java.text. 와 java.util.로 줄인다.


📍 449page 67행

Javadoc 주석은 HTML 태그를 사용한다. 그러나 하나의 소스코드에 여러 언어를 사용하는 것은 지양하는 방식이기 때문에 주석 전부를 < pre> 로 감싸는 편이 좋다.


📍 450page 86행

이 부분은 클래스 선언 부분이다. 클래스 이름이 SerialDate인 이유는 해당 클래스가 일련 번호를 사용해서 구현했기 때문이다.

// 증거!!
/**
 * Returns the serial number for the date, where 1 January 1900 = 2 (this
 * corresponds, almost, to the numbering system used in Microsoft Excel for
 * Windows and Lotus 1-2-3).
 *
 * @return the serial number for the date.
 */

저자는 '일련번호' 라는 용어가 서술적이지 못한 이름이라고 여긴다.
또한, SerialDate라는 이름이 구현을 암시하며 (숨기는 편이 낫다) 추상화 수준이 올바르지 못하다고 생각해서 Date나 Day라는 이름이 적합하다 생각한다.
하지만 이미 Java에는 수많은 Date, Day 클래스가 있기 때문에 SerialDate 대신 DayDate라는 이름을 사용하기로 한다.


📍 DayDate 클래스가 MonthConstants를 상속하는 이유는?

MonthConstants는 상수 모음에 불과하다. 상수는 상속하는것이 좋지 않다. 따라서 enum 형식으로 정의하는 편이 낫다.

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;
    }
}

이렇게 enum 형식으로 변경하게 되면 int로 달을 받던것을 Month로 받을 수 있으며 이에 따라 중복을 줄이기 위해 수정이 필요하게 된다.


📍 450page 91행

serialVersionUID 변수는 직렬화를 제어한다. 해당 변수를 선언하지 않으면 모듈을 변경할때마다 변수의 값이 달라지게 된다. (컴파일러가 자동 생성) 따라서 직접 선언 대신 자동 제어를 하기 위해 변수를 삭제한다.

* 직렬화 : 자바 시스템 내부에서 사용되는 Object 또는 Data를 외부의 자바 시스템에서도 사용할 수 있도록 byte 형태로 데이터를 변환하는 기술.

📍 450page 93행

불필요한 주석은 삭제한다.


📍 450page 97행과 100행

해당 위치의 두 변수는 DayDate 클래스가 표현할 수 있는 최초와 최후 날짜를 의미한다.
좀 더 깔끔하게 표현하기 위해 아래와 같은 코드로 만들어준다.

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이 0이아닌 2인 이유는 엑셀에서 날짜를 표시하는 방법과 관계있다고 하지만, (496쪽 71~76 참고) 이 부분이 SpreadsheetDate의 구현과 관련되지 DayDate와는 아무 상관이 없다. 따라서 SpreadsheetDate 클래스로 옮겨져야 한다. 실제로도 SpreadsheetDate 클래스에서만 두 변수를 사용함을 확인 가능하다.


📍 450page 104행과 107행

MINIMUM_YEAR_SUPPORTED와 MAXIMUM_YEAR_SUPPORTED 두 변수를 살펴보자. DayDate는 추상 클래스 이므로 구체적인 구현 정보를 포함할 필요가 없어서 두 변수를 SpreadsheetDate 클래스로 옮기려고 했다.

하지만 RleativeDayOfWeekRule 클래스에서도 두 변수를 getDate로 넘어온 인수 year가 올바른지 확인할 목적으로 사용함을 알 수 있었다.

이에 따라 DayDate 자체를 훼손하지않으면서 구현 정보를 전달할 방법이 필요하다. 살펴보면 getDate 메서드로 넘어오는 인수는 DayDate 인스턴스가 아니지만 getDate는 DayDate 인스턴스를 반환하는 것을 알 수 있다. 이는 어디선가 인스턴스를 생성하는 부분이 존재함을 의미한다.

일반적으로 기반 클래스는 파생 클래스를 모르는 것이 좋기 때문에 추상 팩토리 패턴을 적용하여
DayDate 인스턴스를 생성하는 DayDateFactory를 만들어 해결한다.

* 추상 팩토리 패턴 : 상세화된 서브클래스를 정의하지 않고도 서로 관련성이 있거나 독립적인 여러 객체의 군을 생성하기 위한 인터페이스를 제공하는 패턴
public abstract class DayDateFactory {
    private static DayDateFactory factory = new SpreadsheetDateFactory();
    public static viud 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라고 좀 더 서술적으로 변경했다.

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) {
        rturn 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로 옮긴다.


📍 450page 109

해당 부분의 요일 상수들은 enum형이여야 하므로 이에 맞게 수정해준다.


📍 451page 140

  • 해당 부분의 변수 이름만으로 설명이 충분하기 때문에 주석 삭제
    • 주석은 최소화 하는것이 좋다.
  • 적절하게 접근 제어자를 이용
  • 사용하지 않는 변수 제거
  • 변수가 한 곳에서만 사용될 경우, 사용 위치와 최대한 가깝게 옮긴다.

📍 451page 162행 ~ 452page 205행

해당 부분은 달에서 주를 의미하는 부분으로 아래와 같이 enum으로 변환이 가능하다.

public enum WeekInMonth {
    FIRST(1), SECOND(2), THIRD(3), FOURTH(4), LAST(0);
    public final int index;

    WeekInMonth(int index) {
        this.index = index;
    }
}

📍 452page 177~187

이 부분의 상수는 모호하다. INCLUDE_NONE, FIRST, SECOND, BOTH 상수는 수학적으로 개구간, 반개구간, 폐구간을 의미한다. 따라서 이를 더 잘 나타낼 수 있도록 enum 이름을 DateInterval로 결정하고 CLOSED, CLOSED_LEFT, CLOSED_RIGHT, OPEN으로 수정한다.


📍 452page 190~205

이 부분또한 enum 형태로 바꿔준다. 해당 부분의 상수는 주어진 날짜를 기준으로 특정 요일을 계산할때 사용된다. 직전 요일은 LAST, 다음 요일은 NEXT, 가까운 요일은 NEAREST로 정의하고 enum 이름을 WeekdayRange로 한다.


📍 불필요한 부분 삭제

  • 452page 208행의 description 변수 (사용 X)
  • 452page 213행의 기본 생성자 (사용 X)
  • 453page 242행에서 454page 270행 까지 설명이 부실한 주석
  • 인수와 변수 선언에서 final 키워드 삭제 (실질적 가치 X)

📍 454page 259

  • for 루프안에 if문 두번을 || 연산자를 이용해 if문을 하나로 만든다.

  • stringToWeekdayCode 메서드는 DayDate 클래스에 속하지 않는 사실상 Day의 구문 분석 메서드이므로 이를 Day로 옮긴다. 실제로 Day라는 개념은 DayDate에 존재하지 않으므로 DayDate 클래스에서 빼낸 다음 독자적인 소스파일로 만든다.


📍 454page 272 ~ 286

weekDayCodeToString 메서드를 Day로 옮기고 toSring이라 명명한다.

weekDayCodeToString : 넘어온 주 중 일자를 표현하는 문자열 반환
public enum Day {
    MONDAY(Calendar.MONDAY),
    TUESDAY(Calendar.TUESDAY),
    WEDNESDAY(Calendar.WEDNESDAY),
    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])) ||
                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];
    }
}

📍 454page 288 ~ 455page 316

getMonths라는 메서드 두 개에서 첫 번째가 두 번째 getMonths를 호출한다. 두 번째 getMonths를 호출하는 메서드는 첫 번째 getMonths 메서드 뿐이기 때문에 이 둘을 합쳐 단순화 하고, 이름을 좀 더 서술적으로 변경하였다.

public static String[] getMonthNames() {
    return dateFormatSymbols.getMonth();
}

📍 455page 326 ~ 346

isValidMonthCode 메서드는 Month enum을 만들면서 불필요해졌기 때문에 삭제한다.


📍 456page 356 ~ 375

monthCodeToQuarter 메서드는 기능 욕심으로 보인다. 이는 뚜렷한 목적없이 상수를 나열하고 return 하는 방식으로 좋지 못한 방법이다. 따라서 아래와 같은 메서드를 Month에 추가해주었다.

public int quarter() {
    return 1 + (index-1)/3;
}

이렇게 수정하게 되면 Month가 너무 커지게 되므로 Day enum과 일관성을 유지하도록 DayDate에서 분리해 독자적인 파일로 만들어 준다.


📍 456page 377 ~ 457page 426

이 부분도 monthCodeToString이라는 메서드 두 개가 나오는데, 한 메서드가 다른 메서드를 호출하며 플래그를 넘긴다. 그러나 메서드 인수로 플래그는 바람직하지 못하기 때문에 이름을 변경하고 단순화하여 Month enum으로 옮긴다.

public String toString() {
    return dateFormatSymbols.getMonths()[index - 1]; // return monthCodeTodString(month, false)
}

public String toShortString() {
    return dateFormatSymbols.getShortMonths()[index - 1];
}

📍 457page 428 ~ 459page 472

문자열을 달 코드로 반환하는 stringToMonthCode 메서드의 이름을 변경하고, 앞선 과정들처럼 Month enum으로 옮기고 단순화 하는 과정을 거친다.

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());
}

📍 459page 495 ~ 517

넘어온 년도가 윤년인지 확인하는 isLeapYear 메서드는 더 서술적인 표현을 이용하여 수정해준다.

public static boolean isLeapYear(int year) {
    boolean fourth = year % 4 == 0;
    boolean hundredth = year % 100 == 0;
    boolean fourHundredth = year % 400 == 0;
    return fourth && (!hundreth || fourHundredth);
}

📍 460page 519 ~ 536

윤년 횟수를 반환해주는 leapYearCount는 DayDate에 속하지 않기 때문에 분리해준다.


📍 460page 538 ~ 560

윤년을 고려해 마지막 일자를 반환하는 메서드인 lastDayOfMonth 에서 사용하는 LAST_DAY_OF_MONTH 배열은 Month enum에 속하므로 메서드도 여기로 옮기고 단순하게 고쳐주었다.

public static int lastDayOfMonth(Month month, int year) {
    if (month == Month.FEBRUARY && isLeapYear(year))
        return month.lastDay() + 1;
    else
        return month.lastDay();
}

📍 461page 562 ~ 576

받아온 기준 날짜에 넘어온 일자를 더해 새로운 날짜를 만들어주는 addDays 메서드에서 DayDate 변수를 사용하는데 해당 함수를 재정의할 가능성이 있기 때문에 인스턴스 메서드로 변경했다. 해당 메서드가 호출하는 toSerial 메서드의 이름을 toOrdinal로 변경하고 단순화 한다.

public DayDate addDays(int days) {
    return DayDateFactory.makeDate(toOrdinal() + days);
}

📍 461page 578 ~ 462page 602

기준 날짜에 넘어온 달을 더해 새로운 날짜를 만드는 addMonths 메서드 에서도 위와 같이 인스턴스 메서드로 변경해주고, 읽기 쉬운 형태로 수정하였다.

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(reusltDay, resultMonth, resultYear);
}

이렇게 정적 메서드를 인스턴스 메서드로 바꾸게 되면, date.addDays(5)라는 표현이 date 객체를 변경하지않고 새 DayDate 인스턴스를 반환한다는 사실을 나타낼 수 있는지 여부가 불확실 하다.

DayDate date = DateFactory.makeDate(5, Month.DECEMBER, 1952);
date.addDays(7); // 날짜를 일주일만큼 뒤로 미룬다.

위와 같이 코드를 작성하게 되면, addDays가 date객체를 변경했다고 생각할 수 있다. 따라서 이를 해결하고자 이름을 변경해준다.

DayDate date = oldDate.plusDays(5);

📍 462page 628 ~ 660

넘어온 주 중 일자 범위에 해당하면서 기준 날짜보다 빠른 마지막 날짜를 반환하는 getPreviousDayOfWeek 함수를 단순화하고, 임시 변수 설명을 이용해 가독성을 높인다. 또한 위처럼 인스턴스 메서드로 변경하고 중복된 부분을 제거한다.

마찬가지로 getFollowingDayOfWeek 도 구조가 같기 때문에 같은 방법으로 수정해준다.

또한 getNearestDayOfWeek 메서드도 위의 두 메서드를 수정한것과 같은 구조를 가지도록 고쳐준다.

* 임시 변수 설명 ?

final 키워드 (const 와 유사 : 변경 불가능한 상수) 를 제거하고 변수 선언

public DayDate getPreviousDayOfWeek(Day targetDayOfWeek) {
    int offsetToTarget = targetDayOfWeek.index - getDayOfWeek().index;
    if (offsetToTarget >= 0)
        offsetToTarget -= 7;
    return plusDays(offsetToTarget);
}

public DayDate getFollowingDayOfWeek(Day targetDayOfWeek) {
    int offsetToTarget = targetDayOfweek.index - getDayOfWeek().index;
    if (offsetToTarget <= 0)
        offsetToTarget += 7;
    return plusDays(offsetToTarget);
}

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);
}

📍 464page 728 ~ 465page 740

현재 달의 마지막 일자를 반환해주는 getEndOfCurrentMonth 메서드를 인스턴스 메서드로 고치고 이름을 수정해준다.

public DayDate getEndOfMonth() {
    Month month = getMonth();
    int year = getYear();
    int lastDay = lastDayOfMonth(month, year);
    return DayDateFactory.makeDate(lastDay, month, year);
}

📍 삭제할 함수

월 중 주를 반환해주는 함수인 weekInMonthToString과 relativeToString 메서드는 테스트 케이스 이외에 다른 코드를 호출하지 않기 때문에 해당 메서드와 테스트케이스를 삭제한다.


이제 추상 클래스 DayDate의 추상 메서드를 살펴보자.


📍 467page 829 ~ 836

  • toSerial 메서드 : 날짜에 대한 직렬번호 반환
    • 처음에 toOrdinal로 변경했지만 getOrdinalDay가 더 적합한 이름이라 판단하여 변경해준다.

📍 467page 844 ~ 846

  • toDate 메서드 : DayDate를 java.util.date 반환
    • SpreadsheetDate에서 구현한 toDate를 살펴보면 메서드는 SpreadsheetDate에 의존하지 않기 때문에 해당 코드를 DayDate 메서드로 옮겨 준다.

📍 468page 879 ~ 900

getDayOfWeek 도 SpreadsheetDate 구현에 의존성을 갖는지 확인해보자. getDayOfWeek 함수는 SpreadsheetDate의 서수 날짜 시작 요일에 논리적 의존성을 가진다. 그렇지 때문에 이 부분은 DayDate 메서드로 옮길 수 없다.

이를 대신하여 물리적 의존성을 가질 수 있도록 DayDate에 getDayOfWeekForOrdinalZerof라는 추상 메서드를 구현하고 SpreadsheetDate에서 Day.SATURDAY를 반환하도록 구현해준다.

* 논리적 의존성 : 다른 함수에서 사용하는 변수를 사용하여 의존성을 갖는 것
* 물리적 의존성 : 변수 대신 함수를 이용해서 함수 간 데이터를 공유하는 것
public Day getDayOfWeek() {
    Day startingDay = getDayOfWeekForOrdinalZero(); // 물리적 의존성
    int startingOffset = startingDay.index - Day.SUNDAY.index;
    return Day.make((getOrdinalDay() + startingOffset) % 7 + 1);
}

이제는 getDayOfWeek 메서드를 DayDate로 옮긴 후 getOrdinalDay와 getDayOfWeekForOrdinalZero를 호출하도록 변경해 줄 수 있게 된다.

위와 같은 방법으로 compare 메서드 (469page 902 ~ 913)도 DayDate로 옮겨주고 불분명한 이름을 변경하고 이에 맞는 테스트 케이스를 추가하는 리팩토링을 진행했다.


📍 470page 982 ~ 471page 995

isInRange 함수도 DayDate로 끌어올리고 if문 연쇄 대신에 enum 형식을 취해 if문 연쇄를 삭제해 주었다.

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;
        }
    };

    public abstract boolean isIn(int d, int left, int right);
}

public boolean isInRange(DayDate di, DayDate d2, DateInterval interval) {
    int left = Math.min(d1.getOrdinalDay(), d2.getOrdinalDay());
    int right = Math.max(d1.getOrdinalDay(), d2.getOrdinalDay());
    return interval.isIn(getOrdinalDay(), left, right);
}

📘 결론

  • 처음에 나오는 주석은 너무 오래되었기 때문에 간단하게 고치고 개선했디.

  • enum을 모두 독자적인 소스 파일로 옮겼다.

  • 정적 변수(dateFormatSymbols)와 정적 메서드(getMonthNames,isLeapYear, lastDayOfMonth)를 DateUtil이라는 새 클래스로 옮겼다.

  • 일부 추상 메서드를 DayDate 클래스로 끌어올렸다.

  • Month.make를 Month.fromInt로 변경했다. 다른 enum도 똑같이 변경했다. 또한 모든 enum에 toInt() 접근자를 생성하고 index 필드를 private로 정의했다.

  • plusYears와 plusMonths에 중복을 correctLastDayOfMonth라는 새 메서드를 통해 중복을 없애 주었다.

  • 사용하던 숫자 1을 없앴다. 모두 Month.JANUARY.toInt() 혹은 Day.SUNDAY.toInt()로 적절히 변경했다. SpreadsheetDate 코드를 살펴보고 알고리즘을 조금 손봤다.

이러한 과정을 통해 클래스 자체의 크기가 줄어 테스트 하지 않는 코드의 비중이 커졌기 때문에 코드 커버리지가 감소하는 결과를


📚 Reference

  • Clean Code : 애자일 소프트웨어 장인 정신
profile
로그를 생활화
post-custom-banner

0개의 댓글