👀 Junit 들여다보기1️⃣ 멤버 변수 접두어(f) 제거2️⃣ 조건문 캡슐화3️⃣ 서로 다른 의미의 변수명4️⃣ 부정문보다는 긍정문5️⃣ 행동을 명확히 묘사하는 함수명6️⃣ 일괄적인 함수 사용방식7️⃣ 시간적인 결함8️⃣ 추가 개선9️⃣ 최종 코드🧑🏻⚖️ 결론
👀 Junit 들여다보기
Junit은 저자가 많다. 하지만 시작은 켄트 벡과 에릭 감마 두 사람이 비행기를 타고 가다 3시간 동안 Junit 기초를 구현했다고 한다.
이번에 살펴 볼 모듈은 문자열 비교 오류를 파악할 때 유용한 코드인ComparisonCompactor
모듈을 살펴보면서 코드를 개선해본다.
아래는
ComparisonCompactor
모듈에 대한 테스트 코드다. 커버리지가 100%로 모든 작성자들의 장인 정신을 높이 샀다고 한다.코드참고
아래는
ComparisonCompactor
모듈을 디팩토링한 코드이다.코드 참고
- 매개변수의 String 변수명을 s1, s2 등으로 사용
- suffix를 sfx로 줄여 사용
- expected, actual을 cmp1, cmp2로 사용 등
코드는 보이스카우트 규칙에 따라 더 깨끗한 코드로 만들어야 한다.
- 보이스카우트 규칙 : 체크 아웃때보다 더 좋은 코드를 체크인 한다. 즉, 코드 정리를 거듭할수록 더 좋은 코드가 되어야 한다는 의미
더 좋은 코드를 위해 아래와 같은 규칙을 한번 적용해보자 !
1️⃣ 멤버 변수 접두어(f) 제거
- 오늘날의 개발 환경에서는 중복되는 정보인 멤버 변수의 접두어는 필요 없다.
2️⃣ 조건문 캡슐화
- 조건문을 적절한 메서드로 뽑아낸다
3️⃣ 서로 다른 의미의 변수명
아래 코드 반환 값은 compact된 문자열이고 this가 붙은 변수들은 입력값들이다 둘의 의미가 다르다.
4️⃣ 부정문보다는 긍정문
부정문보다는 긍정문이 인지적으로 이해하기 쉽다. 조건문을 긍정문으로 바꾸고, 로직의 위치를 반전시켜 준다
5️⃣ 행동을 명확히 묘사하는 함수명
compact 함수는 ?
- 오류 점검 부가 단계 숨겨짐 : canBeCompacted가 false면 compact하지 않는다.
- 형식이 갖춰진 문자열을 반환 : format
따라서
formatCompactedComparison
라는 이름이 더 적합하다.또한 if문 안의 예상 문자열과 실제 문자열을 압축하는 코드는
compactExpectedAndActual
라는 별도의 함수로 빼낸다.이 함수에서 압축만하고 포맷팅을 하는 일은 전적으로 formatCompactedComparison 함수에게 맡긴다.
또한 compactExpected와 ccompactActual은 멤버 변수로 승격했다.
6️⃣ 일괄적인 함수 사용방식
새 함수인 compactExpectedAndActual의 위 두 줄은 반환값이 없이 바로 할당하고, compactString은 반환값을 토대로 할당한다. 함수 사용 방식에 대한 일관성을 맞춰준다. (반환형 변경)
멤버 변수 이름도 색인 위치를 나타내기 때문에 Index라는 단어를 붙여주도록 변경함
7️⃣ 시간적인 결함
findCommonSuffix 함수는 시간적인 결합이 존재한다.
findCommonSuffix는 findCommonPrefix가 계산한 prefixIndex를 토대로 동작한다. 그러므로 findCommonPrefix가 먼저 실행되고 난 후 findCommonSufix가 실행되어야 한다.
두가지 해결방법이 있다.
1. 시간 결합을 외부에 노출하기 위해 findCommonSuffix의 매개변수에 prefixIndex를 포함
썩 좋은 방법은 아니다. 함수의 호출 순서가 확실히 정해지지만 prefixindex가 필요한 이유를 드러내지 않는다. 다른 프로그래머가 원래대로 돌려놓을지도 모른다.
- findCommonPrefixAndSuffix()
다시 아까 반환형을 int로 바꿨던것을 void로 되돌려 놓고 findCommonPrefixAndSuffix라는 함수를 만든다.
호출하는 순서가 앞서 코친 코드보다 훨씬 분명해진다.
또한, PrefixAndSuffix 함수가 얼마나 지저분한지도 드러난다. 개선 해준다.
8️⃣ 추가 개선
고치고 난 후에는 suffixIndex가 실제로는 접미어 길이라는 사실이 드러나므로 Index가아닌 length라는 단어를 사용한다.
그리고 suffixIndex는 0에서 시작하지 않고 1에서 시작하므로 진정한 길이가 아니다. (computeCommonSuffix에 +1이 곳곳에 등장하는 이유)
suffixIndex = 1 을 suffixLength = 0으로 변경하고 그 외 로직들을 다시 올바르게 재조정한다.
- computeCommonSuffix 에서 +1 제거
- charmFromEnd에 -1 추가
- suffixOverlaps에 <= 사용
- if (suffixLength > 0) 은 if (suffixLength >= 0) 으로 바뀌어야한다
- 하지만 suffixLength가 0인 것은 말이 안된다.
- 심지어 suffixLength가 0인 상황은 없다. -> if문을 제거할 수 있다.
- if (prefixLength > 0)도 마찬가지다.
9️⃣ 최종 코드
코드참고
모듈은 일련의 분석 함수와 일련의 조합 함수로 나뉜다.
전체 함수는 위상적으로 정렬하여 각 함수가 사용된 직후에 정의된다.
분석 함수가 먼저 나오고 조합 함수가 뒤에 이어 나온다.
앞에서 개선했던 몇가지 수정 사항들을 번복하기도 했다. 이런 경우는 흔하다(시행착오는 당연한 것)
🧑🏻⚖️ 결론
보이스카우트 규칙을 잘 지킬 수 있었다.
원래 깨끗하지 못했던 코드라는 뜻은 아니고 충분히 우수한 모듈이었지만 세성에 개선이 불필요한 모듈은 없다.
코드를 처음보다 조금 더 깨끗하게 만드는 책임은 우리에게 있다.