Intro
- 이 장에서는 JCommon 라이브러리의 SerialDate 라는 클래스를 탐험한다.
- SerialDate는 날짜를 표현하는 자바 클래스이다.
- 강조하건대, 이 장에서 수행하는는 분석은 악의나 자만과는 거리가 멀다.
- 전문가 입장에서 수행하는 검토, 그 이상도 그 이하도 아니다.
- 우리 모두가 편안하게 여겨야할 활동이다.
- 남이 내게 해준다면 감사히 반겨야 할 활동이다.
첫째, 돌려보자
- SerialDateTests 는 단위 테스트 케이스 몇개를 포함한다.
- 하지만 모든 경우를 점검하지 않고 있다.
- 테스트코드에서 전혀 실행하지 않는 코드도 존재한다.
- 테스트 커버리지가 대략 50% 정도이다.
- 클래스를 철저히 이해하고 리팩터링하려면 높은 테스트 커버리지가 필요하다.
- 따라서 독자적으로 단위 테스트 케이스를 구현했다.
- 경계 조건 오류가 존재한다.
- 논리적 오류로 인하여 결코 실행되지 않는 코드도 존재한다.
둘째, 고쳐보자
주석 분류
- 필요한 주석 : 법적인 정보는 필요하므로 라이선스 정보와 저작권은 보존
- 불필요한 주석 : 변경이력, 소스 코드 제어도구에서 지원하므로 삭제
import 문 간결화
- import 문은 java.text. 와 java.util. 로 줄여도 된다.
- 추가) 네이버 컨벤션에서는 static import 에만 와일드 카드 허용한다.
javadoc 주석
- 여러 언어를 사용시 모양을 맞추기가 어렵다.
- 주석 전부를
<pre>로 감싸면 모양이 유지된다.
서술적인 이름 사용
- 일련번호라는 용어는 날짜보다 제품 식별 번호에 더 적합하다.
- SerialDate라는 이름은 구현을 암시하지만 실상은 추상 클래스 이다.
- 따라서 이름의 추상화 수준이 올바르지 않다.
- 그냥 Date 가 좋지만 이미 많이 존재 하므로 DayDate 로 결정했다.
상수 열거형 보다 Enum 사용
- MonthConstants 를 상속하고 있는데 static final 상수 모음에 불과하다.
- MonthConstants 는 Enum 으로 변경하여 사용하는것이 마땅하다.
serialVersionUID
- serialVersionUID 변수는 직렬화를 제어한다.
- 이 변수 값을 변경하면 이전 소프트웨어 버전에서 직렬화한 DayDate를 더 이상 인식하지 못한다.
- 역직렬화 시에 InvalidClassException 가 발생한다.
- serialVersionUID 변수를 선언하지 않으면 컴파일러가 자동으로 생성하지만 모듈 변경시 자동으로 달라진다.
- 문서에서는 직접 선언하라고 권하지만, 저자는 자동 제어가 훨씬 더 안전하게 여긴다.
- serialVersionUID를 변경하지 않아 생기는 괴상한 오류를 디버깅하는것 보다
차라리 InvalidClassException 을 디버깅하는 편이 훨씬 나으니까. - 주석) 이책을 검토한 검토자 여러명이 반론을 제기 했으며 직렬화ID 를 직접 제어하는 편이 낫다고 주장했다.
저자는 이 의견도 타당하다고 생각한다. - 정리) 특정 버전에서 직렬화한 클래스를 다른 버전에서 복원하지 않는 편이 안전하다.
변수 위치 올바르게 변경
- 책임이 있는 클래스와 관계없는 변수는 제거하거나 관련 책임이있는 클래스로 옮겨야한다.
기반 클래스와 파생 클래스
- 일반적으로 기반 클래스(부모클래스)는 파생 클래스(자식클래스)를 몰라야 바람직하다.
- 추상클래스는 구체적인 구현 정보를 포함할 필요가 없다.
- abstract factory 패턴을 적용하여 객체를 생성하도록 한다.
public abstract class DayDateFactory {
private static DayDateFactory factory = new SpreadsheetDateFactory();
public static void setInstance(DayDateFactory factory) {
DayDateFactory.factory = factory;
}
protected abstract DayDate _makeDate(int ordinal);
protected abstract DayDate _makeDate(int day, int month, int year);
// ... 정적 메서드에서 호출 하는 추상 메서드
public static DayDate makeDate(int ordinal) {
return factory._makeDate(ordinal);
}
public static DayDate makeDate(int day, int month, int year) {
return factory._makeDate(day, month, year);
}
// ... 추상 메서드로 위임하는 정적 메서드
}- 기본적으로 SpreadsheetDateFactory 를 사용하지만 어제든 바꿀수 있다.
- 추상 메서드로 위임하는 정적 메서드는 SINGLETON, DECORATOR, ABSTRACT FACTORY 패턴 조합을 사용한다.
접근 제한자(지정자) 변경
- 내부에서만 사용하고 외부로 공개할 필요가 없는 경우 public -> private 으로 변경
불필요한 final 키워드 제거
- 인수와 변수 선언에서 final 키워드 제거
- 실질적인 가치는 없으면서 코드만 복잡하게 만든다고 판단했기 때문이다.
- 로버트 시몬스는 "코드 전체에 final을 사용하라고 권장한다." 저자와 다른 의견이다.
중첩 if 문 통합
- if 문이 중첩되어 나올 경우 ||, && 등으로 하나로 만든다.
테스트 케이스에서만 호출 한다면 제거
- 일련의 리팩터링 작업은 나름대로 우아했지만
실제로 이 메서드를 사용하는 코드는 방금 수정한 테스트 케이스가 유일했다. - 그래서 저자는 테스트 케이스를 삭제했다.
물리적 의존성과 논리적 의존성
- 물리적 의존성은 없지만 논리적 의존성이 존재하는 경우도 있다.
- 뭔가가 구현에 논리적으로 의존한다변 물리적으로도 의존해야 마땅하다.
결론
- 다시 한 번 우리는 보이스카우트 규칙을 따랐다.
- 체크아웃한 코드보다 좀 더 깨끗한 코드를 체크인하게 되었다.
- 시간은 걸렸지만 가치있는 작업이었다.
- 테스트 커버리지가 증가했으며, 버그 몇 개를 고쳤으며, 코드 크기가 줄었고, 코드가 명확해졌다.
- 다음 사람은 우리보다 코드를 좀 더 쉽게 이해하리라.
- 그래서 우리 보다 코드를 좀 더 쉽게 개선하리라.
Backend Developer