I Need to Change a Monster Method and I Can’t Write Tests for It
레거시 코드로 작업할 때 가장 힘든 일 중 하나는 규모가 큰 메서드를 다루는 일이다.
대규모 메서드를 다루기 힘든 이유는 너무 크고 방대하기에 변경을 시도할 때마다 매번 기억을 더듬고, 내용을 이해해야하기 때문이다.
이 대규모 메서드가 단순히 다루기 힘든 녀석이라면 레거시 메서드(Monster Method) 코드가 너무 길고, 내용이 복잡해서 손대기가 힘든 메서드를 의미한다.
발아 메서드 기법(Sprout Method) 이나 발아 클래스 기법(Sprout Class) 을 사용함으로써 리팩토링을 회피할 수는 있지만 근본적인 해결책은 되지 못한다.
레거시 메서드를 다루려면 어디서부터 손을 대야할까?
참고 원서는 Monster Method라고 부르고, 번역서는 괴물 메서드라고 하는데 둘 다 어감이 좋지 않아 본 포스팅 한정으로 레거시 메서드 로 치환하여 다루겠다.
1. 레거시 메서드의 종류(Varieties of Monsters)
레거시 메서드를 구별하기위한 명확한 기준이 있는 것은 아니고, 구별을 하더라도 실무에서 만나는 레거시 메서드는 몇 가지 사례가 뒤섞여있는 경우가 대부분이다.
불릿 메서드(Bulleted Methods)
불릿 메서드는 들여쓰기가 거의 되어있지 않은 메서드를 말한다.
메서드 전체를 기준으로 보았을 때, 일부는 들여쓰기가 되어있을 수는 있지만 전반적으로 들여쓰기가 되어있지않다.
일반적으로 아래와 같은 형태이다.
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
35
36
37
38
39
40
41
42
| void Reservation::extend(int additionalDays) {
int status = RIXInterface::checkAvailable(type, location, startingDate);
int identCookie = -1;
switch(status) {
case NOT_AVAILABLE_UPGRADE_LUXURY:
identCookie = RIXInterface::holdReservation(Luxury,location,startingDate, additionalDays +dditionalDays);
break;
case NOT_AVAILABLE_UPGRADE_SUV: {
int theDays = additionalDays + additionalDays;
if (RIXInterface::getOpCode(customerID) != 0)
theDays++;
identCookie = RIXInterface::holdReservation(SUV,location,startingDate, theDays);
}
break;
case NOT_AVAILABLE_UPGRADE_VAN:
identCookie = RIXInterface::holdReservation(Van, location,startingDate, additionalDays + additionalDays);
break;
case AVAILABLE:
default:
RIXInterface::holdReservation(type,location,startingDate);
break;
}
if (identCookie != -1 && state == Initial) {
RIXInterface::waitlistReservation(type,location,startingDate);
}
Customer c = res_db.getCustomer(customerID);
if (c.vipProgramStatus == VIP_DIAMOND) {
upgradeQuery = true;
}
if (!upgradeQuery)
RIXInterface::extend(lastCookie, days + additionalDays);
else {
RIXInterface::waitlistReservation(type,location,startingDate);
RIXInterface::extend(lastCookie, days + additionalDays +1);
}
}
|
혼잡 메서드(Snarled Methods)
혼잡 메서드란 들여쓰기는 되어있지만, 대규모 단락으로 구성된 메서드를 말한다.
아래는 대규모 조건문을 가진 혼잡 메서드의 예시이다.
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
| Reservation::Reservation(VehicleType type, int customerID, long startingDate, int days, XLocation l)
: type(type), customerID(customerID), startingDate(startingDate), days(days), lastCookie(-1), state(Initial), tempTotal(0) {
location = l;
upgradeQuery = false;
if (!RIXInterface::available()) {
RIXInterface::doEvents(100);
PostLogMessage(0, 0, "delay on reservation creation");
int holdCookie = -1;
switch(status) {
case NOT_AVAILABLE_UPGRADE_LUXURY:
holdCookie = RIXInterface::holdReservation(Luxury,l,startingDate);
if (holdCookie != -1) {
holdCookie |= 9;
}
break;
case NOT_AVAILABLE_UPGRADE_SUV:
holdCookie = RIXInterface::holdReservation(SUV,l,startingDate);
break;
case NOT_AVAILABLE_UPGRADE_VAN:
holdCookie = RIXInterface::holdReservation(Van,l,startingDate);
break;
case AVAILABLE:
default:
RIXInterface::holdReservation;
state = Held;
break;
}
}
}
|
얼핏 보면 불릿 메서드와 같다고 느낄 수도 있는데, 진정한 혼잡 메서드 여부를 판정하려면 코드를 아래와 같이 정렬해보면 된다.
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
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
| Reservation::Reservation(VehicleType type, int customerID, long startingDate, int days, XLocation l)
: type(type), customerID(customerID), startingDate(startingDate), days(days), lastCookie(-1), state(Initial), tempTotal(0) {
location = l;
upgradeQuery = false;
while(!RIXInterface::available()) {
RIXInterface::doEvents(100);
PostLogMessage(0, 0, "delay on reservation creation");
int holdCookie = -1;
switch(status) {
case NOT_AVAILABLE_UPGRADE_LUXURY:
holdCookie = RIXInterface::holdReservation(Luxury,l,startingDate);
if (holdCookie != -1) {
if (l == GIG && customerID == 45) {
while (RIXInterface::notBusy()) {
int code = RIXInterface::getOpCode(customerID);
if (code == 1 || customerID > 0)) {
PostLogMessage(1, 0, "QEX PID");
for (int n = 0; n < 12; n++) {
int total = 2000;
if (state == Initial || state == Held) {
total += getTotalByLocation(location);
tempTotal = total;
if (location == GIG && days > 2) {
if (state == Held)
total += 30;
}
}
RIXInterface::serveIDCode(n, total);
}
} else {
RIXInterface::serveCode(customerID);
}
}
}
}
break;
case NOT_AVAILABLE_UPGRADE_SUV:
holdCookie = RIXInterface::holdReservation(SUV,l,startingDate);
break;
case NOT_AVAILABLE_UPGRADE_VAN:
holdCookie = RIXInterface::holdReservation(Van,l,startingDate);
break;
case AVAILABLE:
default:
RIXInterface::holdReservation(type,l,startingDate); state = Held;
break;
}
}
}
|
대부분의 메서드는 불릿 메서드나 혼잡 메서드가 아닌 그 중간쯤의 형태를 보이며, 내부적으로 블록이 깊게 중첩되어있기에 테스트 루틴 작성이 어려워진다.
2. 리팩토링 자동화 도구를 사용해 레거시 메서드 공략하기(Tackling Monsters with Automated Refactoring Support)
메서드 추출 도구를 이용하면 레거시 메서드를 좀 더 효과적으로 리팩토링할 수 있다.
메서드 추출을 수행하는 주요 목적은 아래와 두 가지이다.
- 이상한 의존 관계로부터 로직을 분리한다.
- 이후의 리팩토링을 위해 테스트 루틴을 작성하기 쉽도록 봉합부를 작성한다.
예제를 통해 살펴보자.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
| class CommoditySelectionPanel {
public void update() {
if (commodities.size() > 0 && commodities.GetSource().equals("local")) {
listbox.clear();
for (Iterator it = commodities.iterator(); it.hasNext(); ) {
Commodity current = (Commodity)it.next();
if (commodity.isTwilight() && !commodity.match(broker))
listbox.add(commodity.getView());
}
}
}
}
|
어떤 점들을 개선할 수 있을까?
CommoditySelectionPanel 클래스는 Panel 클래스이므로 화면의 렌더링만 책임지는 것이 좋다.
헌데, update 메서드에서 데이터들을 필터링하고 있는 점을 볼 수 있다.
리팩토링 도구를 사용해서 메서드를 추출하면 함수의 네이밍을 지정해주면서 분리할 수 있다.
분리 후 결과는 아래와 같다.
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
35
36
| class CommoditySelectionPanel {
public void update() {
if (commoditiesAreReadyForUpdate()) {
clearDisplay();
updateCommodities();
}
}
private boolean commoditiesAreReadyForUpdate() {
return commodities.size() > 0 && commodities.GetSource().equals("local");
}
private void clearDisplay() {
listbox.clear();
}
private void updateCommodities() {
for (Iterator it = commodities.iterator(); it.hasNext(); ) {
Commodity current = (Commodity)it.next();
if (singleBrokerCommodity(commodity)) {
displayCommodity(current.getView());
}
}
}
private boolean singleBrokerCommodity(Commodity commodity) {
return commodity.isTwilight() && !commodity.match(broker);
}
private void displayCommodity(CommodityView view) {
listbox.add(view);
}
}
|
update 내부의 코드는 그다지 개선된 것이 없어보이지만 데이터 갱신의 책임을 별도의 메서드에 위임했다는 것을 확인할 수 있다.
함수의 네이밍이 부적절해보일 수 있지만, 최소한 해당 함수가 어떤 책임을 지는 지 알 수 있으며 이를 상위 수준에서 보고 봉합 지점을 파악할 수도 있다.
만약 추가로 리팩토링을 한다면 update와 updateCommodities 메서드를 별도의 클래스로 분리할 수 있을 것이다.
리팩토링을 마치면 아래의 UML 처럼 작성될 것이다.
3. 수동 리팩토링(The Manual Refactoring Challenge)
리팩토링 도구가 없는 경우 별수 없이 직접 정확성을 검증해나가야 하며, 이때 리팩토링 도구대신 테스트 루틴을 사용할 수 있다.
레거시 메서드는 상술했듯 테스트 루틴의 작성이나 리팩토링, 기능의 추가가 매우 어려운 메서드이다.
만약 이 레거시 메서드를 포함하는 클래스의 객체를 테스트 하네스 내에서 생성할 수 있다면, 검증을 반복하며 메서드를 분리할 수 있을 것이다.
수동으로 메서드를 분리하는 경우 흔히 실수하는 케이스는 아래와 같다.
- 추출한 메서드에 변수를 전달하는 것을 누락한다.
- 추출된 메서드에 붙인 이름이 기초 클래스 내에 존재하는 동일한 이름의 메서드를 은폐하거나, 재정의한다.
- 파라미터를 전달하거나, 반환값을 대입할 때 잘못된 값을 전달 혹은 대입한다.
감지 변수 도입(Introduce Sensing Variable)
리팩토링을 하는 경우 클래스에 변수를 추가하여 감지를 할 수 있게끔 처리할 수 있다.
리팩토링 이후 추가된 변수를 제거한다면 이전의 상태로 돌아갈 것이다.
이를 가리쳐 감지 변수 도입이라고 부른다.
아래의 예제를 보자.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
| public class DOMBuilder {
void processNode(XDOMNSnippet root, List childNodes) {
if (root != null) {
if (childNodes != null)
root.addNode(new XDOMNSnippet(childNodes));
root.addChild(XDOMNSnippet.NullSnippet);
}
List paraList = new ArrayList();
XDOMNSnippet snippet = new XDOMNReSnippet();
snippet.setSource(m_state);
for (Iterator it = childNodes.iterator(); it.hasNext();) {
XDOMNNode node = (XDOMNNode)it.next();
if (node.type() == TF_G || node.type() == TF_H ||
(node.type() == TF_GLOT && node.isChild())) {
paraList.addNode(node);
}
}
}
}
|
processNode 메서드의 많은 처리가 대부분 XDOMNSnippet 객체에 대해서 실행되고 있음을 확인할 수 있다.
이를 감지하기 위해 특정 타입의 노드가 paraList에 추가되었음을 확인하기 위해 감지 변수를 추가해보자.
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
| public class DOMBuilder {
public boolean nodeAdded = false;
void processNode(XDOMNSnippet root, List childNodes) {
if (root != null) {
if (childNodes != null)
root.addNode(new XDOMNSnippet(childNodes));
root.addChild(XDOMNSnippet.NullSnippet);
}
List paraList = new ArrayList();
XDOMNSnippet snippet = new XDOMNReSnippet();
snippet.setSource(m_state);
for (Iterator it = childNodes.iterator(); it.hasNext();) {
XDOMNNode node = (XDOMNNode)it.next();
if (node.type() == TF_G || node.type() == TF_H ||
(node.type() == TF_GLOT && node.isChild())) {
paraList.addNode(node);
nodeAdded = true;
}
}
}
}
|
nodeAdded가 존재하더라도 특정 노드 타입을 특정해야하므로 로직을 더 추가해야한다.
node.type()이 TF_G인 경우에 대해서 확인할 수 있도록 메서드를 추가해보자.
1
2
3
4
5
6
7
8
| void testAddNodeOnBasicChild() {
DOMBuilder builder = new DomBuilder();
List children = new ArrayList();
children.add(new XDOMNNode(XDOMNNode.TF_G));
Builder.processNode(new XDOMNSnippet(), children);
assertTrue(builder.nodeAdded);
}
|
이번엔 잘못된 노드 타입인 경우, 노드가 추가되지않았음을 확인할 수 있도록 메서드를 추가해보자.
1
2
3
4
5
6
7
8
| void testNoAddNodeOnNonBasicChild() {
DOMBuilder builder = new DomBuilder();
List children = new ArrayList();
children.add(new XDOMNNode(XDOMNNode.TF_A));
Builder.processNode(new XDOMNSnippet(), children);
assertTrue(!builder.nodeAdded);
}
|
위와 같은 테스트 루틴들을 작성하고나면, 노드 추가 여부를 판단하는 조건문을 안심하고 추가할 수 있다.
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
| public class DOMBuilder {
public boolean nodeAdded = false;
void processNode(XDOMNSnippet root, List childNodes) {
if (root != null) {
if (childNodes != null)
root.addNode(new XDOMNSnippet(childNodes));
root.addChild(XDOMNSnippet.NullSnippet);
}
List paraList = new ArrayList();
XDOMNSnippet snippet = new XDOMNReSnippet();
snippet.setSource(m_state);
for (Iterator it = childNodes.iterator(); it.hasNext();) {
XDOMNNode node = (XDOMNNode)it.next();
if (isBasicChild(node)) {
paraList.addNode(node);
nodeAdded = true;
}
}
}
private boolean isBasicChild(XDOMNode node) {
return node.type() == TF_G
|| node.type() == TF_H
|| (node.type() == TF_GLOT && node.isChild());
}
}
|
물론 추가한 감지 변수는 이후에 손쉽게 제거할 수 있다.
알고 있는 부분 추출하기(Extract What You Know)
레거시 메서드 내에서 명확하게 알고 있는 작은 로직을 찾았다면, 이를 분리하기 위한 테스트 코드를 작성하여 분리하는 것도 좋은 방법이다.
아래 간단한 예제를 보자.
1
2
3
4
5
6
7
8
| void process(int a, int b, int c) {
int maximum;
if (a > b)
maximum = a;
else
maximum = b;
}
|
추출하면 아래와 같이 바꿀 수 있다.
1
2
3
4
| void process(int a, int b, int c) {
int maximum = max(a, b);
}
|
이처럼 명백하게 알고 있는 로직은 분리하여 처리할 수 있다.
여기서 적용한 max 함수는 두 개의 인풋과 한 개의 아웃풋을 가지고 있어 결합 계수(copling count) 는 3인데, 실수를 줄이기 위해선 이러한 결합 계수가 낮은 코드를 추출하는 것이 바람직하다.
특히 결합 계수가 0인 로직을 분리한다면 레거시 메서드를 축소하는 데 많은 도움이 될 것이다.
의존 관계 수집 기법(Gleaning Dependencies)
레거시 메서드엔 메서드가 가진 책임과 거리가 먼 코드가 포함되어있을 수 있다.
로직상 필요하더라도 메서드의 주요 로직을 붕괴시킬 여지가 남아있는 이상 분리대상이며, 이때 의존 관계 수집 기법(Gleaning Dependencies) 을 사용할 수 있다.
의존 관계 수집 기법은 먼저 변경이 되어선 안되는 부분에 대해 테스트 루틴을 작성한 뒤,
테스트 대상이 아닌 부분을 추출하여, 주요 동작을 보전시키는 기법이다.
아래 예제를 보자.
1
2
3
4
5
6
7
8
9
10
11
12
| void addEntry(Entry entry) {
if (view != null && DISPLAY == true) {
view.show(entry);
}
if (entry.category().equals("single") || entry.category("dual")) {
entries.add(entry);
view.showUpdate(entry, view.GREEN);
} else {
}
}
|
위 코드는 항목을 추가하는 로직에 오류가 있다면 알아채기 힘든 부분이 있다.
이 경우 올바른 조건에서 항목이 추가되는 지를 검증하는 테스트 루틴을 작성하고, 이후 렌더링 관련 코드를 추출하여 주요 동작을 보전 시킨다.
의존 관계 수집 기법은 일종의 책임 회피처럼 느껴질 수도 있지만, 다른 더 중요한 동작을 보호할 수 있다는 것이 맹점이다.
메서드 객체 추출 기법(Break Out a Method Object)
때로는 별도로 추가하지않더라도 감지 변수 역할로 쓸만한 변수가 이미 추가되어있는 경우가 있다.
단, 인스턴스 변수라면 메서드의 실행 후에 값을 검증할 수 있으므로 어떤 상황에 어떠한 값을 가지고 있을지 파악하는 것이 어렵다는 단점이 있다.
이때 메서드 객체 추출 기법(Break Out a Method Object) 을 사용할 수 있다.
레거시 메서드에서 메서드 객체를 추출한 경우, 이 객체의 유일한 책임은 레거시 메서드의 처리를 수행하는 것이므로 리팩토링이 매우 용이해진다.
4. 전략(Strategy)
이번엔 코드의 구조에서 나타나는 장점과 단점에 대해서 알아보자.
골격 메서드(Skeletonize Methods)
조건문을 포함하는 코드 내에서 추출할 부분을 찾는 경우, 두 가지 방법 중 하나를 선택할 수 있다.
- 조건 부분과 처리 부분을 별도로 추출한다.
- 조건 부분과 처리 부분을 한 번에 추출한다.
먼저 별도로 추출하는 경우부터 살펴보자.
1
2
3
4
| if (marginalRate() > 2 && order.hasLimit()) {
order.readjust(rateCalculator.rateForToday());
order.recalculate();
}
|
조건부와 처리부를 별도로 추출하면 아래와 같다.
1
2
3
| if (orderNeedsRecalculation(order)) {
recalculateOrder(order, rateCalculator);
}
|
이러한 구조를 골격 메서드라고 부른다.
참고 번역서는 뼈대 메서드라고 번역하였다.
메서드에 남은 것은 제어 구조 및 다른 메서드로의 위임뿐이다
처리 시퀀스 탐지(Find Sequences)
이번엔 조건부와 처리부를 한 번에 추출해보자.
1
2
3
4
| if (marginalRate() > 2 && order.hasLimit()) {
order.readjust(rateCalculator.rateForToday());
order.recalculate();
}
|
위 코드를 아래와 같이 아예 분리해버린다.
1
2
3
4
5
6
7
8
9
10
|
recalculateOrder(order, rateCalculator);
void recalculateOrder(Order order, RateCalculator rateCalculator) {
if (marginalRate() > 2 && order.hasLimit()) {
order.readjust(rateCalculator.rateForToday());
order.recalculate();
}
}
|
레거시 메서드에 남은 부분은 그저 처리를 위한 호출부 뿐이다.
현재 클래스 내에서의 우선 추출(Extract to the Current Class First)
레거시 메서드에서 메서드를 추출할때, 추출 대상으로 검토중인 코드가 실제로는 다른 클래스의 것임을 알게될 때가 있다.
이 경우 해당 코드는 아예 그 코드가 속한 클래스로 옮기는 것이 나을 것이다.
예제를 보자.
1
2
3
4
| if (marginalRate() > 2 && order.hasLimit()) {
order.readjust(rateCalculator.rateForToday());
order.recalculate();
}
|
이 코드 스니펫에 recalculateOrder라는 이름을 붙여, Order 클래스에 포함시켜버리는 것이다.
작은 코드 조각 추출(Extract Small Pieces)
결합 계수에서도 언급했지만, 아주 작은 코드 조각부터 추출하는 것이 좋다.
처음에는 레거시 메서드에 미치는 영향이 매우 미미하겠지만, 꾸준히 해나간다면 처리 시퀀스를 발견하던가, 좀 더 적절한 방법이 떠오를 것이다.
처음부터 대규모 코드를 분ㄹ히ㅏ려는 시도는 생각보다 어려우며 무엇보다 안전하다는 보장이 없다.
다시 추출할 각오(Be Prepared to Redo Extractions)
레거시 메서드를 분할하는 방법은 매우 다양하다.
몇 번 추출작업을 하다보면, 새로운 기능을 더 쉽게 적용할 수 있는 방법이 발견되곤 하는데,
이때 과감하게 기존 추출작업을 롤백하고 다시 추출하는 것이 최선일 수도 있다.
다소 허무할 수도 잇지만, 기존 설계와 구조에 대한 통찰력과 앞으로 개선해나갈 방향성을 얻을 수 있음에 의의를 두자.
