[Clean Code] Chapter 16 SerialDate 리팩터링

SerialDate란?

[http://www.jfree.org/jcommon/index.php]에서 JCommon 라이브러리를 제공

JCommon 라이브러리 내 org.jfree.data 패키지가 있으며 이 패키지 내 SerialDate 클래스가 있다.

 

SerialDate는 날짜를 표현하는 자바 클래스이다. 이미 java.util.Date, java.tuil.Calendar 등과 같은 클래스가 있는데

SerialDate를 새로 구현한 이유는? Javadoc에 의하면...

SerialDate javadoc

 

---

돌려보자!

SerialDateTests라는 클래스는 단위 테스트 케이스 몇 개를 포함한다. 돌려보면 실패하는 테스트 케이스는 없다.

하지만 주의깊게 코드를 보게되면 모든 코드를 점검하지 않는다는 사실을 알 수 있다. 이를 검증하기 위해서 Clover를 사용하면 실제 전체 코드 중 50% 가량의 코드만 테스트한다는 것을 알 수 있다.

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

 

테스트 케이스를 훑어보면 많은 코드들이 주석으로 처리되었다 -> 실패한 테스트 케이스들

추가적으로 저자는 더 높은 커버리지를 위해 단위 테스트 케이스를 구현하고, 해당 단위 테스트에서 실패한 경우를 성공시킬 수 있도록 원본을 리팩토링한다.

- 리팩토링 -

해당 테스트를 통해서 커버리지가 얼마나되는지 확인했고 테스트를 하지 않거나 부실한 테스트를 추가

사용하기 모호한 변수명에 대한 리팩토링

직관적이지 않은 메소드명에 대한 리팩토링

오류 발생 가능한 애매한 알고리즘에 대한 리팩토링

 

---

고쳐보자!

SerialDate 라이브러리 클래스에서는 실패한 테스트 케이스, 과한 설명 등과 같은 불필요한 주석이 너무 많다.

이러한 코드들을 기반으로 리팩토링을 진행하였다.

 

- 불필요한 주석 제거 - 

라이선스 정보, 저작권, 작성자, 변경 이력 부분에서는 법적인 정보는 필요하므로 라이선스 정보와 저작권은 보존한다.

하지만 작성자, 변경 이력과 같은 경우는 소스 코드 관리 툴을 이용하게 되면 불필요한 정보가 되므로 삭제해도 된다.

+

불필요한 주석은 거짓말과 잘못된 정보가 쌓이기 좋은 곳이다.

+

주석은 최소화하는 것이 바람직하므로 변수명으로 직관적인 코드 설명이 가능한 경우 주석을 삭제

 

- 과다한 import 문 정리 -

import java.util.Calendar, import java.util.GregorianCalendar ; 등은 import java.util.*으로 줄여 써되 되므로

이와 같은 패턴의 나열해 놓은 import문은 정리한다.

 

- Javadoc 정리 -

Javadoc 주석은 HTML 태그를 사용한다. 하나의 클래스 내에서 다양한 언어를 사용하게 되면 javadoc을 사용하는 순간 코드 본연의 위치를 잃을 가능성이 있다. 이러한 문제를 사전에 방지하기 위해서 주석 전체를 <pre> 태그를 사용하여 유지하자.

<pre>태그를 사용하게 되면 코드 형식이 javadoc내에서도 유지된다.

 

- 클래스 명 정리 -

클래스 이름이 SerialDate인 이유는 해당 클래스가 일련 번호를 사용하여 구현하기 때문이다.

'일련번호'라는 용어가 서술적이지 못하고 이러한 점을 고려하여 Date나 Day라는 명이 더 적합하다.

하지만 이미 java에는 Date, Day 클래스가 있으므로 SerialDate 대신 DayDate라는 이름을 사용하도록 한다.

 

- 변수 정리 -

MINIMUM_YEAR_SUPPORTED와 MAXIMUM_YEAR_SUPPORTED 두 변수의 경우를 확인해보면

public static final int SERIAL_YEAR_SUPPORTED = 1900;

public static final int MAXIMUM_YEAR_SUPPORTED = 9999;

DayDate 클래스는 추상 클래스 이므로 구체적인 구현 정보를 포함할 필요가 없다. 따라서 두 변수를 SpreadsheetDate 클래스로 옮기려한다. 하지만 RelativeDayOfWeekRule 클래스에서도 두 변수를 getDate로 넘어온 인수 year가 올바른지 확인할 목적으로 사용함을 확인 가능하다.

이에 따라 DayDate 자체를 훼손하지 않으면서 구현 정보를 전달할 방법이 필요하다.

일반적으로 우리는 파생 클래스의 인스턴스로부터 구현 정보를 가져온다. 하지만 getDate메소드로 넘어오는 인수는 DayDate 인스턴스가 아니다. 반면 getData는 DayData 인스턴스를 반환하는 것을 알 수 있다. 이는 어디선가 인스턴스를 생성하는 부분이 존재함을 의미한다.

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

 

변수 정리- 정리) 구체적 정보를 포함할 필요가 없는 변수들을 다른 클래스로 넘겨 정리하려다 보면 인스턴스 관련 문제가 발생할 수 있다. 이러한 문제를 방지하기 위해서 추상 팩토리 패턴을 적용하여 인스턴스를 생성하는 클래스를 따로 정의하여 사용.

 

+

 

변수가 한 곳에서만 사용되는 경우, 사용 위치에 최대한 가깝게 배치하여 사용하자.

 

- 경계 구간 상수 -

대부분의 코드에서 오류가 가장 많이 발생하고 그만큼 error catch하기 어려운 부분이 바로 상수 정의 경계값이다. 따라서 이러한 상수들의 명을 모호하지 않고 명확하게 표현한다.

ex_ CLOSED, CLOSED_LEFT, CLOSED_RIGHT, OPEN으로 개구간 표현

 

- 함수 정리 -

같은 역할을 하지만 다른 위치에서 사용되는 메소드들을 하나의 메소드로 정리하고 메소드명을 좀 더 서술적으로 표현하여 역할을 명확하게 한다.

public static SerialDate addMonths(final int months, final SerialDate base){
	final int yy = (12* base.getYYYY() + base.getMonth() + months - 1) / 12;
    final int mm = (12* base.getYYYY() + base.getMonth() + months - 1) % 12 + 1;
    final int dd = Math.min(base.getDayOfMonth(), SerialDate, lastDayOfMonth(mm, yy));
    
    return SerialDate.createInstance(dd, mm, yy);
 }

기준 날짜에 넘어온 달을 더해 새로운 날짜를 만드는 addMonths 메소드에서도 다음과 같이 반환 인스턴스 타입, 인자 등을 수정해 더욱 가독성이 좋은 형태로 수정하였다. (인스턴스 메서드로 변경)

public DayDate addMonths(int months){
	int thisMonthAsOrdinal = 12 * getYear() + getMonths().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);
}

 

 

- 의존성 -

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

저자는 구현에 논리적으로 의존한다면 물리적으로도 의존해야 마땅하다고 판단

이를 대신하여 물리적 의존성을 가질 수 있도록 DayDate에 getDayofWeekForOrdinalZero라는 추상 메소드를 구현하고 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를 호출하도록 변경해 줄 수 있게 된다.

 

---

결론!

다시 한번 우리는 보이스카우트 규칙을 따랐다. 체크아웃한 코드보다 좀 더 꺠끗한 코드를 체크인하게 되었다. 시간은 걸렸지만 가치 있는 작업이었다. 테스트 커버리지가 증가했으며, 버그 몇 개를 고쳤으며, 코드 크기가 줄었고, 코드가 명확해졌다. 다음 사람은 우리보다 코드를 좀 더 쉽게 이해하리라. 그래서 우리보다 코드를 좀 더 쉽게 개선하리라

보이스카웃 규칙 : 처음 왔을 때보다 더 깨끗하게 떠나라

 

리팩토링의 시작 계기는 테스트 케이스의 커버리지 비율이 너무 낮다는 점이었다. 그래서 이번 챕터에서 진행한 사항은 다음과 같다.

 

오래되고 불확실한 주석을 수정 및 제거

부정확하고 모호한 알고리즘 수정

정적 변수, 정적 메소드를 DateUtil이라는 새 클래스로 move

추상 메소드를 DayDate 클래스로 끌어올림

접근제어자 수정

중복 및 불필요 메소드 정리

 

완벽한 리팩토링은 아닐지라도, 테스트 커버리지를 높일 수 있는 리팩토링을 진행하고 정리함으로서 다음 사람들이 더 리팩토링을 쉽고 정확하게 진행할 것이라는 기대를 할 수 있게 되었다.