—
Ep.26
몇백 줄, 혹은 그 이상의 라인이 존재하는 메서드나 클래스를 다루며 어려움을 겪은 경험이 있으신가요? 이 글을 읽는 대부분의 개발자들이 그렇다고 답하리라 생각합니다. 저 또한 복잡한 코드를 만나 어려움을 겪은 적이 종종 있습니다.
널리 알려져 있듯 이러한 문제를 해결하는 데 훌륭한 도구로 리팩터링과 테스트 코드가 있으며, 제 경험 또한 그러했습니다. 그래서 리팩터링과 테스트 코드의 중요성이라는 주제로 글을 작성하게 됐습니다. 다만 일반적으로 알려진 중요성을 나열하기보다는 주니어 백엔드 개발자로서 왜 리팩터링과 테스트 코드가 중요하다고 느끼게 되었는지 그 경험을 담아내려 합니다.
(*리팩터링 : 소프트웨어의 겉보기 동작은 그대로 유지한 채, 코드를 이해하고 수정하기 쉽도록 내부 구조를 변경하는 기법)
아래 보여드릴 코드는 GitHub에서 4k+ star를 받은 리팩터링 연습용 Code Kata입니다. 먼저, 이 프로젝트의 요구사항, 배경을 간단히 살펴보겠습니다.
여러 아이템이 존재하며 아이템은 남은 판매 일수(sellIn)와 가치(quality)를 가집니다.
기본적으로 아이템은 하루가 지나면 판매 일수, 가치가 감소합니다.
일부 아이템은 각자의 방식으로 가치가 감소합니다.
새로운 아이템을 추가해야 하며, 이 또한 고유의 방식으로 가치를 감소합니다.
아래 코드에서 볼 수 있듯 중첩된 조건문과 중복 코드로 새로운 기능 추가가 쉽지 않습니다. 이러한 상황 속에서 이 코드를 크게 두 단계에 걸쳐 리팩터링하며 왜 리팩터링과 테스트 코드가 중요한지 알아보겠습니다.
예제 1) Gilded Rose Refactoring Kata
class GildedRose {
Item[] items;
public GildedRose(Item[] items) {
this.items = items;
}
public void updateQuality() {
for (int i = 0; i < items.length; i++) {
if (!items[i].name.equals("Aged Brie")
&& !items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (items[i].quality > 0) {
if (!items[i].name.equals("Sulfuras, Hand of Ragnaros")) {
items[i].quality = items[i].quality - 1;
}
}
} else {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
if (items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (items[i].sellIn < 11) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
if (items[i].sellIn < 6) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
}
}
}
if (!items[i].name.equals("Sulfuras, Hand of Ragnaros")) {
items[i].sellIn = items[i].sellIn - 1;
}
if (items[i].sellIn < 0) {
if (!items[i].name.equals("Aged Brie")) {
if (!items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (items[i].quality > 0) {
if (!items[i].name.equals("Sulfuras, Hand of Ragnaros")) {
items[i].quality = items[i].quality - 1;
}
}
} else {
items[i].quality = items[i].quality - items[i].quality;
}
} else {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
}
}
}
}
(*해당 프로젝트의 배경, 요구사항은 링크를 통해 확인할 수 있습니다. 리팩터링을 크게 두 단계로 나눈 것은 글의 설명을 돕기 위한 목적이며, 실제 리팩터링은 작은 단위로 나누어 수행했습니다.)
(*Code Kata : 무술의 ‘카타’에서 유래한 용어로, 짧고 반복적인 코딩 연습을 통해 프로그래밍 실력을 수련하는 과제나 연습 문제)
아래는 첫 번째 리팩터링 결과입니다. 이번 단계의 리팩터링은 중첩 조건문 제거, 매직 넘버 상수화, 메서드 추출과 같은 리팩터링 기법을 활용해 기존 흐름을 유지한 채 코드의 가독성을 향상하는 데 집중했습니다. 먼저, 반복되는 조건식 및 quality 증가, 감소 로직 등을 메서드로 추출해 어떤 행동을 하는지 명시했습니다. 또 중첩된 조건문을 메서드로 추출한 다음 조건문을 합치거나 보호 구문 형태로 변경했습니다. 이번 단계에서 중요한 점은 간단한 리팩터링 기법(대부분 IDE에서 제공하는 기능)만으로도 코드의 가독성을 크게 향상할 수 있다는 사실입니다.
예제 2) 첫 번째 리팩터링 결과
class GildedRose {
public static final int MINIMUM = 0;
public static final int MAXIMUM = 50;
. . .
public void updateQuality() {
for (Item item : items) {
updateQualityBySellIn(item);
}
}
public void updateQualityBySellIn(Item item) {
updateQualityByItemType(item);
decreaseSellInExceptSulfuras(item);
updateQualityWhenNotRemainSellIn(item);
}
private void updateQualityByItemType(Item item) {
if (isNormalItem(item.name)) {
decreaseQuality(item);
} else {
increaseQuality(item);
}
}
private boolean isNormalItem(String name) {
return !isAgedBrie(name) && !isBackstagePasses(name) && !isSulfuras(name);
}
private void decreaseSellInExceptSulfuras(Item item) {
if (isSulfuras(item.name)) {
return;
}
item.sellIn = item.sellIn - 1;
}
private void updateQualityWhenNotRemainSellIn(Item item) {
if (hasRemainingSellIn(item.sellIn) || isSulfuras(item.name)) {
return;
}
if (isAgedBrie(item.name)) {
increaseQualityBy(1, item);
} else if (isBackstagePasses(item.name)) {
item.quality = MINIMUM;
} else if (isDecreasable(item.quality)) {
decreaseQuality(item);
}
}
private boolean hasRemainingSellIn(int sellIn) {
return sellIn >= MINIMUM;
}
private void increaseQuality(Item item) {
if (isBackstagePasses(item.name)) {
increaseBackstagePassesQuality(item);
return;
}
increaseQualityBy(1, item);
}
private void increaseBackstagePassesQuality(Item item) {
if (item.sellIn < 6) {
increaseQualityBy(3, item);
} else if (item.sellIn < 11) {
increaseQualityBy(2, item);
} else {
increaseQualityBy(1, item);
}
}
private void increaseQualityBy(int qualityToAdd, Item item) {
if (isIncreasable(item.quality, qualityToAdd)) {
item.quality = item.quality + qualityToAdd;
}
}
private boolean isIncreasable(int quality, int qualityToAdd) {
return quality + qualityToAdd <= MAXIMUM;
}
private void decreaseQuality(Item item) {
if (isDecreasable(item.quality)) {
item.quality = item.quality - 1;
}
}
private boolean isDecreasable(int quality) {
return quality > MINIMUM;
}
private boolean isSulfuras(String name) {
return name.equals("Sulfuras, Hand of Ragnaros");
}
private boolean isBackstagePasses(String name) {
return name.equals("Backstage passes to a TAFKAL80ETC concert");
}
private boolean isAgedBrie(String name) {
return name.equals("Aged Brie");
}
}
가독성 향상이란 장점만으로도 첫 번째 리팩터링을 해야 할 이유는 충분한 듯합니다. 하지만 첫 번째 단계의 리팩터링을 해야만 하는 또 다른 이유가 존재합니다. 바로 유연한 구조를 만들기 위한 단서가 되기 때문입니다.
첫 번째 리팩터링을 통해 updateQuality()가 하는 일을 크게 세 가지로 구분할 수 있었습니다. 먼저 아이템 종류에 따라 quality(가치)값을 수정하고, sellIn(판매기간)을 감소합니다. 이후 다시 sellIn이 남아 있지 않은 아이템의 quality 값을 수정합니다.
이렇게 보니 몇 군데 개선할 여지가 보입니다. 먼저 quality 값을 수정하는 작업을 굳이 두 번에 나눠서 할 필요가 있을까 싶습니다. 또 아이템 종류에 따라 여러 조건문이 반복되는데 이를 추상화해 보면 어떨까 싶습니다. 마지막으로 현재 코드에서 요구 사항인 Conjured 아이템에 대한 새로운 기능을 추가해 보겠습니다.
다음 코드는 앞서 리팩터링한 코드에서 기능을 추가하며 변경된 부분만 작성한 코드입니다. 코드에서 볼 수 있듯 새로운 기능을 추가하는 것은 어렵지 않지만 기존 코드 베이스의 변경이 불가피한 문제가 존재합니다.
class GildedRose {
. . .
private void updateQualityByItemType(Item item) {
if (isNormalItem(item.name) || isConjured(item.name)) {
decreaseQuality(item);
} else {
increaseQuality(item);
}
}
. . .
private void decreaseQuality(Item item) {
if (!isDecreasable(item.quality)) {
return;
}
if (isConjured(item.name)) {
decreaseQualityBy(2, item);
return;
}
decreaseQualityBy(1, item);
}
private void decreaseQualityBy(int qualityToSubtract, Item item) {
item.quality = item.quality - qualityToSubtract;
}
. . .
private boolean isConjured(String name) {
return name.equals("Conjured Mana Cake");
}
}
첫 번째 리팩터링을 통해 더 유연한 구조로 발전할 수 있는 실마리를 찾을 수 있었습니다. 물론 원본 코드에서도 이와 같은 단서를 찾을 수도 있었겠지만, 구체적인 리팩터링 방향을 찾기는 쉽지 않았을 것입니다. 그러므로 평소에 코드를 작성할 때, 이번 단계의 리팩터링 수준까지 진행하는 것이 바람직합니다.
상황에 따라 이쯤 마무리하는 것도 좋은 방법이지만 여기서는 남은 리팩터링의 중요성을 설명하기 위해 두 번째 단계의 리팩터링을 진행하겠습니다.
<두 번째 리팩터링>
첫 번째 리팩터링을 통해 코드의 문맥과 구조를 명확히 할 수 있었습니다. 이번 단계는 반복되는 조건문 로직을 디자인 패턴 중 하나인 전략 패턴을 활용해 역할과 책임을 분리하는 데 집중했습니다. 이를 통해 얻고자 함은 추가될 기능에 따른 기존 코드 베이스 변경의 최소화입니다. 즉, 유지보수성 향상이 핵심입니다.
예제 3) 두 번째 단계 리팩터링 결과
class GildedRose {
. . .
public void updateQuality() {
for (Item item : items) {
QualityStrategyFactory.createByName(item.name)
.updateQualityByStrategy(item);
}
}
}
public class QualityStrategyFactory {
public static QualityStrategy createByName(String name) {
switch (name) {
case "Backstage passes to a TAFKAL80ETC concert" :
return new BackstagePassesQualityStrategy();
case "Sulfuras, Hand of Ragnaros" :
return new SulfurasQualityStrategy();
case "Aged Brie" :
return new AgedBrieQualityStrategy();
default:
return new DefaultQualityStrategy();
}
}
}
public abstract class QualityStrategy {
public static final int MINIMUM = 0;
public static final int MAXIMUM = 50;
protected abstract void updateQualityByItem(Item item);
protected abstract void decreaseSellIn(Item item);
public void updateQualityByStrategy(Item item) {
updateQualityByItemType(item);
decreaseSellIn(item);
}
protected void increaseQualityBy(int qualityToAdd, Item item) {
if (isIncreasable(item.quality, qualityToAdd)) {
item.quality = item.quality + qualityToAdd;
} else {
item.quality = MAXIMUM;
}
}
protected boolean isIncreasable(int quality, int qualityToAdd) {
return quality + qualityToAdd <= MAXIMUM;
}
protected void decreaseQualityBy(int qualityToSubtract, Item item) {
if (isDecreasable(item.quality, qualityToSubtract)) {
item.quality = item.quality - qualityToSubtract;
} else {
item.quality = MINIMUM;
}
}
protected boolean isDecreasable(int quality, int qualityToSubtract) {
return quality - qualityToSubtract >= MINIMUM;
}
protected boolean isSellInOver(Item item) {
return item.sellIn <= MINIMUM;
}
}
public class BackstagePassesQualityStrategy extends QualityStrategy {
@Override
protected void updateQualityByItemType(Item item) {
if (isSellInOver(item)) {
item.quality = MINIMUM;
} else if (isSellInLessThanSixDays(item)) {
increaseQualityBy(3, item);
} else if (isSellInLessThanElevenDays(item)) {
increaseQualityBy(2, item);
} else {
increaseQualityBy(1, item);
}
}
private boolean isSellInLessThanSixDays(Item item) {
return item.sellIn < 6;
}
private boolean isSellInLessThanElevenDays(Item item) {
return item.sellIn < 11;
}
@Override
public void decreaseSellIn(Item item) {
item.sellIn--;
}
}
public class AgedBrieQualityStrategy extends QualityStrategy {
@Override
protected void updateQualityByItemType(Item item) {
if (isSellInOver(item)) {
increaseQualityBy(2, item);
} else {
increaseQualityBy(1, item);
}
}
@Override
public void decreaseSellIn(Item item) {
item.sellIn--;
}
}
public class DefaultQualityStrategy extends QualityStrategy {
@Override
protected void updateQualityByItemType(Item item) {
if (isSellInOver(item)) {
decreaseQualityBy(2, item);
} else {
decreaseQualityBy(1, item);
}
}
@Override
public void decreaseSellIn(Item item) {
item.sellIn--;
}
}
public class SulfurasQualityStrategy extends QualityStrategy {
@Override
public void updateQualityByItemType(Item item) {
return;
}
@Override
public void decreaseSellIn(Item item) {
return;
}
}
public class ConjuredQualityStrategy extends QualityStrategy {
@Override
protected void updateQualityByItemType(Item item) {
decreaseQualityBy(2, item);
}
@Override
protected void decreaseSellIn(Item item) {
item.sellIn--;
}
}
이번 단계에서는 앞서 찾은 단서를 토대로 리팩터링을 진행했습니다. 먼저 두 개로 나눠진 quality 수정 메서드를 하나로 합치고, 아이템별로 quality 값을 수정할 수 있게 조건문을 전략 패턴으로 변경했습니다. 이를 통해 핵심 요구 사항인 새로운 아이템을 추가해도 다형성을 활용해 코드 베이스의 변경을 최소화할 수 있게 됐습니다. 다만 두 번째 리팩터링의 경우 복잡도가 함께 올라가는 문제가 존재하기 때문에 실무에서는 조심스럽게 접근해야 합니다.
<중간 결론>
만약 첫 번째 단계 없이 바로 두 번째 단계의 리팩터링을 하려 했다면 중첩된 조건문에서 방향을 잃고 더 많은 시간을 소모했을 것입니다. 다시 말하자면 리팩터링엔 어느 정도 방향성이 존재합니다. 두 번째 단계와 같은 더 큰 단위(클래스)의 변경은 작은 단위(메서드)의 변경이 선행되어야 합니다. 작은 단위의 변경은 코드 베이스에 대한 깊은 이해를 이끌어내며, 더 큰 단위의 변경을 시도할 수 있는 좋은 단서가 됩니다.
테스트 코드가 없다면 리팩터링 전후 결과가 같은지, 어떤 사이드 이펙트가 발생할지 쉽게 확인할 수 없습니다. 이러한 이유로 테스트 코드는 리팩터링에 반드시 필요합니다. 이번에 진행한 리팩터링 또한 아래와 같은 테스트 코드를 먼저 작성해 변경 전후 결과가 같음을 보장할 수 있었습니다. (여기서 언급되는 테스트는 자동화 테스트를 의미합니다.)
코드 예제 4) 테스트 코드 일부
class GildedRoseTest {
@DisplayName("특수 아이템을 제외한 일반 아이템은 하루가 지날 때마다 판매 기간, 가치가 1씩 감소한다.")
@Test
void updateQualityWhenNormalItem() {
Item[] items = new Item[] {new Item("+5 Dexterity Vest", 10, 20)};
GildedRose app = new GildedRose(items);
app.updateQuality();
assertAll(
() -> assertEquals("+5 Dexterity Vest", app.items[0].name),
() -> assertEquals(9, app.items[0].sellIn),
() -> assertEquals(19, app.items[0].quality)
);
}
. . .
}
이렇게 작성된 테스트 코드는 자주 실행되어야 그 가치를 증명할 수 있습니다. 작은 변경마다 테스트를 실행하여 문제가 발생할 수 있는 범위를 최소화할 수 있기 때문입니다. 저 또한 실제로 리팩터링을 할 땐 아래 커밋 내역처럼 작은 단위로 작업을 수행하며 테스트를 반복했습니다.
만약 테스트 코드를 작성하지 않았다면 리팩터링 전후 결과가 같다고 확신할 수 없었을 것입니다. 또한 어느 부분에서 버그가 발생했는지 파악하는 데 시간이 더 걸릴 것이고, 전체적인 생산성은 저하될 수밖에 없었을 것입니다. 이러한 이유로 테스트 코드 또한 리팩터링만큼 중요하다고 볼 수 있습니다.
테스트 코드가 중요한 또 다른 이유는 코드 구조 개선에 선순환을 만들 수 있기 때문입니다. 하나의 테스트 코드는 하나의 기능만을 테스트하고 메서드는 하나의 기능만을 수행하며, 클래스는 하나의 책임만을 가지는 것이 바람직한 모습입니다. 만약 테스트하기 어려운 코드라면 이 모습과 동떨어져 있을 가능성이 있습니다. 구체적으로 다음 두 가지의 경우가 존재합니다.
하나의 메서드에 여러 기능이 존재하는 경우
하나의 클래스에 여러 책임이 존재하는 경우
(*테스트 코드를 작성하기 어려운 경우는 굉장히 많습니다. 여기선 도메인, 클래스 설계와 관련된 문제에 한정하여 작성했습니다.)
일반적으로 하나의 public 메서드에 여러 개의 private 메서드가 따라오며 군집 형태를 이루는 것이 바람직한 모습입니다. 첫 번째의 경우 대개 이러한 군집이 public 메서드에 몰려있습니다. 마치 앞서 예제로 보여드린 ‘Gilded Rose Refactoring Kata’의 원본 코드와 같은 경우입니다. 해당 프로젝트에선 배경, 요구사항이 줄글로 명시되어 있어서 테스트 코드 작성이 어렵지 않았습니다. 하지만 반대였다면 테스트 코드 작성이 어려웠을 것이며, 이 경우 IDE 자체에서 제공하는 도구를 활용해 안정적으로 리팩터링하여 코드 구조를 먼저 개선해 볼 수 있습니다.
두 번째의 경우 의존하는 객체가 많아 테스트 코드 작성을 위한 설정이 어렵습니다. 더불어 테스트 케이스가 지나치게 많아져 테스트 코드가 문서로서의 역할을 제대로 하지 못하게 됩니다. 이 경우엔 public 메서드 군집 단위로 객체의 책임을 분리해 보거나 공통된 작업을 수행하는 private 메서드 단위로 책임을 분리하는 방향으로 구조를 개선해 볼 수 있습니다.
위 이야기를 정리하자면 아래와 같은 선순환이 생기게 됩니다.
[간단한 리팩터링 → 테스트 코드 작성 → 리팩터링 → 테스트 코드 보완 → 리팩터링]
사실 테스트 코드를 작성하기 어려운 경우에 대해 다소 모호하고 추상적인 조건을 제시했는데요, 이는 경험적인 부분에 의존할 수밖에 없기 때문입니다. 다만, 이와 같은 기준을 나아가야 할 큰 방향으로 잡고 다양한 경험을 쌓으면 구현체가 될 기준들이 차곡차곡 쌓일 것이라 생각합니다.
마틴 파울러는 자신의 저서 리팩터링 2판에서 이렇게 말합니다.
<인용문 1>
“사람들이 빠지기 쉬운 가장 위험한 오류는 리팩터링을 ‘클린 코드’나 ‘바람직한 엔지니어링 습관’처럼 도덕적인 이유로 정당화하는 것이다. 리팩터링의 본질은 코드 베이스를 예쁘게 꾸미는 데 있지 않다. 오로지 경제적인 이유로 하는 것이다.”
리팩터링은 최소한의 비용으로 최대한의 효과를 누리기 위한 일종의 도구지 그 자체로 추구해야 할 목적은 아닙니다. 그러므로 리팩터링하는 것이 옳고, 하지 않는 것이 그르다란 도덕적인 잣대를 내미는 것은 지양해야 할 태도입니다. 대신 내가 리팩터링으로 얻을 수 있는 이점이 많을 경우에 수행하는 것이 지향해야 할 태도입니다.
리팩터링 해야 하는 또 다른 순간은 도메인과 코드 베이스 이해도 향상을 원할 때입니다. 마틴 파울러 또한 자신의 저서 리팩터링 2판에서 이렇게 말했습니다.
<인용문 2>
“코드를 분석할 때 리팩터링을 해보면, 그렇지 않았다면 도달하지 못했을 더 깊은 수준까지 이해하게 된다. 이해를 위한 리팩터링을 의미 없이 코드를 만지작거리는 것이라고 무시하는 이들은 복잡한 코드 아래 숨어 있는 다양한 기회를 결코 발견할 수 없다.”
대생성형 AI 시대에 딸깍 한 번으로 리팩터링을 손쉽게 수행할 수 있습니다. 이는 비용적인 측면에서는 옳지만, 도메인과 코드 베이스 이해도 향상에는 좋지 않은 방법입니다. 그 때문에 새로운 코드 베이스를 파악해야 할 경우 직접 리팩터링하는 것은 아주 좋은 수단이 될 수 있습니다. 이 외에도 리팩터링해야 할 많은 순간들이 존재합니다. 관련된 내용은 마틴 파울러의「리팩터링 2판」에 더욱 상세히 작성되어 있습니다.
읽는 사람으로 하여금 리팩터링과 테스트 코드의 중요성을 깨달을 수 있는 계기가 되었으면 하는 바람이 있어, 글을 어떻게 풀어 나갈지 많이 고민했습니다. 더욱이 꽤나 진부하고도 깊은 주제이기에 고민은 더욱 깊어졌습니다. 고민 결과, 「리팩터링 2판」1장의 구조를 참고하여 겪어온 경험을 예제 코드에 녹여내는 방법이 진정성을 조금 더 높일 것이라고 판단하며 글을 작성하였습니다.
주니어 개발자의 관점에서 볼 때도 리팩터링과 테스트 코드는 건강한 코드 베이스를 유지하는 데 매우 중요합니다. 그렇기에 마치 우리 몸과 마음의 건강처럼 특별히 시간을 내서 챙기기보다는 평소에 습관처럼 관리하는 것이 바람직하다고 생각합니다.
코드 예제 출처 : https://github.com/emilybache/GildedRose-Refactoring-Kata
인용문 출처 : 리팩터링 2판 / 마틴파울러 저 / 개앞맵시, 남기혁 역 / 한빛 미디어 / 2020년
—
by 아이나비시스템즈 응용기술개발팀 백종훈
#리팩터링 #테스트코드 #백엔드개발자 #코딩 #코드베이스 #메서드 #아이나비 #아이나비내비게이션 #지도 #아이나비시스템즈