JUnit을 작성하고 테스트 코드를 통해 리팩토링을 하는 과정에 대하여
* JUnit : 자바 프로그래밍 언어용 유닛 테스트 프레임워크
📕 JUnit 프레임워크
예시 코드는 두 문자열을 받아 차이를 반환해주는 코드이다.
💻 15-1 ComparisonCompactorTest.java
아래 코드는 ComparisonCompactor 모듈에 대한 커버리지를 분석해주는 테스트 케이스이다.
* 코드 커버리지 : 소프트웨어의 테스트 케이스가 얼마나 충족되었는지를 나타내는 지표로, 테스트를 진행하였을 때 '코드 자체가 얼마나 실행되었느냐' 에 해당
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);
}
}위의 테스트 케이스로 코드 커버리지 분석을 하면 100%가 된다. 이는 테스트 케이스가 모든 행, 모든 if, for문을 실행한다는 것을 의미한다. 따라서 모듈이 올바로 동작한다고 볼 수 있게 되었다.
💻 15-2 ComparisonCompactor.java
아래 코드는 ComparisonCompactor 모듈이다.
코드 분리가 적절하게 되어있고, 구조가 단순하다.
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);
}
}위의 코드도 좋은 코드지만, 보이스카우트 규칙에 따라 더 깨끗한 코드로 만들어야 한다.
* 보이스카우트 규칙 : 체크 아웃때보다 더 좋은 코드를 체크인 한다. 즉, 코드 정리를 거듭할수록 더 좋은 코드가 되어야 한다는 의미
더 좋은 코드를 위해 아래와 같은 규칙을 적용한다.
1. 인코딩을 피하라 (403)
이름에 유형 정보나 범위 정보를 나타내는 접두어를 붙이는 것은 불필요하다.
따라서 멤버 변수 앞에 붙인 접두어 f를 제거한다.
private int contextLength;
private String expected;
private String actual;
private int prefix;
private int suffix;2. 조건을 캡슐화 하라. (403)
아래 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();
}3. 가능하다면 표준 명명법을 사용하라 (402)
compact 함수에서 사용하는 this.expected와 this.actual도 이미 지역변수가 있기 때문에 좋지 못하다. 이는 fExpected에서 f를 빼버리는 바람에 생긴 결과다. (접두어 제거하면서 문제 발생)
멤버 변수와 이름이 똑같은 변수가 다른 기능을 한다면 이름을 구분해서 붙여준다.
String compactExpected = compactString(expected);
String compactActual = compactString(actual);4. 부정 조건은 피하라 (389)
부정문은 긍정문보다 이해하기 약간 더 어렵다. 그러므로 첫 문장 if를 긍정으로 만들어 조건문을 반전한다.
public String compact(String message) {
if (canBeCompacted()) { // if (shouldNotCompact()) {
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();
// return expected == null || actual == null || areStringsEqual();
}5. 이름으로 부수 효과를 설명하라 (404)
compact 함수는 문자열 압축을 위한 함수이지만, 실제로 canBeCompacted가 false이면 압축하지 않는다. 이렇게 부가 단계가 숨겨지는 이름은 좋지 못하다. 이름은 함수, 변수, 클래스가 하는 일을 모두 담고 있는 것으로 사용해야 한다.
그리고 단순한 압축 문자열이 아닌 형식이 갖춰진 문자열을 반환하기 때문에 실제로는 formatCompactedComparison이라는 이름이 적합하다.
public String formatCompactedComparison(String message) {
// public String compact(String message) {6. 함수는 한 가지만 해야 한다 (389)
formatCompactedComparison 내의 if문 안에서는 예상 문자열과 실제 문자열을 압축한다. 이 부분을 따로 빼내서 compactExpectedAndActual이라는 메서드로 만들어 압축을 수행하도록하고, formatCompactedComparison 함수는 형식을 맞추는 작업만 할 수 있도록 한다.
그렇게 되면 각 함수는 하나의 기능만 수행하게 된다.
...
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);
}7. 일관성 부족 (376)
앞서 compactExpected와 compactActual을 멤버 변수로 변경 해주었다.
compactExpectedAndActual 함수에서 마지막 두 줄은 변수를 반환하지만 첫째 줄과 둘째 줄은 반환 값이 없다. (일관성 부족) 그래서 findCommonPrefix와 findCommonSuffix를 변경해 접두어 값과 접미어 값을 반환할 수 있도록 함수를 void에서 in로 바꿔주고 return 값을 추가해준다.
private compactExpectedAndActual() {
prefixIndex = findCommonPrefix(); // findCommonPrefix();
suffixIndex = findCommonSuffix(); // findCommonSuffix();
compactExpected = compactString(expected);
compactActual = compactString(actual);
}
...
private int findCommonPrefix() // void
return prefixIndex;
private int findCommonSuffix() // void
return expected.length() - expectedSuffix;
...
8. 서술적인 이름을 사용하라 (376)
멤버 변수가 색인 위치를 나타내기 때문에 더 명확한 의미를 주기 위해 prefix, suffix로 선언되어있던 것을 prefixIndex, suffixIndex로 수정한다.
private int prefixIndex;
private int suffixIndex;9. 숨겨진 시각적인 결합 (390)
findCommonSuffix를 살펴보면 숨겨진 시간적인 결합이 존재한다. 다시 말해, findCommonSuffix는 findCommonPrefix가 prefixIndex를 계산한다는 사실에 의존한다는 것을 의미한다. 만약 순서가 잘못될 경우 끝없는 디버깅을 해야한다. 그래서 시간 결합을 외부에 노출하고자 findCommonPrefix를 고쳐 prefixIndex를 인수로 넘겼다.
* 시각적인 결합 나타내기 : 함수를 짤때는 함수 인수를 적절하게 배치해서 함수가 호출되는 순서를 명백히 드러낸다.
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;
}10. 일관성을 유지하라 (391)
코드를 구조를 짤때는 이유를 고민하고, 그 이유를 코드 구조가 명백히 표현할 수 있도록 해야한다. 그래야 일관성있는 구조를 만들 수 있다.
앞선 코드처럼 prefixIndex를 인수로 전달하는 방식을 이용하면 함수 호출 순서는 명확해지지만 인수가 필요한 이유가 분명히 드러나지 않는다.
그래서, findCommonPrefix와 findCommonSuffix를 원래대로 되돌리고 findCommonSuffix라는 이름을 findCommonPrefixAndSuffix로 바꾸고, findCommonPrefixAndSuffix에서 가장 먼저 findComonPrefix를 호출한다. 이 경우 호출하는 순서가 앞서 고친 코드 보다 훨씬 더 분명해진다.
private compactExpectedAndActual() {
findCommonPrefixAndSuffix();
String compactExpected = compactString(expected);
String compactActual = compactString(actual);
}
private void findCommonPrefixAndSuffix() {
findCommonPrefix();
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;
}
}
suffixIndex = expected.length() - expectedSuffix;
}
private void findCommonPrefix() {
int prefixIndex = 0;
int end = Math.min(expected.length(), actual.length());
for (; prefixIndex < end; prefixIndex++) {
if (expected.charAt(prefixIndex) != actual.charAt(prefixIndex)) {
break;
}
}
}이제 findCommanPrefixAndsuffix 함수를 정리해보자. 정리하는 과정에서 suffixIndex가 실제 코드에서 1부터 시작하기 때문에 Index보다는 길이에 해당된다고 판단하여 suffixLength로 변수 이름을 변경해주었다. (실제로 기존의 suffixIndex는 접미어 길이에 해당한다 Index보다는 Length가 더 적합)
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;
}11. 경계 조건을 캡슐화하라 (392)
경계 조건은 한 곳에서 별도로 처리한다. 다시말해, 코드 여기저기에 +1이나 -1을 흩어놓지 않는다. (392 예제)
이를 만족시키기 위한 코드로 수정해보자.
원본 코드를 보면 computeCommSuffix에 +1 하는 부분을 확인 할 수 있다.(경계 조건 검색) 따라서 해당 부분을 없애기 위해 suffixLength = 1는 1이 아닌 0으로 초기화 해주고 suffixLength + 1 부분을 suffixLength로 수정해준다. 이에 맞게 charFromEnd에 -1을 추가하고 suffixOverlapsPrefix에 < 대신 <=를 사용한다.
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 : ""); // 수정
}
}그런데 여기서 이 부분을 보자
if (suffixLength > 0) {suffixLength를 0으로 초기화 하면서 1씩 감소했기 때문에 > 연산자를 >= 연산자로 변경해주는 것이 타당하다. 이렇게 되면 이 if문은 항상 참이 되기 때문에 무의미한 조건문이 된다!
12. 죽은 코드 (376)
죽은 코드란 실행되지 않는 코드를 말한다. 예를 들어 throw문이 없는 try catch 문이나 방금 본 불필요한 if문 같은 것들이 있다.
이들은 시스템에서 제거해주는것이 좋다. 따라서 if문을 제거하고 구조를 다듬자.
private String compactString(String source) {
return computeCommonPrefix() + DELTA_START
+ source.substring(prefixLength, source.length() - suffixLength)
+ DELTA_END + computeCommonSuffix();
}이렇게 변경하게 되면 compactString 함수는 문자열 조각만 결합하는 1가지 기능을 하는 함수로 변경할 수 있다.
이러한 과정을 거쳐서 만들어진 최종 코드는 아래와 같다.
💻 15-5 ComparsionCompactor.java
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 : "");
}
}이렇게 정리된 모듈은 문자열 분석 함수와, 조합 함수로 나뉜다. 또한 각 함수가 사용된 직후에 정의되도록 정렬하여 분석 함수가 먼저 나오고 조합 함수가 뒤이어 나오는 구조이다.
이렇게 코드가 어느 수준에 이를 때까지 수많은 시행착오를 반복하는 작업을 거쳐 더 좋은 코드를 만들어낼 수 있다.
📚 Reference
- Clean Code : 애자일 소프트웨어 장인 정신
- JUnit : https://ko.wikipedia.org/wiki/JUnit
- 코드 커버리지 : https://woowacourse.github.io/tecoble/post/2020-10-24-code-coverage/
로그를 생활화