8. 리팩토링 - 메서드 추출 기법
원제 : Refactoring to Cleaner Code
시스템의 규모가 커질수록, 중복 코드가 존재할 확률도 증가할 수 있다.
이러한 중복은 유지보수 비용은 물론 변경에 대한 리스크도 늘어나는 단점을 야기한다.
따라서 개발자들은 시스템 내 중복 코드를 최소화하기위해 노력해야 한다.
증가한 규모에 맞춰 덩달한 증가한 코드를 이해하기 위해 들이는 비용은 또 어떤가?
복잡하고 지저분한 코드는 막대한 비용을 소모해야 이해할 수 있기때문에 좋은 구조를 갖추는 것이 중요해진다.
결론적으로 시스템은 낮은 중복성과 높은 명확성을 지향해야하며, 단위 테스트를 통한 리팩토링을 통해 이를 달성하는 방법에 대해 알아보자.
8.1. 작은 리팩토링
원제 : A Little Bit o’ Refactor
리팩토링이란 기존 기능은 그대로 유지하면서 코드의 하부 구조를 변경하는 것이다.
다른 말로, 시스템의 정상 동작은 보장하면서 코드의 위치를 바꾸는 것이라고 이해해도 좋다.
다만 코드의 구조를 마음대로 바꾸는 것은 매우 위험한 일이므로 테스트를 통한 보호 장치를 두는 것이 좋다.
8.1.1. 리팩토링의 기회
원제 : An Opportunity for Refactoring
이전 포스팅에서 다루었던 Profile 클래스의 코드 일부를 다시 살펴보자.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
| class Profile {
fun matches(criteria: Criteria): Boolean {
score = 0
var kill = false
var anyMatches = false
criteria.forEach { criterion ->
var answer = answers.get(criterion.answer.questionText)
var match = criterion.weight == Weight.DontCare || answer?.match(criterion.answer) == true
if (match.not() && criterion.weight == Weight.MustMatch) {
kill = true
}
if (match) {
score += criterion.weight.value
}
anyMatches = anyMatches || match
}
if (kill) {
return false
}
return anyMatches
}
}
|
이 matches()를 대응하기 위해 많은 테스트 케이스를 추가해야 한다.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
| @Test
fun matchAnswersFalseWhenMustMatchCriteriaNotMet() {
}
@Test
fun matchAnswersTrueForAnyDontCareCriteria() {
}
@Test
fun matchAnswersTrueWhenAnyOfMultipleCriteriaMatch() {
}
@Test
fun matchAnswersFalseWhenNoneOfMultipleCriteriaMatch() {
}
@Test
fun scoreIsZeroWhenThereAreNoMatches() {
}
@Test
fun scoreIsCriterionValueForSingleMatch() {
}
@Test
fun scoreAccumulatesCriterionValuesForMatches() {
}
|
이를 개선하기 위해 matches() 메서드의 복잡도를 줄이고 좀 더 명확하게 만들어보도록 하자.
8.1.2. 메서드 추출 기법
원제 : Extract Method: Your Second-Best Refactoring Friend
복잡한 코드에서 조건문은 유난히 이해하기 어려운 경우가 많다.
matches() 메서드의 match 변수를 초기화하는 부분이 그 예시이다.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
| fun matches(criteria: Criteria): Boolean {
criteria.forEach { criterion ->
var answer = answers.get(criterion.answer.questionText)
var match = criterion.weight == Weight.DontCare || answer?.match(criterion.answer) == true
if (match.not() && criterion.weight == Weight.MustMatch) {
kill = true
}
if (match) {
score += criterion.weight.value
}
anyMatches = anyMatches || match
}
}
|
이 초기화 영역을 별도의 메서드 matches()로 추출해서 복잡성을 격리시킨다.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
| fun matches(criteria: Criteria): Boolean {
criteria.forEach { criterion ->
var answer = answers.get(criterion.answer.questionText)
var match = criterion.weight == Weight.DontCare || answer?.match(criterion.answer) == true
if (match.not() && criterion.weight == Weight.MustMatch) {
kill = true
}
if (match) {
score += criterion.weight.value
}
anyMatches = anyMatches || match
}
}
private fun matches(criterion: Criterion, answer: Answer): Boolean {
return criterion.weight == Weight.DontCare || answer.match(criterion.answer) == true
}
|
기존에 작성한 테스트 코드들을 통해 기존의 동작이 보장됨을 확인할 수 있을 겅시다.
8.2. 최적의 메서드 배치
원제 : Finding Better Homes for Our Methods
위의 메서드 추출 기법을 통해 가독성은 확보되었다.
추출된 matches() 메서드를 다시 살펴보면 주어진 조건에 의해 참과 거짓을 판별할 뿐 Profile 클래스와의 연관성이 없음을 알 수 있다.
즉 추춢된 메서드는 Profile 클래스에 있을 필요가 없다는 것이다.
반대로 연관이 있는 클래스를 생각해본다면, Criterion와 Answer가 눈에 보인다.
다만 Criterion 객체는 이미 Answer를 알고 있는 데 반해, Answer는 Criterion에 대한 의존성이 없다.
Answer로 의존하면 의존성이 양방향으로 동작하므로 이는 고려대상이 아니다.
따라서 아래와 같이 Criterion 클래스로 메서드를 이동시키는 것이 합리적이다.
1
2
3
4
5
6
7
8
| class Criterion : Scoreable {
fun matches(answer: Answer): Boolean {
return weight == Weight.DontCare || answer.match(answer) == true
}
}
|
이후 Profile 클래스의 matches()에서 Criterion의 matches()를 참조하도록 변경한다.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
| class Profile {
fun matches(criteria: Criteria): Boolean {
score = 0
var kill = false
var anyMatches = false
criteria.forEach { criterion ->
var answer = answers.get(criterion.answer.questionText)
var match = criterion.matches(answer)
if (match.not() && criterion.weight == Weight.MustMatch) {
kill = true
}
if (match) {
score += criterion.weight.value
}
anyMatches = anyMatches || match
}
if (kill) {
return false
}
return anyMatches
}
}
|
리팩토링을 하고보니 이번엔 answer 변수의 초기화에서 디미터의 법칙을 위반하고 있음을 발견했다.
1
| var answer = answers.get(criterion.answer.questionText)
|
이 부분도 메서드 추출 기법을 사용해 복잡도를 낮추어 보자.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
| class Profile {
fun matches(criteria: Criteria): Boolean {
score = 0
var kill = false
var anyMatches = false
criteria.forEach { criterion ->
var answer = answerMatching(criterion)
var match = criterion.matches(answer)
if (match.not() && criterion.weight == Weight.MustMatch) {
kill = true
}
if (match) {
score += criterion.weight.value
}
anyMatches = anyMatches || match
}
if (kill) {
return false
}
return anyMatches
}
private fun answerMatching(criterion: Criterion): Answer {
return answers.get(criterion.answer.questionText)
}
}
|
이제 전반적으로 가독성이 많이 좋아졌다.
8.3. 과도해보일 수 있는 리팩토링
원제 : Taking Refactoring Too Far?
메서드 추출 기법을 통해 리팩토링을 하다보면 오히려 반복문이 더 늘어나거나 조건문이 추가되는 경우가 발생할 수 가 있다.
흔한 오버엔지니어링일까?
아래 두 가지 포인트를 기준으로 검토해보자.
1. 좀 더 명확한 코드과 결과값의 획득이 가능한가?
리팩토링 후 추출된 메서드의 결과값이 명확하고, 목적성이 잘 확인된다면
오히려 단위 테스트를 작성하기 좋은 형태이기 때문에 좋은 리팩토링이다.
단위 테스트의 메서드는 각각 독립적으로 동작해야한다는 것을 생각해보면, 추출을 통해 단위 테스트 대상이 된 메서드들의 독립적 동작은 테스트 작성을 용이하게 해준다.
2. 유의미한 성능 저하가 발생하는가?
메서드의 호출 횟수가 증가하거나, 순회가 추가된다면 당연히 성능이 저하된다.
하지만 사용자가 느낄 수 없는 정도의 저하라면 가독성 및 명확성을 높이고, 테스트 코드를 통해 안정성을 확보하는 것이 더 좋다.
같이 보기
