IT Book Summary/Clean Code
Chapter 15, 16
laughcryrepeat
2021. 6. 27. 21:11
15 JUnit 들여다보기
JUnit 프레임워크
ComparisonCompactor 모듈
두 문자열을 받아 차이를 반환. ABCDE 와 ABCXDE 를 받아 <...B[X]D...> 를 반환.
ComparisonCompactorTest.java
package junit.tests.framework;
import junit.framework.ComparisonCompactor;
import junit.framework.TestCase;
public class ComparisonCompactorTest extends TestCase {
public void testMessage() {
String failure= new ComparisonCompactor(0, "b", "c").compact("a");
assertTrue("a expected:<[b]> but was:<[c]>".equals(failure));
}
public void testStartSame() {
String failure= new ComparisonCompactor(1, "ba", "bc").compact(null);
assertEquals("expected:<b[a]> but was:<b[c]>", failure);
}
public void testEndSame() {
String failure= new ComparisonCompactor(1, "ab", "cb").compact(null);
assertEquals("expected:<[a]b> but was:<[c]b>", failure);
}
public void testSame() {
String failure= new ComparisonCompactor(1, "ab", "ab").compact(null);
assertEquals("expected:<ab> but was:<ab>", failure);
}
public void testNoContextStartAndEndSame() {
String failure= new ComparisonCompactor(0, "abc", "adc").compact(null);
assertEquals("expected:<...[b]...> but was:<...[d]...>", failure);
}
public void testStartAndEndContext() {
String failure= new ComparisonCompactor(1, "abc", "adc").compact(null);
assertEquals("expected:<a[b]c> but was:<a[d]c>", failure);
}
public void testStartAndEndContextWithEllipses() {
String failure= new ComparisonCompactor(1, "abcde", "abfde").compact(null);
assertEquals("expected:<...b[c]d...> but was:<...b[f]d...>", failure);
}
public void testComparisonErrorStartSameComplete() {
String failure= new ComparisonCompactor(2, "ab", "abc").compact(null);
assertEquals("expected:<ab[]> but was:<ab[c]>", failure);
}
public void testComparisonErrorEndSameComplete() {
String failure= new ComparisonCompactor(0, "bc", "abc").compact(null);
assertEquals("expected:<[]...> but was:<[a]...>", failure);
}
public void testComparisonErrorEndSameCompleteContext() {
String failure= new ComparisonCompactor(2, "bc", "abc").compact(null);
assertEquals("expected:<[]bc> but was:<[a]bc>", failure);
}
public void testComparisonErrorOverlapingMatches() {
String failure= new ComparisonCompactor(0, "abc", "abbc").compact(null);
assertEquals("expected:<...[]...> but was:<...[b]...>", failure);
}
public void testComparisonErrorOverlapingMatchesContext() {
String failure= new ComparisonCompactor(2, "abc", "abbc").compact(null);
assertEquals("expected:<ab[]c> but was:<ab[b]c>", failure);
}
public void testComparisonErrorOverlapingMatches2() {
String failure= new ComparisonCompactor(0, "abcdde","abcde").compact(null);
assertEquals("expected:<...[d]...> but was:<...[]...>", failure);
}
public void testComparisonErrorOverlapingMatches2Context() {
String failure= new ComparisonCompactor(2, "abcdde", "abcde").compact(null);
assertEquals("expected:<...cd[d]e> but was:<...cd[]e>", failure);
}
public void testComparisonErrorWithActualNull() {
String failure= new ComparisonCompactor(0, "a", null).compact(null);
assertEquals("expected:<a> but was:<null>", failure);
}
public void testComparisonErrorWithActualNullContext() {
String failure= new ComparisonCompactor(2, "a", null).compact(null);
assertEquals("expected:<a> but was:<null>", failure);
}
public void testComparisonErrorWithExpectedNull() {
String failure= new ComparisonCompactor(0, null, "a").compact(null);
assertEquals("expected:<null> but was:<a>", failure);
}
public void testComparisonErrorWithExpectedNullContext() {
String failure= new ComparisonCompactor(2, null, "a").compact(null);
assertEquals("expected:<null> but was:<a>", failure);
}
public void testBug609972() {
String failure= new ComparisonCompactor(10, "S&P500", "0").compact(null);
assertEquals("expected:<[S&P50]0> but was:<[]0>", failure);
}
}
ComparisonCompactor.java
package junit.framework;
public class ComparisonCompactor {
private static final String ELLIPSIS = "...";
private static final String DELTA_END = "]";
private static final String DELTA_START = "[";
// 거슬리는 부분1 -> 멤버변수 앞에 붙은 f 접두어
private int fContextLength;
private String fExpected;
private String fActual;
private int fPrefix;
private int fSuffix;
public ComparisonCompactor(int contextLength, String expected, String actual) {
fContextLength = contextLength;
fExpected = expected;
fActual = actual;
}
// 거슬리는 부분4 -> 형식이 갖춰진 압축된 문자열을 반환하므로 함수 명칭 변경.
public String compact(String message) {
// 거슬리는 부분2 -> 함수 시작부에 캡슐화 되지 않은 조건문.
// 조건문을 메서드로 뽑아내 적절한 이름을 붙여야 함.
if (fExpected == null || fActual == null || areStringsEqual())
return Assert.format(message, fExpected, fActual);
// 거슬리는 부분5 -> 압출을 수행하는 부분을 함수로 따로 분리.
findCommonPrefix();
findCommonSuffix();
// 거슬리는 부분3 -> 변수의 명확한 이름 필요.
// f 제거한 멤버변수로 인해 지역변수의 이름 변경 필요.
String expected = compactString(fExpected);
String actual = compactString(fActual);
return Assert.format(message, expected, actual);
}
private String compactString(String source) {
// 거슬리는 부분8 -> +1을 사용하는 부분을 명확히 함. suffixLength 변수 할당해 사용.
String result = DELTA_START + source.substring(fPrefix, source.length() - fSuffix + 1) + DELTA_END;
// 거슬리는 부분9 -> 불필요한 if문 제거.
if (fPrefix > 0) result = computeCommonPrefix() + result;
if (fSuffix > 0) result = result + computeCommonSuffix();
return result;
}
// 거슬리는 부분6 -> 함수 사용이 일관적이지 않으므로 반환값을 사용. 변수에 할당.
private void findCommonPrefix() {
fPrefix = 0;
int end = Math.min(fExpected.length(), fActual.length());
for (; fPrefix < end; fPrefix++) {
if (fExpected.charAt(fPrefix) != fActual.charAt(fPrefix)) break;
}
}
// 거슬리는 부분7 -> findCommonSuffix는 findCommonPrefix가 prefixIndex를 계산하는것에 의존.
// 잘못된 순서로 호출하지 않게 해야함.
private void findCommonSuffix() {
int expectedSuffix = fExpected.length() - 1;
int actualSuffix = fActual.length() - 1;
for (; actualSuffix >= fPrefix && expectedSuffix >= fPrefix; actualSuffix--, expectedSuffix--) {
if (fExpected.charAt(expectedSuffix) != fActual.charAt(actualSuffix)) break;
}
fSuffix = fExpected.length() - expectedSuffix;
}
private String computeCommonPrefix() {
return (fPrefix > fContextLength ? ELLIPSIS : "") +
fExpected.substring(Math.max(0, fPrefix - fContextLength), fPrefix);
}
private String computeCommonSuffix() {
int end = Math.min(fExpected.length() - fSuffix + 1 + fContextLength,
fExpected.length());
return fExpected.substring(fExpected.length() - fSuffix + 1, end) +
(fExpected.length() - fSuffix + 1 < fExpected.length() -
fContextLength ? ELLIPSIS : "");
}
private boolean areStringsEqual() {
return fExpected.equals(fActual);
}
}
거슬리는 부분을 리팩토링한 소스
package junit.framework;
public class ComparisonCompactor {
private static final String String ELLIPSIS = "...";
private static final String DELTA_END = "]";
private static final String DELTA_START = "[";
private int contextLength;
private String expected;
private String actual;
private int prefixLength;
private int suffixLength;
public ComparisonCompactor( int contextLength, String expected, String actual){
this.contextLength = contextLength;
this.expected = expected;
this.actual = actual;
}
public String formatCompactedComparison(String message) {
String compactExpected = expected;
String compactActual = actual;
if (shouldBeCompacted()) {
findCommonPrefixAndSuffix();
compactExpected = compact(expected);
compactActual = compact(actual);
}
return Assert.format(message, compactExpected, compactActual);
}
private boolean shouldBeCompacted() {
return !shouldNotBeCompacted();
}
private boolean shouldNotBeCompacted() {
return expected == null || actual == null || expected.equals(actual);
}
private void findCommonPrefixAndSuffix() {
findCommonPrefix();
suffixLength = 0;
for (; !suffixOverlapsPrefix(); suffixLength++) {
if (charFromEnd(expected, suffixLength) != charFromEnd(actual, suffixLength)
)
break; }
}
private char charFromEnd(String s, int i) {
return s.charAt(s.length() - i - 1);
}
private boolean suffixOverlapsPrefix() {
return actual.length() - suffixLength <= prefixLength ||
expected.length() - suffixLength <= prefixLength;
}
private void findCommonPrefix() {
prefixLength = 0;
int end = Math.min(expected.length(), actual.length());
for (; prefixLength < end; prefixLength++)
if (expected.charAt(prefixLength) != actual.charAt(prefixLength)) break;
}
private String compact(String s) {
return new StringBuilder().append(startingEllipsis()) .append(startingContext()) .append(DELTA_START) .append(delta(s)) .append(DELTA_END) .append(endingContext()) .append(endingEllipsis()) .toString();
}
private String startingEllipsis() {
return prefixLength > contextLength ? ELLIPSIS : "";
}
private String startingContext() {
int contextStart = Math.max(0, prefixLength - contextLength);
int contextEnd = prefixLength;
return expected.substring(contextStart, contextEnd);
}
private String delta(String s) {
int deltaStart = prefixLength;
int deltaEnd = s.length() - suffixLength;
return s.substring(deltaStart, deltaEnd);
}
private String endingContext() {
int contextStart = expected.length() - suffixLength;
int contextEnd = Math.min(contextStart + contextLength, expected.length());
return expected.substring(contextStart, contextEnd);
}
private String endingEllipsis() {
return (suffixLength > contextLength ? ELLIPSIS : "");
}
}
분석함수가 먼저 나오고 조합함수가 그 뒤를 이어 나옴.
결론
리팩터링은 코드가 어느 수준에 이를때까지 많은 시행착오를 반복하는 작업.
세상에 개선이 불필요한 모듈은 없음.
16 SerialDate 리팩터링
JCommon 라이브러리
org.jfree.date 패키지
SerialDate 클래스 : 날짜를 표현하는 자바클래스
첫째, 돌려보자 First, Make It Work.
SerialDateTests 클래스는 커버리지가 50% 정도임.
많은 코드가 주석 처리가 되어있고, 실패한 테스트 케이스임.
클로버로 확인한 테스트 커버리지 패턴.
주석으로 처리한 코드에 실패하는 패턴.
변수 adjust 가 항상 음수이므로 틀린 알고리즘.
모든 테스트 케이스를 통과 시킨 후 리팩터링 시작.
둘째, 고쳐보자 Then Make It Right
코드를 고칠때마다 JCommon 단위 테스트와 다시 짠 단위테스트를 실행하여 확인.
소스코드 제어도구를 이용하니, 변경이력은 제거 가능.
클래스 이름이 SerialDate 인 이유는 seral number 를 사용해 클래스를 구현했기 때문.
일련번호라는 용어가 적합하지 않고, 구현을 숨기는 추상화된 이름으로 적합하도록 DayDate 용어 사용.
MonthConstants 클래스는 달을 정의하는 static final 상수 모을을 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;
}
기반클래스 (부모 클래스)는 파생클래스(자식클래스) 를 몰라야 함.
ABSTRACT FACTORY 패턴을 적용해 DayDateFactory를 만들고
DayDateFactory는 DayDate 인스턴스를 생성해 구현관련 응답.
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, 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 이름으로 바꿈.
추상메서드로 위임하는 정적 메서드는
SINGLETON, DECORATOR, 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) {
return 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;
}
}
변수 이름만으로 의미가 확실하면 주석은 삭제.
특정 구현에 의존하지 않는 변수는 enum으로 변경.
public enum WeekInMonth {
FIRST(1),
SECOND(2),
THIRD(3),
FOURTH(4),
LAST(0);
public final int index;
WeekInMonth(int index) {
this.index = index;
}
}
복잡한 알고리즘은 임시변수설명 explaning temporary variables 를 사용. 가독성 높임.
객체를 변경시킨다고 오해할 수 있는 메소드 이름 변경. addDays -> plusDays
추상메서드여야 하는지 판별하여 DayDate 클래스로 끌어올림.
지금까지 한 작업 정리
- 주석 고치고 개선.
- enum을 독자 소스파일로 옮김.
- 정적 변수와 정적 메서드를 DateUtil 새 클래스로 옮김.
- 일부 추상 메서드를 DayDate 클래스로 끌어올림.
- Month.make 를 Month.fromInt로 변경.
enum에 toInt()접근자 생성해 index필드를 private로 정의. - 새 메서드를 생성해 중복을 없앰
- 날짜나 월을 지칭하는 숫자 1을 Month.JANUARY.toInt() 혹은 Day.SUNDAY.toInt() 로 변경.
결론
보이스카우트 규칙.
테스트 커버리지가 증가 했고, 버그를 고치고, 코드 크기가 줄고, 코드가 명확해짐.