[클린코드(CleanCode)] 16장 SerialDate 리팩터링

개요

https://www.jfree.org/jcommon/

JCommon

이번 장에서는 JCommon 라이브러리 내부의 org.jfree.date 패키지 내의 SerialDate 클래스 분석해 본다.

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

SerialDate.java

0.03MB

 

위의 코드를 참고하기를 바란다.

---

첫째, 돌려보자

SerialDateTest.java

0.01MB

 

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

하지만 코드를 살펴보면 모든 경우를 점검하지 않고 있다.

테스트 코드에서 전혀 실행하지 않는 코드도 존재한다.

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

 

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

따라서 독자적으로 단위 테스트 케이스를 구현했다.

BobsSerialDateTest.java

0.02MB

---

둘째, 고쳐보자

주석 분류

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

import 문 간결화

• import 문은 java.text. 와 java.util.로 줄여도 된다.

Javadoc 주석

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

서술적인 이름 사용

• 일련번호라는 용어는 날짜보다 제품 식별 번호에 더 적합하다.
• SerialDate라는 이름은 구현을 암시하지만 실상은 추상 클래스이다.
• 따라서 이름의 추상화 수준이 올바르지 않다.
• 그냥 Date 가 좋지만 이미 많이 존재하므로 DayDate로 결정했다.

상수 열거형 보다 Enum 사용

• MonthConstants를 상속하고 있는데 static final 상수 모음에 불과하다.
• 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;
    }
}

 

serialVersionUID

• serialVersionUID 변수는 직렬화를 제어한다.
• 이 변수값을 변경하면 이전 소프트웨어 버전에서 직렬화한 DayDate를 더 이상 인식하지 못한다.
  • 역직렬화 시에 InvalidClassException 가 발생한다.
• 해당 변수를 선언하지 않으면 모듈을 변경할 때마다 변수의 값이 달라지게 된다.
• serialVersionUID 변수를 선언하지 않으면 컴파일러가 자동으로 생성하지만 모듈 변경 시 자동으로 달라진다.
• 따라서 직접 선언 대신 자동 제어를 하기 위해 변수를 삭제한다.

변수 위치 올바르게 변경

• 책임이 있는 클래스와 관계없는 변수는 제거하거나 관련 책임이 있는 클래스로 옮겨야 한다.

기반 클래스와 파생 클래스

• 일반적으로 기반 클래스(부모 클래스)는 파생 클래스(자식 클래스)를 몰라야 바람직하다.
• 추상 클래스는 구체적인 구현 정보를 포함할 필요가 없다.
• abstract factory 패턴을 적용하여 객체를 생성하도록 한다.

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

접근 제한자(지정자) 변경

• 내부에서만 사용하고 외부로 공개할 필요가 없는 경우 public => private로 변경

불필요한 final 키워드 제거

• 인수와 변수 선언에서 final 키워드 제거
• 실질적인 가치는 없으면서 코드만 복잡하게 만든다고 판단했기 때문이다.
• 로버트 시몬스는 "코드 전체에 final을 사용하라고 권장한다." 저자와 다른 의견이다.

중첩 if 문 통합

• if 문이 중첩되어 나올 경우 ||, && 등으로 하나로 만든다.

테스트 케이스에서만 호출한다면 제거

• 일련의 리팩터링 작업은 나름대로 우아했지만, 실제로 이 메서드를 사용하는 코드는 방금 수정한 테스트 케이스가 유일했다.
• 그래서 저자는 테스트 케이스를 삭제했다.

물리적 의존성과 논리적 의존성

• 물리적 의존성은 없지만 논리적 의존성이 존재하는 경우도 있다.
• 뭔가가 구현에 논리적으로 의존한다면 물리적으로도 의존해야 마땅하다.

 

지금까지 한 작업을 정리한다

1. 처음에 나오는 주석은 너무 오래되었다. 그래서 간단하게 고치고 개선했다.
2. enum을 모두 독자적인 소스파일로 옮겼다.
3. 정적 변수(dateFormatSymbols)와 정적 메서드(getMonthNames, isLeapYear, lastDayOfMonth)를 DateUtil이라는 새 클래스로 옮겼다.
4. 일부 추상 메서드를 DayDate 클래스로 끌어올렸다.
5. Month.make를 Month.fromInt로 변경했다. 다른 enum도 똑같이 변경했다. 또한 모든 enum에 toInt() 접근자를 생성하고 index 필드를 private로 정의했다.
6. plusYears와 plusMonths에 중복이 있었다. correctLastDayOfMonth라는 새 메서드를 통해 중복을 없애고 셋 모두 좀 더 명확하게 했다.
7. 팔방미인으로 사용하던 숫자 1을 없앴다. 모두 Month.JANUARY.toInt() 혹은 Day.SUNDAY.toInt()로 적절히 변경했다. SpreadsheetDate 코드를 살펴보고 알고리즘을 조금 손봤다.
   

흥미롭게도 DayDate 코드 커버리지는 84.9%로 감소했다.

테스트 코드가 줄어서가 아니라 클래스 크기가 작아지는 바람에 테스트하지 않는 코드의 비중이 커졌기 때문이다.

---

결론

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

체크아웃한 코드보다 좀 더 깨끗한 코드를 체크인하게 되었다.

시간은 걸리지만 가치 있는 작업이다.

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

다음 사람은 우리보다 코드를 더 쉽게 이해하고 더 쉽게 개선하리라.

---

글쓴이의 생각

저번 장에서는 JUnit이라는 프레임워크를 가지고 리펙토링하는 것을 다뤘다.

JUnit보다 더 복잡하고 자세하게 리팩토링을 하여 유익한 시간이었다.

예전에는 혼자 작업하느라 나만 알아볼 수 있게 코드 짜고 문제가 없었지만

협업을 하면서 다른 사람이 내 코드를 읽어 볼 때 명확하지 않아 애먹는 모습을 보고

코드를 이상하게 짠 걸 많이 반성하였다.

그래서 클린 코드를 읽게 되었다.

앞으로는 코드를 짜면서 상대방이 읽어도 이해할 수 있고 오해가 생기지 않도록 명확하게 짜는 연습을 해야겠다.