- 날짜를 표현하는 SerialDate 클래스를 살펴보고 리팩터링 한다
돌려보자
- SerialDateTests라는 클래스가 있지만 모든 경우를 커버하지 않기 때문에 단위 테스트 케이스를 구현한다
- 새 단위 테스트는 92% 정도의 코드 커버리지를 가진다
- 테스트 케이스를 통해 일부를 변경한다
고쳐보자
클래스 이름에 관한 고찰
- 클래스 선언에 Serial이라는 단어가 들어가는 이유는 일련번호(serial number)를 사용해 클래스를 구현했기 때문이다
- serialDate라는 이름이 서술적이지 못하다고 생각한다
- SerialDate라는 이름은 구현을 암시하지만 실상은 추상 클래스이기 때문에 구현을 암시할 필요가 없다
- 고민 끝에 DayDate라는 용어로 변경한다
MonthConstants를 상속한다
- MonthConstants 클래스는 달을 정의하는 static final 상수 모음
- monthConstants는 enum으로 정의해야 마땅하다
- 달을 int로 받던 메서드는 Month로 받도록 한다
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;
}
}
serialVersionUID 변수는 직렬화를 제어한다
- 자동 제어가 되지 않기 때문에 해당 변수를 당분간 삭제한다
불필요한 주석은 삭제한다
DayDate 클래스가 표현할 수 있는 최초와 최후 날짜를 의미하는 변수를 좀 더 깔끔하게 표현하기 위해 아래와 같은 코드로 만들어준다
public static final int EARLIEST_DATE_ORDINAL = 2;
public static final int LATEST_DATE_ORDINAL = 2958465;
- EARLIEST_DATE_ORDINAL이 0이 아닌 2인 이유는 엑셀에서 날짜를 표시하는 방법과 관계있다고 하지만 이 부분이 SpreadsheetDate의 구현과 관련되지 DayDate와는 아무 상관이 없다
- 따라서 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로 옮긴다
해당 부분의 요일 상수들은 enum형이여야 하므로 이에 맞게 수정해준다.
해당 부분의 변수 이름만으로 설명이 충분하기 때문에 주석 삭제
- 주석은 최소화 하는것이 좋다
- 적절하게 접근 제어자를 이용, 사용하지 않는 변수 제거
- 변수가 한 곳에서만 사용될 경우 사용 위치와 최대한 가깝게 옮긴다
달에서 주를 의미하는 부분을 아래와 같이 enum으로 변환한다
public enum WeekInMonth {
FIRST(1), SECOND(2), THIRD(3), FOURTH(4), LAST(0);
public final int index;
WeekInMonth(int index) {
this.index = index;
}
}
모호한 상수를 수정한다
- INCLUDE_NONE, FIRST, SECOND, BOTH 상수는 수학적으로 개구간, 반개구간, 폐구간을 의미하기 때문에 이를 더 잘 나타낼 수 있도록 enum 이름을 DateInterval로 결정하고 CLOSED, CLOSED_LEFT, CLOSED_RIGHT, OPEN으로 수정한다
enum 형태 적용
- 주어진 날짜를 기준으로 특정 요일을 계산할때 사용되는 상수를 enum으로 변경한다
- 직전 요일은 LAST, 다음 요일은 NEXT, 가까운 요일은 NEAREST로 정의하고 enum 이름을 WeekdayRange로 한다
불필요한 부분을 삭제한다
- 사용하지 않는 변수와 기본 생성자, 주석, final 키워드 등을 삭제한다
for 루프안에 if문 두번을 || 연산자를 이용해 if문을 하나로 만든다
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];
}
}
이름을 서술적으로 변경한다
- getMonths라는 메서드 두 개에서 첫 번째가 두 번째 getMonths를 호출한다
- 두 번째 getMonths를 호출하는 메서드는 첫 번째 getMonths 메서드 뿐이기 때문에 이 둘을 합쳐 단순화 하고, 이름을 좀 더 서술적으로 변경하였다
public static String[] getMonthNames() {
return dateFormatSymbols.getMonth();
}
isValidMonthCode 메서드는 Month enum을 만들면서 불필요해졌기 때문에 삭제한다
메서드를 추가한다
- monthCodeToQuarter 메서드는 기능 욕심으로 보인다
- 이는 뚜렷한 목적없이 상수를 나열하고 return 하는 방식으로 좋지 못한 방법이다 - 따라서 아래와 같은 메서드를 Month에 추가해주었다
public int quarter() {
return 1 + (index-1)/3;
}
- 이렇게 수정하게 되면 Month가 너무 커지게 되므로 Day enum과 일관성을 유지하도록 DayDate에서 분리해 독자적인 파일로 만들어 준다.
인수로 플래그를 보내지 않도록 변경한다
- monthCodeToString이라는 메서드 두 개가 나오는데 한 메서드가 다른 메서드를 호출하며 플래그를 넘긴다
- 메서드 인수로 플래그는 바람직하지 못하기 때문에 이름을 변경하고 단순화하여 Month enum으로 옮긴다
public String toString() {
return dateFormatSymbols.getMonths()[index - 1];
}
public String toShortString() {
return dateFormatSymbols.getShortMonths()[index - 1];
}
단순화한다
- 문자열을 달 코드로 반환하는 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());
}
넘어온 년도가 윤년인지 확인하는 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);
}
윤년 횟수를 반환해주는 leapYearCount는 DayDate에 속하지 않기 때문에 분리해준다
단순화한다
- 윤년을 고려해 마지막 일자를 반환하는 메서드인 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();
}
재정의 가능성이 있는 함수는 인스턴스 메서드로 변경한다
- 받아온 기준 날짜에 넘어온 일자를 더해 새로운 날짜를 만들어주는 addDays 메서드에서 DayDate 변수를 사용하는데 해당 함수를 재정의할 가능성이 있기 때문에 인스턴스 메서드로 변경한다
- 해당 메서드가 호출하는 toSerial 메서드의 이름을 toOrdinal로 변경하고 단순화 한다.
public DayDate addDays(int days) {
return DayDateFactory.makeDate(toOrdinal() + days);
}
- 기준 날짜에 넘어온 달을 더해 새로운 날짜를 만드는 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);
인스턴스 메서드 변경과 중복 제거를 한다
- 넘어온 주 중 일자 범위에 해당하면서 기준 날짜보다 빠른 마지막 날짜를 반환하는 getPreviousDayOfWeek 함수를 단순화하고 임시 변수 설명을 이용해 가독성을 높인다
- 인스턴스 메서드로 변경하고 중복된 부분을 제거한다
- 마찬가지로 getFollowingDayOfWeek 도 구조가 같기 때문에 같은 방법으로 수정해준다.
- 또한 getNearestDayOfWeek 메서드도 위의 두 메서드를 수정한것과 같은 구조를 가지도록 고쳐준다.
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);
}
- 현재 달의 마지막 일자를 반환해주는 getEndOfCurrentMonth 메서드를 인스턴스 메서드로 고치고 이름을 수정해준다.
public DayDate getEndOfMonth() {
Month month = getMonth();
int year = getYear();
int lastDay = lastDayOfMonth(month, year);
return DayDateFactory.makeDate(lastDay, month, year);
}
불필요한 코드는 삭제한다
- 월 중 주를 반환해주는 함수인 weekInMonthToString과 relativeToString 메서드는 테스트 케이스 이외에 다른 코드를 호출하지 않기 때문에 해당 메서드와 테스트케이스를 삭제한다
추상 클래스 DayDate의 추상 메서드를 살펴본다
- toSerial 메서드 : 날짜에 대한 직렬번호 반환
- 처음에 toOrdinal로 변경했지만 getOrdinalDay가 더 적합한 이름이라 판단하여 변경해준다.
- toDate 메서드 : DayDate를 java.util.date 반환
- SpreadsheetDate에서 구현한 toDate를 살펴보면 메서드는 SpreadsheetDate에 의존하지 않기 때문에 해당 코드를 DayDate 메서드로 옮겨 준다
- 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 메서드도 DayDate로 옮겨주고 불분명한 이름을 변경하고 이에 맞는 테스트 케이스를 추가하는 리팩토링을 진행한다
- 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을 모두 독자적인 소스 파일로 옮긴다
- 정적 변수와 정적 메서드를 새 클래스로 이동시킨다
- 일부 추상 메서드를 DayDate 클래스로 끌어올린다
- Month.make를 Month.fromInt로 변경하고 다른 enum도 똑같이 변경한다
- 모든 enum에 toInt() 접근자를 생성하고 index 필드를 private로 정의한다
- 새 메서드를 생성하고 중복을 삭제한다
- 여기저기서 사용되던 숫자 1을 삭제했다
- 테스트 커버리지 수치가 낮아지게 되었다
보이스카우트 규칙을 따라 체크아웃한 코드보다 좀 더 깨끗한 코드를 체크인하게 되었다
개인적인 감상
- 처음에 import 문은 java.text.와 java.util.로 줄여도 된다는 말이 나와서 모든 것을 임포트하면 느려지지 않을까 하는 의문이 생겼다
- 컴파일을 할 때엔 느려질 수 있으나 런타임 시에는 속도에 영향을 주지 않는 것으로 판단된다
- 테스트 커버리지를 100%로 만들지 않아도 된다는 것을 알 수 있었다
- 테스트 커버리지 100%를 맞추려고 하다보면 테스트를 통과하기 위한 코드를 짜게 될 가능성이 있다
- 테스트 때문에 로직을 변경하는 것은 바람직하지 않다
