[클린코드] 37일차/38일차/39일차 - SerialDate 리팩터링

[클린코드] 37일차/38일차 - SerialDate 리팩터링

클린코드 37일차 (p. 344 ~ 351 (16장) )

클린코드 38일차 (p. 352 ~ 362 (16장) )

클린코드 39일차 (p. 363 ~ 369 (16장) )

16장 SerialDate 리팩터링

이 장에서는 JCommon 라이브러리에 있는 org.jfree.date 패키지의 SerialDate라는 클래스를 리팩터링을 진행한다. 

 

(* 책에서는 https://www.jfree.org/jcommon/index.php 라고 언급되어있지만
https://www.jfree.org/jcommon/ 로 들어가야한다. + https://www.jfree.org/jcommon/api/index.html)

 

 

 

SerialDate는 날짜를 표현하는 자바 클래스이다.

하지만 자바는 이미 java.util.Date, java.util.Calendar 등과 같은 클래스를 제공한다.

 

그럼 왜 필요한가 ?

시간 기반 날짜 클래스보다 순수 날자 클래스를 위해 만들었다고 한다.

 

일단, 첫 번째는 무조건 돌려봐야한다.

SerialDateTests 클래스는 단위 테스트 케이스 몇 개를 포함한다.

 

실패하는 테스트 케이스는 없지만, 모든 경우를 점검하지는 않는다.

 

(ex. SerialDateTests 클래스는 MonthCodeToQuarter 메서드를 전혀 호출 X)

 

클로버 Clover(코드 커버리지 분석 도구)를 활용하여 단위 테스트가 실행하는 코드와 실행하지 않는 코드를 확인할 수 있다.

 

클로버에 따르면, SerialDate에서 실행 가능한 문장 185개 중 91개만 단위 테스트를 실행한다. (약 50% 정도)

 

클래스를 철저히 이해하고 리팩터링하려면 훨씬 높은 테스트 커버리지가 필요하다. 

 

두 번째는 고쳐보자.

1) 라이센스 정보, 저작권, 작성자, 변경 이력은 이제 소스 코드 제어 도구를 사용하므로 없애자.

 

 

2) import문은 java.text.*와 java.util.*로 줄여도 된다.

import java.text.DateFormatSymbols;
import java.text.SimpleDateFormat;
import java.util.Calendar;
import java.util.GregorianCalendar;

 

 

3) 한 소스 코드에 여러 언어를 사용하지 말자. (like 주석에 4가지 언어(Java, 영어, Javadoc, HTML)를 사용하는 경우)

 

 

4) 클래스 이름이 SerialDate인 이유가 무엇일까? Serial 단어가 왜 들어갈까?

 

해당 증거는 467쪽 830행부터 시작하는 주석에 있다.

클래스 이름이 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라는 클래스가 너무 많기에 DayDate라는 용어를 쓰기로 결정..!

 

 

5) 불필요한 상속은 없애자. 마치 SerialDate 클래스가 MonthConstants를 상속하고 있는 것처럼

public interface MonthConstants {
    ...
    public static final int JANUARY = 1;
    public static final int FEBRUARY = 2;
    ...
}

 

MonthConstants는 달을 정의한 상수 모음에 불과하다. 옛 자바 프로그래머들이 많이 쓰던 기교로 지금은 바람직하지 않다.

 

상수를 사용할거면 enum으로 정의하자.

 

enum으로 정의함으로써 isValidMonthCode 메서드와 monthCodeToQuarter 오류 검사 코드도 더이상 필요하지 않다.

 

 

6) 불필요한 주석은 제발 없애자.

 

불필요한 주석은 거짓말과 잘못된 정보로 쌓이기 쉽상이다. 

 

 

7) 인수와 변수 선언에서 final 키워드를 모두 없앴다.

 

실질적인 가치는 없으면서 코드만 복잡하게 만든다고 판단했기 때문이다.

 final 키워드를 제거하겠다는 결정은 일부 기존 관례와 어긋난다. 

 

예를 들어, 로버트 시몬스는 "코드 전체에 final을 사용하라"고 강력히 권장한다.

 

하지만 final 키워드는 final 상수 등 몇 군데를 제하면 별다른 가치가 없으며 코드를 복잡하게 만든다. 

 

어쩌면 이렇게 느끼는 이유는 작성한 테스트가 final 키워드로 잡아낼 오류를 모두 잡아내기 때문인지도 모르겠다.

 

 

8) for 루프 안에 if문이 두 번 나오는데 이를 || 연산자를 사용하여 if문 하나로 만들었다.

...
s = s.trim();
for (int i = 0;i < weekDayNames.length; i++) {
    if (s.equals(shortWeekdayNames[i])) {
        result = i;
        break;
    }
    if (s.equals(weekDayNames[i])) {
        result = i;
        break;;
    }
}
...

...
s = s.trim();
for (int i = 0;i < weekDayNames.length; i++) {
    if (s.equals(shortWeekdayNames[i]) || s.equals(weekDayNames[i]) {
        result = i;
        break;
    }
...

 

 

9) 한 메서드가 다른 메서드를 호출하며 플래그를 넘긴다. 일반적으로 메서드 인수로 플래그는 바람직하지 못하다.

출력 형식을 선 택하는 플래그는 가급적 피하는 편이 좋다. 

 

결론 및 작업 정리

주석 개선 (불필요한 주석 제거/주석에 다양한 언어 쓰지 않기/일련 번호같은 부분은 주석으로 추가 설명하기)

enum 사용, 독자적인 파일로 만들기

정적 변수, 정적 메서드(isLeapYear)를 새로운 파일(DateUtil)로 이동

네이밍 변경(add -> plus)

경계값 개선(Month.JANURARY.toInt())

 

다시 한 번 보이스카우트 규칙을 따랐다. 

테스트 커버리지가 증가했으며, 코드 크기가 줄었고, 명확해졌다.