김주엽

1. 📌 핵심 개념 정리

✅ 요약하기

JUnit 프레임워크

JUnit은 저자가 많지만 켄트 벡과 에릭 감마 두 사람이 아틀란타 행 비행기를 타고 가다 만들었다.

저자가 챕터에서 소개할 코드는 ComparisonCompactor모듈로 문자열 비교 오류를 파악할 때 유용한 모듈이다.

예를 들어 ABCDE, ABXDE를 입력받으면 <...B[X]D...>를 반환한다

ComparisonCompactor 모듈 코드

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

  public String compact(String message) {
    if (fExpected == null || fActual == null || areStringsEqual()) {
      return Assert.format(message, fExpected, fActual);
    }
    findCommonPrefix();
    findCommonSuffix();
    String expected = compactString(fExpected);
    String actual = compactString(fActual);
    return Assert.format(message, expected, actual);
  }

  private String compactString(String source) {
    String result = DELTA_START + source.substring(fPrefix, source.length() - fSuffix + 1) + DELTA_END;
    if (fPrefix > 0) {
      result = computeCommonPrefix() + result;
    }
    if (fSuffix > 0) {
      result = result + computeCommonSuffix();
    }
    return result;
  }

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

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

저자들이 모듈을 아주 좋은 상태로 남겨두었지만 보이스카우트 규칙에 맞게 더 깨끗한 코드를 만들어보자.

접두어 f

가장 먼저 눈에 거슬리는 부분은 멤버 변수 앞에 붙인 접두어 'f'이다.

오늘날 개발 환경에서는 변수 이름에 범위를 명시할 필요가 없으므로 제거한다.

개선 전

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

개선 후

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

캡슐화되지 못한 조건문

compact 메서드 시작부를 보면 캡슐화되지 못한 조건문이 존재한다.

의도를 명확히 표현하기 위해서 조건문을 캡슐화하고 적절한 이름을 붙여준다.

또한 부정문은 긍정문보다 이해하기가 약간 더 어렵기에 긍정문으로 만들어 조건문을 반전한다.

개선 전

public String compact(String message) {
    if (fExpected == null || fActual == null || areStringsEqual()) {
      return Assert.format(message, fExpected, fActual);
    }
    ...
}

개선 후

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

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

함수 이름 변경

위 코드를 보면 canBeCompacted 메서드가 false이면 압축하지 않는다.

함수는 단순히 압축된 문자열이 아니라 형식을 갖춘 문자열을 반환한다.

따라서 적합한 이름으로 메서드 명을 바꿀 필요가 있다.

개선 전

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

개선 후

// 멤버 변수로 승격시킴
private String compactExpected;
private String compactActual;

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

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

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

private int findCommonSuffix(int prefixIndex) {
    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;
}

압축하는 기능을 compactExpectedAndActual 메서드에게 전적으로 맡기고 함수를 분리해 코드를 개선시켰다.
findCommonSuffix 메서드는 findCommonPrefix가 prefixIndex를 계산하는 것에 의존한다.
따라서 숨겨진 시간적인 결합을 노출시키고자 prefixIndex를 인수로 넘긴다.

깨끗하게 고치기

prefixIndex를 인수로 전달하는 방식은 다소 자의적이다.

prefixIndex가 필요한 이유가 드러나지 않아 다른 프로그래머가 원래대로 되돌릴 가능성이 존재한다.

개선 전

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

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

private int findCommonSuffix(int prefixIndex) {
    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;
}

개선 후

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

private void findCommonPrefixAndSuffix() { 
    findCommonPrefix(); 
    int suffixLength = 1; 
    for (; !suffixOverlapsPrefix(suffixLength); suffixLength++) {
        if (charFromEnd(expected, suffixLength) != 
                charFromEnd(actual, suffixLength))
            break;
    }
    suffixIndex = suffixLength;
}

private char charFromEnd(String s, int i) { 
    return s.charAt(s.length()-i);
}

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

결론
- 코드를 리팩터링 하다 보면 원래 했던 변경을 되돌리는 경우가 흔하다.
- 리팩터링은 코드가 어느 수준에 이를 때까지 수많은 시행착오를 반복하는 작업이기 때문이다.
- 보이스카우트 규칙을 통해 모듈을 처음보다 조금 더 깨끗하게 수정할 수 있었다.
- 코드를 처음보다 조금 더 깨끗하게 만드는 책임은 우리 모두에게 있다.

2. 🤔 이해가 어려운 부분

🔍 질문하기

캡슐화되지 못한 조건문
- 어려웠던 부분
  저자는 조건문을 캡슐화하기 위해 분리를 했는데 불필요한 분리가 아닌가?
- 궁금한 점
  메서드와 클래스를 너무 작게 쪼개면 Shotgun Surgery 형태의 코드 구조가 나오게 될 것 같은데
  저자가 왜 이렇게 코드를 리팩터링했는지 궁금하다.

3. 📚 참고 사항

📢 논의하기

관련된 자료가 있다면 공유하고, 더 깊이 논의하고 싶은 아이디어나 의견을 정리합니다.

관련 자료 공유
- 추가 자료
  Shotgun Surgery