15장 JUnit 들여다보기
이 장에서는 JUnit 프레임워크에서 가져온 코드를 평가한다.
JUnit 프레임워크
우리가 살펴볼 모듈은 문자열 비교 오류를 파악할 때 유용한 ComparisionCompactor라는 모듈로, 두 문자열을 받아 차이를 반환한다. 예를 들어, ABCDE와 ABXDE를 받아 <..B[x]D..>를 반환한다.
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);
}
}
전반적으로 훌륭한 모듈이다. 그렇지만 보이스카우트 규칙에 따르면 우리는 처음 왔을 때보다 더 깨끗하게 해놓고 떠나야 한다. 위 코드를 개선해 보자.
접두어 없애기
private int contextLength;
private String expected;
private String actual;
private int prefix;
private int suffix;private int fContextLength; 오늘 날의 개발환경에서는 이렇게 f를 붙여서 변수 이름에 범위를 명시할 필요가 없다. 그러므로 접두어 f를 모두 제거하자.
조건문을 캡슐화하자.
if (Expected == null || Actual == null || areStringsEqual()) {
return Assert.format(message, Expected, Actual);
}조건문을 메소드로 뽑아내 적절한 이름을 붙이자.
if (shouldNotCompact()) {
return Assert.format(message, expected, actual);
}이름을 명확히 붙이자.
String expected = compactString(fExpected);
String actual = compactString(fActual);이전
String compactExpected = compactString(expected);
String compactActual = compactString(actual);이후
부정문을 긍정문으로 표현하자.
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();
}
적절한 함수 이름을 붙이자.
public String formatCompatedComparison(String message) {
}compact보다는 formatCompatedComparison이라는 이름이 더 저갑하다.
함수는 한 가지 일만 한다.
formatCompatedComparison는 지금 여러 가지 일을 하고 있다.(형식 맞추기+압축하기) 한 가지 일만 하도록 함수를 둘로 쪼개자.
public String formatCompactedComparison(String message) {
if (canBeCompacted()) {
compactExpectedAndActual();
return Assert.format(message, compactExpected, compactActual);
} else {
return Assert.format(message, expected, actual);
}
}
private compactExpectedAndActual() {
findCommonPrefix();
findCommonSuffix();
compactExpected = compactString(expected);
compactActual = compactString(actual);
}함수를 일관적으로 사용하자.
private compactExpectedAndActual() {
prefixIndex = findCommonPrefix();
suffixIndex = findCommonSuffix();
String compactExpected = compactString(expected);
String 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(prefixIndex)) {
break;
}
}
return prefixIndex;
}
private int findCommonSuffix() {
int expectedSuffix = expected.length() - 1;
int actualSuffix = actual.length() - 1;
for (; actualSuffix >= prefixIndex && expectedSuffix >= prefix; actualSuffix--, expectedSuffix--) {
if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix)) {
break;
}
}
return expected.length() - expectedSuffix;
}새 함수에서 마지막 두 줄은 변수를 반환하지만 첫째 줄과 둘째 줄은 반환값이 없다. 함수 사용 방식이 일관적이지 못하므로 findCommonPrefix와 findCommonSuffix를 변경해 접두어 값과 접미어 값을 반환한다.
숨겨진 시각적인 결합
private compactExpectedAndActual() {
prefixIndex = findCommonPrefix();
suffixIndex = findCommonSuffix(prefixIndex);
String compactExpected = compactString(expected);
String compactActual = compactString(actual);
}
private int findCommonSuffix(int prefixIndex) {
int expectedSuffix = expected.length() - 1;
int actualSuffix = actual.length() - 1;
for (; actualSuffix >= prefixIndex && expectedSuffix >= prefix; actualSuffix--, expectedSuffix--) {
if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix)) {
break;
}
}
return expected.length() - expectedSuffix;
}만약 findCommonPrefix와 findCommonSuffix를 잘못된 순서로 호출하면 밤샘 디버깅이라는 고생문이 열린다. 그러므로 시간 결합을 외부에 노출하고자 findCommonSuffix를 고쳐 prefixIndex를 인수로 넘겼다.
일관성을 유지하라
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 suffixOverlapsPrefix(int suffixLength) {
return actual.length() = suffixLength < prefixLength || expected.length() - suffixLength < prefixLength;
}그런데 생각해보니 위의 방식이 지저분해 보여서 썩 마음에 들지 않는다..findcommonPrefix와 findCommonSuffix를 원래대로 되돌리고 findCommonSuffix라는 이름을 findCommonPrefixSuffix로 바꾸고, findCOmmonPrefixAndSuffix에서 가장 먼저 findCommonPrefix를 호출한다. 그러면 두 함수를 호출하는 순서가 앞서 고친 코드보다 훨씬 더 명확해진다.
경계 조건을 캡슐화하라
코드를 고치고 나니까 suffixIndex가 실제로는 접미어 길이라는 사실이 드러난다. 이름이 적절하지 못하다는 말이다. 실제로 suffixIndex는 0에서 시작하지 않고 1에서 시작하므로 "
length"가 더 합당하다.
코드 참고: https://han.gl/draPe
개발 공부합니다. 파이팅!