JUnit은 자바 프레임워크 중 가장 유명하다. 여기서는 JUnit 프레임워크에서 가져온 코드를 리팩터링 해본다.
JUnit 프레임워크
JUnit의 다양한 모듈 중 ComparisonCompactor 모듈을 살펴보자. 문자열 비교 오류를 파악할 때 유용하다. ABCDE와 ABXDE를 받아 <...B[X]D...> 를 반환한다.
테스트 케이스
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 testComparisonErrorOverlappingMatches() {
String failure = new ComparisonCompactor(0, "abc", "abbc").compact(null);
assertEquals("expected:<...[]...> but was:<...[b]...>", failure);
}
public void testComparisonErrorOverlappingMatchesContext() {
String failure = new ComparisonCompactor(2, "abc", "abbc").compact(null);
assertEquals("expected:<ab[]c> but was:<ab[b]c>", failure);
}
public void testComparisonErrorOverlappingMatches2() {
String failure = new ComparisonCompactor(0, "abcdde", "abcde").compact(null);
assertEquals("expected:<...[d]...> but was:<...[]...>", failure);
}
public void testComparisonErrorOverlappingMatches2Context() {
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 모듈에 대한 코드 커버리지 분석을 수행하여 테스트케이스가 모든 행, 모든 if문, 모든 for문을 실행할 수 있도록 하였다.
ComparisonCompactor.java 원본
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);
}
}ComparisonCompactor.java 리팩터링
저자들이 아주 좋은 상태로 모듈을 남겨 두었지만 우리는 보이스카우트 규칙에 따라 처음 왔을 때 보다 더 깨끗하게 해 놓고 떠나야한다. 개선 방법들은 다음과 같다.
접두어 f 제거[N6]
가장 먼저 거슬리는 부분은 멤버 변수 앞에 붙인 접두어 f다. 오늘날 사용하는 개발 환경에서는 이처럼 변수 이름에 범위를 명시할 필요가 없다.
private int contextLength;
private String expected;
private String actual;
private int prefix;
private int suffix;조건문 캡슐화[G28]
compact 함수 시작부에 캡슐화되지 않은 조건문이 있다.
의도를 명확히 표현하려면 조건문을 메서드로 뽑아내 적절한 이름을 붙여 캡슐화해야 한다.
//개선 전
public String compact(String message) {
if (expected == null || actual == null || areStringsEqual()) {
return Assert.format(message, expected, actual);
}
findCommonPrefix();
findCommonSuffix();
String expected = compactString(this.expected);
String actual = compactString(this.actual);
return Assert.format(message, expected, actual);
}//개선 후
public String compact(String message) {
if (shouldNotCompact()) {
return Assert.format(message, expected, actual);
}
findCommonPrefix();
findCommonSuffix();
String expected = compactString(this.expected);
String actual = compactString(this.actual);
return Assert.format(message, expected, actual);
}
private boolean shouldNotCompact() {
return expected == null || actual == null || areStringsEqual();
}명확한 이름 사용[N4]
함수에 이미 expected라는 지역 변수가 있는데 fExpected에서 f를 빼버리는 바람에 this.expected라는 식으로 사용하게 되었다. 멤버변수와 이름이 같은 변수를 사용할 이유가 없다.
String compactExpected = compactString(expected);
String compactActual = compactString(actual);부정 조건 미사용[G29]
부정문은 긍정문보다 이해하기 약간 더 어렵다. 그러므로 첫 문장 if를 긍정으로 만들어 조건문을 반전한다.
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();
}이름으로 부수 효과 설명[N7]
문자열을 압축하는 함수라지만 실제로 canBeCompacted가 false이면 압축하지 않는다. 그리고 단순한 압축 문자열이 아닌 형식이 갖춰진 문자열을 반환한다. 인수를 고려하면 가독성이 훨씬 좋아진다.
public String formatCompactedComparison(String message) {함수는 한 가지 업무만 수행[G30]
if 문 안에서 예상 문자열과 실제 문자열을 진짜로 압축한다.
이 부분을 분리하여 형식을 맞추는 작업은 formatCompactedComparison, 압축은 compacteExpectedAndActual이 수행하도록 변경한다.
...
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 compactExpectedAndActual() {
findCommonPrefix();
findCommonSuffix();
compactExpected = compactString(expected);
compactActual = compactString(actual);
}일관적 함수 사용[G11]
새 함수에서 마지막 두 줄은 변수를 반환하지만 첫째 줄과 둘째 줄은 반환값이 없다.
함수 사용 방식이 일관적이지 못하므로 findCommonPrefix와 findCommonSuffix를 변경해 접두어 값과 접미어 값을 반환한다.
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;
}정확한 이름 사용[N1]
변수가 하는 일과 관련되는 정확한 이름으로 다시 명명한다.
private int prefixIndex;
private int suffixIndex;숨겨진 시각적인 결합 없애기[G31][G32]
공통되는 문자열을 앞에서부터 찾은 다음 뒤에서부터 찾도록 만들어져 있지만 호출자가 알기 어렵다. 따라서 다음과 같이 내용을 수정한다.
- findCommonPrefix 호출
- findCommonSuffix 호출하는 메소드 생성
- findCommonSuffixAndSuffix();
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;
}정확한 이름 사용[G33]
변수가 하는 일과 관련되는 정확한 이름으로 다시 명명한다.
private int suffixLength;중간 점검
public class ComparisonCompactor {
...
private int suffixLength;
...
private void findCommonPrefixAndSuffix() {
findCommonPrefix();
suffixLength = 0;
for (; suffixOverlapsPrefix(suffixLength); 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(int suffixLength) {
return actual.length() = suffixLength <= prefixLength || expected.length() - suffixLength <= prefixLength;
}
...
private String compactString(String source) {
String result = DELTA_START + source.substring(prefixLength, source.length() - suffixLength) + DELTA_END;
if (prefixLength > 0) {
result = computeCommonPrefix() + result;
}
if (suffixLength > 0) {
result = result + computeCommonSuffix();
}
return result;
}
...
private String computeCommonSuffix() {
int end = Math.min(expected.length() - suffixLength + contextLength, expected.length());
return expected.substring(expected.length() - suffixLength, end) + (expected.length() - suffixLength < expected.length() - contextLength ? ELLIPSIS : "");
}
}불필요한 코드 제거[G9]
불필요한 if문들을 제거하고, compactString 구조를 다듬어 좀 더 깔끔하게 만든다.
private String compactString(String source) {
return computeCommonPrefix() + DELTA_START + source.substring(prefixLength, source.length() - suffixLength) + DELTA_END + computeCommonSuffix();
}ComparisonCompactor.java 최종본
모듈은 일련의 분석 함수와 일련의 조합 함수로 나뉘고, 전체 함수는 위상적으로 정렬했으므로 각 함수가 사용된 직후에 정의된다. 분석 함수가 먼저 나오고 조합 함수가 그 뒤를 이어서 나온다.
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 : "");
}
}결론
저자들은 우수한 모듈을 만들었다. 하지만 세상에 개선이 불필요한 모듈은 없다.
코드를 처음보다 조금 더 깨끗하게 만드는 책임은 우리 모두에게 있다.
개발공부 요약노트