[CLEAN CODE] JUnit 들여다보기(JUnit Internals)

📢 JUnit 프레임워크

The JUnit Framework

ComparisonCompactor 모듈을 살펴보자.

1. 인코딩을 피하라 [N6]
이름에 유형 정보나 점위 정보를 넣어서는 안된다.
멤버 변수 앞에 붙인 접두어 f처럼 변수 이름에 범위를 명시할 필요가 없으므로 모두 제거한다.

// Before
private int fContextLength;
private String fExpected;
private int fActual;
private int fPrefix;
private int fSuffix;

// After
private int contextLength;
private String expected;
private int actual;
private int prefix;
private int suffix;

2. 조건을 캡슐화하라 [G28]
부울 논리는 이해하기 어려우므로 의도를 분명히 밝히는 함수로 표현하라.
조건문을 메서드로 뽑아내 적절한 이름을 붙인다.

// Before
public String compact(String message) {
    if(expected == null || actual == null || areStringsEquls())
        return Assert.format(message, expected, actual);
    // 이하 생략
}

// After
public String compact(String message) {
    if(shouldNotCompact())
        return Assert.format(message, expected, actual);
    // 이하 생략
}

private boolean shouldNotCompact() {
    return expected == null || actual == null || areStringsEquls();
}

3. 명확한 이름 [N4]
함수나 변수의 목적을 명확히 밝히는 이름을 선택한다.

// Before
String expected = compactString(this.expected);
String actual = compactString(this.actual);

// After
String compactExpected = compactString(expected);
String compactActual = compactString(actual);

4. 부정 조건은 피하라 [G29]
부정 조건은 긍정 보건보다 이해하기 어려우므로, 가능하면 긍정 조건으로 표현한다.

// Before
public String compact(String message) {
    if(shouldNotCompact())
        return Assert.format(message, expected, actual);
    // 이하 생략
}

private boolean shouldNotCompact() {
    return expected == null || actual == null || areStringsEquls();
}

// After
public String compact(String message) {
    if(canBeCompacted()) {
        // 이하 생략
    }
}

private boolean canBeCompacted() {
    return expected != null && actual != null && !areStringsEquls();
}

5. 이름으로 부수 효과를 설명하라 [N7]
함수, 변수, 클래스가 하는 일을 모두 기술하는 이름을 사용한다.

// Before
public String compact(String message) {
    if(canBeCompacted()) {
       // 이하 생략
    }
}

// After
public String formatCompactedComparison(String message) {
    if(canBeCompacted()) {
        // 이하 생략
    }
}

6. 함수는 한 가지만 해야 한다 [G30]
함수는 한 가지만 수행하는 좀 더 작은 함수 여럿으로 나눠야 마땅하다.

// Before
public String formatCompactedComparison(String message) {
    if(canBeCompacted()) {
        findCommonPrefix();
        findCommonSuffix();
        String compactExpected = compactString(expected);
        String compactActual = compactString(actual);
        return Assert.format(message, compactExpected, compactAtucal);
    } else {
        return Assert.format(message, expected, actual);
    }
}

// After
private String compactExpected;
private String compactActual;

public String formatCompactedComparison(String message) {
    if(canBeCompacted()) {
        compactExpectedAndActual();
        return Assert.format(message, compactExpected, compactAtucal);
    } else {
        return Assert.format(message, expected, actual);
    }
}

private void compactExpectedAndActual() {
    findCommonPrefix();
    findCommonSuffix();
    compactExpected = compactString(expected);
    compactActual = compactString(actual);
}

7. 일관성 부족 [N7]

// Before
private void compactExpectedAndActual() {
    findCommonPrefix();
    findCommonSuffix();
    compactExpected = compactString(expected);
    compactActual = compactString(actual);
}

// After
private void compactExpectedAndActual() {
    prefixIndex = findCommonPrefix();
    suffixIndex = findCommonSuffix();
    compactExpected = compactString(expected);
    compactActual = compactString(actual);
}

8. 서술적인 이름을 사용하라 [N1]

// Before
private void findCommonPrefix() {
    prefix = 0;
    int end = Math.min(expected.length(), actual.length());
    for (; prefix <end; prefix++) {
        if (expected.charAt(prefix) != actual.charAt(prefix))
            break;
    }
}

private void findCommonSuffix() {
    int expectedSuffix = expected.length() - 1;
    int actualSuffix = actual.length() - 1;
    for (; actualSuffix >= prefix && expectedSuffix >= prefix; actualSuffix--, expectedSuffix--) {
        if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix))
            break;
    }
    suffix = expected.length() - expectedSuffix;
}

// After
private int findCommonPrefix() {
    int prefixIndex = 0;
    int end = Math.min(expected.length(), actual.length());
    for (; prefixIndex <end; prefixIndex++) {
        if (expected.charAt(prefixIndex) != actual.charAt(prefixIndex))
            break;
    }
    return prefixIndex;
}

private int findCommonSuffix() {
    int expectedSuffix = expected.length() - 1;
    int actualSuffix = actual.length() - 1;
    for (; actualSuffix >= prefixIndex && expectedSuffix >= prefixIndex; actualSuffix--, expectedSuffix--) {
        if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix))
            break;
    }
    return expected.length() - expectedSuffix;
}

9. 숨겨진 시간적인 결합
때로는 시간적인 결합이 필요하지만, 시간적인 결합은 숨겨서는 안 된다.
함수를 짤 때는 함수 인수를 적절히 배치해 함수가 호출되는 순서를 명백히 드러낸다.

// Before
private void compactExpectedAndActual() {
    prefixIndex = findCommonPrefix();
    suffixIndex = findCommonSuffix();
    compactExpected = compactString(expected);
    compactActual = compactString(actual);
}

private int findCommonSuffix() {
    // 이하 생략
}

// After
private void compactExpectedAndActual() {
    prefixIndex = findCommonPrefix();
    suffixIndex = findCommonSuffix(prefixIndex);
    compactExpected = compactString(expected);
    compactActual = compactString(actual);
}

private int findCommonSuffix(int prefixIndex) {
    // 이하 생략
}

10. 일관성을 유지하라
코드 구조를 잡을 때는 이유를 고민하고, 그 이유를 코드 구조로 명백히 표현하라.

// Before
private void compactExpectedAndActual() {
    prefixIndex = findCommonPrefix();
    suffixIndex = findCommonSuffix(prefixIndex);
    compactExpected = compactString(expected);
    compactActual = compactString(actual);
}

// After
private void compactExpectedAndActual() {
    findCommonPrefixAndSuffix();
    compactExpected = compactString(expected);
    compactActual = compactString(actual);
}

private void findCommonPrefixAndSuffix() {
    findCommonPrefix();
    int expectedSuffix = expected.length() - 1;
    int actualSuffix = actual.length() - 1;
    for (; actualSuffix >= prefixIndex && expectedSuffix >= prefixIndex; actualSuffix--, expectedSuffix--) {
        if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix))
            break;
    }
    suffixIndex = expected.length() - expectedSuffix;
}

11. 경계 조건을 캡슐화하라
경계 조건은 여기저기에서 처리하지 않고, 한 곳에서 별도로 처리한다.
코드 여기저기에 +1 이나 -1을 흩어놓지 않는다.

12. 죽은 코드 [G9]
실행되지 않는 코드는 죽은코드이다.
불필요한 if문을 제거하고 구조를 다듬어 좀 더 깔끔하게 만들자.

최종 코드

package junit.framework;

public class ComparisonCompactor {

    private static final 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 = comapct(expected);
            compactActual = comapct(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); suffixLength++) {
            if (charFromEnd(expected, suffixLength) != charFromEnd(actual, suffixLength)) {
                break;
            }
        }
    }

    private boolean suffixOverlapsPrefix(int suffixLength) {
        return actual.length() = suffixLength <= prefixLength || expected.length() - suffixLength <= prefixLength;
    }

    private void findCommonPrefix() {
        int prefixIndex = 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() {
        prefixIndex > 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 : "");
    }
}

📢 결론

Conclusion

보이스카우트 규칙을 지키며 처음보다 조금 더 깨끗해진 모듈을 만들었다.
코드를 처음보다 조금 더 깨끗하게 만드는 책임은 우리 모두에게 있다.

📖 느낀점
리팩터링은 수많은 시행착오를 겪어야 하는 것 같다.
이렇게도 해보고 저렇게도 해보고 계속 코드를 다듬는 행위를 차근 차근 따라가니까 흥미로웠다.