리팩토링 읽고 적용해보기 - 클래스로 다형성 구현하기, 조건문 뎁스 줄이기

리팩토링 공부한 내용 적용해보기

개발을 하면서 리팩토링을 진행하는 일이 많아졌습니다.

어떻게 하면 리팩토링을 더 잘 할 수 있을까하는 생각이 많이 들었죠..
읽은 책 내용을 정리만 한다고 리팩토링 실력이 느는건 아니니까요.
그래서 오픈소스 레포지토리를 사용해 공부한 내용을 적용해보는 중입니다.
오늘은 지금까지 리팩토링 책에서 읽은 내용을 어떻게 적용시켜봤는지에 대해서 한번 정리해보겠습니다.

문제 소개

Gilded-Rose란?

공개되어 있는 오픈 소스 리팩토링 과제입니다.
fork해서 제시된 로직을 요구사항에 맞게 리팩토링해볼 수 있습니다.

Gilded Rose 요구사항 명세

안녕하세요, Gilded Rose에 오신 것을 환영합니다. 우리는 도시의 주요 지역에있는 작은 숙소(상점)이며, Allison(앨리슨)이라는 상냥한 사람이 운영하고 있습니다.

우리는 최고의 제품만을 구입하여 판매하고 있습니다. 불행히도, 상품 판매 기한이 가까워 질수록 품질이 저하되어가고 있습니다.

우리는 재고를 업데이트하는 시스템이 있습니다. 이 시스템은 지금은 새로운 모험을 떠나고 없는 Leeroy(리로이)라는 빡빡한 성격의 인물에 의해 개발되었습니다.

당신이 할 일은 시스템에 새로운 기능을 추가하여, 새로운 카테고리의 상품을 판매할 수 있도록하는 것입니다.

시스템 소개

먼저 시스템을 소개합니다.

• 모든 아이템은 SellIn 값을 가지며, 이는 아이템을 판매해야하는 (남은) 기간을 나태냅니다.
• 모든 아이템은 Quality 값을 가지며, 이것은 아이템의 가치를 나타냅니다.
• 하루가 지날때마다, 시스템은 두 값(SellIn, Quality)을 1 씩 감소시킵니다.

간단하죠? 흥미로운 부분은 지금부터입니다.

• 판매하는 나머지 일수가 없어지면, Quality 값은 2배로 떨어집니다.
• Quality 값은 결코 음수가 되지는 않습니다.
• "Aged Brie"(오래된 브리치즈)은(는) 시간이 지날수록 Quality 값이 올라갑니다.
• Quality 값은 50를 초과 할 수 없습니다.
• Sulfuras는 전설의 아이템이므로, 반드시 판매될 필요도 없고 Quality 값도 떨어지지 않습니다.
• "Backstage passes(백스테이지 입장권)"는 "Aged Brie"와 유사하게 SellIn 값에 가까워 질수록 Quality 값이 상승하고, 10일 부터는 매일 2 씩 증가하다, 5일 부터는이 되면 매일 3 씩 증가하지만, 콘서트 종료 후에는 0으로 떨어집니다.

시스템 업데이트 요구 사항

최근 "Conjured"(마법에 걸린) 상품 공급 업체와 계약했습니다. 따라서 시스템의 업데이트가 필요합니다.

• "Conjured" 아이템은 일반 아이템의 2배의 속도로 품질(Quality)이 저하됩니다.

모든 것이 제대로 작동하는 한에서는 UpdateQuality() 메서드를 변경하거나 새로운 코드의 추가를 자유롭게 할 수 있습니다. 그러나 Item 클래스와 Items 속성은 변경하지 마세요.

이것들은 저기 구석에있는 고블린의 것이고, 그 친구는 코드의 공유 소유권을 믿지 않기 때문에, 미친듯이 화를 내며(insta-rage) 여러분에게 한 방(one-shot)을 날릴 수도 있습니다. (UpdateQuality() 메서드와 Items 속성을 정적(static)으로 만드는 것은 괜찮습니다. 저희가 책임질게요.)

다시 한 번 확인하자면, 아이템의 Quality는 50 이상으로 증가할 수는 없습니다. 하지만 Sulfuras는 전설의 아이템이기 때문에 Quality 값은 80이며, 값이 바뀌지 않습니다.

적용하기

1. for 문 변경해 가독성 높이기

우선 로직을 살펴보면, for문으로 this.items[i]와 같은 형태로 접근하고 있는 코드가 정말 많습니다.

for (let i = 0; i < this.items.length; i++) {
      if (this.items[i].name != 'Aged Brie' && this.items[i].name != 'Backstage passes to a TAFKAL80ETC concert') {
        if (this.items[i].quality > 0) {
          if (this.items[i].name != 'Sulfuras, Hand of Ragnaros') {
            this.items[i].quality = this.items[i].quality - 1
          }
//...

그러나 i+1과 같이 인덱스로 접근하는 다른 코드가 없기 때문에 for…in문으로 수정해 가독성을 개선해보겠습니다.

for (const item of this.items) {
      if (item != 'Aged Brie' && item.name != 'Backstage passes to a TAFKAL80ETC concert') {
        if (item.quality > 0) {
          if (item != 'Sulfuras, Hand of Ragnaros') {
            item.quality = item.quality - 1
          }
//...

2. 상수 사용하기

로직 특성상 다양한 종류의 아이템을 처리해야해 많은 조건문을 포함하고 있습니다. 그런데 아이템의 이름이 일반 문자열로 되어 있는데, 문자열의 길이가 길고 같은 문자열이 여러번 반복되어 가독성과 유지보수 측면에서 불리한 점이 있습니다.

if (item != 'Aged Brie' && item.name != 'Backstage passes to a TAFKAL80ETC concert') {
  //...

따라서 상수로 관리해 오타로 인한 오류의 가능성을 줄이고, 가독성도 높이겠습니다. 또한 이렇게 상수를 사용하면 자동완성 기능을 사용할 수 있기 때문에 개발 편의성도 증가합니다.

export const ITEMS = {
  BRIE: "Aged Brie",
  SURFRAS: "Sulfuras, Hand of Ragnaros",
  PASSES: "Backstage passes to a TAFKAL80ETC concert",
};

for (const item of this.items) {
      if (item != ITEMS.BRIE && item.name != ITEMS.PASSES) {
        if (item.quality > 0) {
          if (item != ITEMS.SURFRAS) {
            item.quality = item.quality - 1
          }
//...

3. 조건문 뎁스 줄이기

• 제어 플래그를 탈출문으로 바꾸기
• 중첩 조건문을 보호 구문으로 바꾸기

조건문의 뎁스를 줄이고, 특정 조건이 충족될 경우 그 아래의 코드를 읽지 않아도 되도록 가독성을 높이겠습니다.

일반적으로는 early return을 사용할 수도 있고, 여기에서는 for문 안이기 때문에 break문으로 빠져나갈 수 있습니다.

이 때 정말 많은 중복 코드가 발생하는 것을 발견할 수 있어 중복 코드를 전부 삭제할 수 있습니다.

for (const item of this.items) {
      if (!item.quality) break;
      if (item.name != ITEMS.SURFRAS) {
        item.sellIn -= 1;
			this.handleIfSellInIs0(item);

      if (item.name != ITEMS.BRIE && item.name != ITEMS.PASSES) {
        if (item.name === ITEMS.SURFRAS) break;
        item.quality -= 1;

			if (item.quality >= 50) break;
      item.quality += 1;

      this.handlePassesQuality(item);
//...

다음으로 조건식 통합/분해하기에 앞서, 다소 로직들이 엉켜있기 때문에 함수 추출이 필요합니다.

4. 함수 추출하기

다음과 같은 로직은 굉장히 복잡하지만, 자세히 살펴보면 다음과 같은 로직이라는 걸 알 수 있습니다.

• items.sellIn이 0 이하로 떨어지는 것 = 판매 기한이 지나는 아이템에 대한 조건문 내부 로직입니다.

//...
if (item.name != ITEMS.SURFRAS) {
  item.sellIn = item.sellIn - 1;
}
if (item.sellIn < 0) {
  if (item.name != ITEMS.BRIE) {
    if (item.name != ITEMS.PASSES) {
      if (item.quality > 0) {
        if (item.name != ITEMS.SURFRAS) {
          item.quality = item.quality - 1;
        }
      }
    } else {
      item.quality = item.quality - item.quality;
    }
  } else {
    if (item.quality < 50) {
      item.quality = item.quality + 1;
    }
  }
}
//...

그리고 판매 기한이 지났을 경우의 아이템별 로직을 switch문으로 별도의 함수로 분리해 처리합니다.
전체 로직에서는 해당 로직이 어떤 로직인지 함수명으로 알 수 있고,

      if (item.name != ITEMS.SURFRAS) {
        item.sellIn -= 1;
      }
      this.handleIfSellInIs0(item);

이에 대한 자세한 로직은 함수 내부에서 확인하면 됩니다.
또한 switch문으로 작성했기 때문에 아이템별 로직 확인이 가능합니다.

  handleIfSellInIs0(item) {
    if (item.sellIn >= 0) return;

    switch (item.name) {
      case ITEMS.BRIE:
        if (item.quality >= 50) return;
        item.quality = item.quality + 1;
        break;
      case ITEMS.SURFRAS:
        item.quality = item.quality - item.quality;
        break;
      default:
        if (!item.quality) return;
        item.quality -= 1;
        break;
    }
  }

이제 로직의 조건문 뎁스가 감소했고, 전반적인 로직을 파악하기가 조금 더 쉬워졌습니다.
아직 리팩토링이 완료되지는 않았지만, 지금까지의 과정을 거쳐 코드는 이 정도로 정리되었습니다.

  updateQuality() {
    for (const item of this.items) {
      if (!item.quality) break;
      if (item.name != ITEMS.SURFRAS) {
        item.sellIn -= 1;
      }
      this.handleIfSellInIs0(item);

      if (item.name != ITEMS.BRIE && item.name != ITEMS.PASSES) {
        if (item.name === ITEMS.SURFRAS) break;
        item.quality -= 1;
      }

      if (item.quality >= 50) break;
      item.quality += 1;

      this.handlePassesQuality(item);
    }

    return this.items;
  }

여기에서 부터는 로직 자체의 가독성 보다는 구조적으로 클래스를 사용한 리팩토링을 진행해보겠습니다.

5. Class로 다형성 이용하기

전반적인 로직이 어느정도 읽어서 파악할 수 있을정도로 정리가 되었으니, 유지보수 측면에서 다양한 종류의 아이템별 처리로직에 대한 설계를 개선해보겠습니다.

클래스를 사용해 조건부 로직을 다형성으로 변경할 수 있습니다.

우선 속성을 변경할 수 없는, 기본 제공되는 Item 클래스 먼저 살펴보겠습니다.

export class Item {
  name: string;
  sellIn: number;
  quality: number;

  constructor(name, sellIn, quality) {
    this.name = name;
    this.sellIn = sellIn;
    this.quality = quality;
  }
}

name, sellIn, quality 3개 속성을 가지고 있고 이들을 인자로 받아 인스턴스를 생성해주는 클래스입니다. 이제 이 Item을 상속받아 다형성을 구현하는 종류별 아이템 클래스들을 만들겠습니다.

이렇게 하기로 결정한 이유가 잇는데요, 이 리팩토링의 핵심은 다음과 같다고 생각합니다.

• 각 종류별 아이템은 각기 다른 특성을 가짐
  • 날짜가 지남에 따라 품질이 얼마나 떨어지는지 혹은 증가하는지
  • 판매 기한이 지났을 때의 품질은 어떻게 변화하는지 등
• 판매할 아이템은 언제든 추가되거나 변경, 제외될 수 있음을 고려

기존 로직이 가장 가독성이 떨어지는 이유는 어디까지 각각의 로직이 영항을 미치는지 알 수 없다는 것입니다.

예를 들어, 이러한 조건이 있을 때:
A 아이템과 B 아이템은 둘 다 하루가 지날 때마다 quality가 1씩 떨어짐
C 아이템은 하루가 지날 때마다 quality가 2씩 떨어짐

기존 로직은 다음과 같은 방식대로 작성되어 있습니다.

if(item === A || item === B || item === C) {
	quality--;
}
if(item === C) {
  quality--;
}

이럴 경우 다른 복잡한 조건이 추가될 때의 사이드 이펙트를 예측하기 어려워집니다.

따라서 각 아이템별 클래스를 만들어보겠습니다.
각각의 클래스들은 Item클래스를 상속받되 각 아이템의 특성에 맞게 sellIn과 quality속성을 계산하도록 다형성을 구현합니다.

*이 단계에서부터는 타입스크립트 인터페이스 사용을 위해 상수 대신 일반문자열을 사용하고, 인터페이스를 정의해 사용하겠습니다. 더 이상 updateQuality() 메서드 내부에서 문자열을 중복 사용하고 있지도 않고, Item 상위 클래스에서 인스턴스 생성 시 인자로 전달 가능한 name property를 인터페이스로 규격화할 수 있기 때문입니다.

export class AgedBrie extends Item {
  constructor(sellIn, quality) {
    super("Aged Brie", sellIn, quality);
  }
  handleQuality() {
    if (this.quality >= 50 || this.quality === 0) return;
    if (this.sellIn === 0) {
      this.quality = this.quality / 2;
      return;
    }
    this.quality++;
  }

  handleSellIn() {
    this.sellIn--;
  }
}

export class Passes extends Item {
  constructor(sellIn, quality) {
    super("Backstage passes to a TAFKAL80ETC concert", sellIn, quality);
  }

  handleQuality() {
    if (this.quality >= 50 || this.quality === 0) return;

    if (this.sellIn === 0) {
      this.quality = 0;
      return;
    }

    if (6 <= this.sellIn && this.sellIn < 11) {
      this.quality += 2;
    }

    if (this.sellIn < 6) {
      this.quality += 3;
    }
  }

  handleSellIn() {
    this.sellIn--;
  }
}

그리고 기존의 GildedRose 클래스를 수정합니다.

GildedRose는 items 인스턴스 배열을 인자로 전달받아 각 인스턴스의 sellIn과 quality 속성을 변경하는 클래스입니다.

let gildedRose = new GildedRose([new Item("Aged Brie", 1, 0)]);

export class GildedRose {
  items: Array<Item>;

  constructor(items = [] as Array<Item>) {
    this.items = items;
  }

	//아이템의 quality와 sellIn을 계산하는 로직
	for (const item of this.items) {
      if (!item.quality) break;
      if (item.name != ITEMS.SURFRAS) {
        item.sellIn -= 1;
      }

인자로 전달받은 items의 인스턴스를 담은 배열들을 매핑하면서 각각의 item의 sellIn과 quality를 계산한 배열을 리턴합니다.
Item클래스를 상속받은 판매아이템 클래스는 여기에서 ItemForSale이라고 정의했습니다. 이 클래스는 Item을 상속받고 추가적인 메서드를 포함한 인터페이스를 따릅니다.

let gildedRose = new GildedRose([new AgedBrie(1, 0)]);

interface ItemForSale extends Item {
  handleQuality: () => void;
  handleSellIn: () => void;
}

export class GildedRose {
  items: ItemForSale[];
  constructor(items = [] as ItemForSale[]) {
    this.items = items;
  }

  updateQuality() {
    const updatedItems = this.items.map((item) => {
      item.handleSellIn();
      item.handleQuality();

      return item;
    });
    return updatedItems as ItemClasses;
  }
  }

각각의 아이템 인스턴스들은 자신들이 갖는 특성을 나타내는 handleSellIn, handlequality함수를 실행해 자신의 속성을 관리합니다.

6. 반복문을 파이프라인으로 바꾸기

또한 위 코드에서처럼 반복문을 파이프라인으로 변경할 수 있습니다.
이 내용은 for 문 안에서 다양한 처리를하는 것을 연결된 파이프라인으로 구성할 수 있다는 말인데, 그 예시를 들어보며 다음과 같습니다.

책 내용 먼저 살펴보기

책에서 공개한 예시는 다음과 같습니다.
for... of문을 사용해 특정 데이터를 여러 방식으로 변환하고 있다면,

export function acquireData(input) {
  const lines = input.split('\n');
  let firstLine = true;
  const result = [];
  for (const line of lines) {
    if (firstLine) {
      firstLine = false;
      continue;
    }
    if (line.trim() === '') continue;
    const record = line.split(',');
    if (record[1].trim() === 'India') {
      result.push({ city: record[0].trim(), phone: record[2].trim() });
    }
  }
  return result;
}

이런 방식으로 파이프라인을 구현해 변경할 수 있다는 내용입니다.

export function acquireData(input) {
  return input
    .split("\n")
    .splice(1)
    .filter((line) => line.trim() !== "")
    .map((line) => line.split(",").filter((line) => line[1].trim() === "India"))
    .map((line) => ({
      city: line[0].trim(),
      phone: line[2].trim(),
    }));
}

적용하기

완벽히 동일한 케이스는 아니지만 이 내용을 적용해볼 수 있습니다.
기존 코드는 for문을 사용하고 있지만, 이제 map메서드를 사용할 수 있습니다.

updateQuality() {
  for (const item of this.items) {
    //...
    return this.items;
  }

updateQuality() {
  return this.items.map((item) => {
	//...
    return item;
  });
}

새로운 아이템 추가하기

위에 언급된 시스템 요구사항을 볼 차례입니다.
과제 중 하나로, 특수한 성질을 가진 아이템이 상점에 추가되어야 합니다.
이 때 클래스로 다형성을 구현한 것이 장점이 될 수 있습니다.

해봅시다!

다형성으로 구현된 판매아이템 클래스 살펴보기

이제 기본적인 아이템이 가져야 하는 sellIn, quality, name 속성을 갖고 있지만 name에 따라 quality와 sellIn속성이 각기 다르게 변경되도록 각각의 다형성을 구현했습니다.

export class AgedBrie extends Item {
  constructor(sellIn, quality) {
    super("Aged Brie", sellIn, quality);
  }

  handleQuality() {
	//...
    if (this.sellIn === 0) {
      this.quality = this.quality / 2;
      return;
    }
    this.quality++;
  }

  handleSellIn() {
    if (this.sellIn <= 0) return;
    this.sellIn--;
  }
}

export class Passes extends Item {
  constructor(sellIn, quality) {
    super("Backstage passes to a TAFKAL80ETC concert", sellIn, quality);
  }

  handleQuality() {
	//...

    if (this.sellIn < 6) {
      this.quality += 3;
    }
  }

  handleSellIn() {
    if (this.sellIn <= 0) return;
    this.sellIn--;
  }
}

export class Surfras extends Item {
  constructor(sellIn, quality) {
    const QUALITY = 80;
    super("Sulfuras, Hand of Ragnaros", sellIn, QUALITY);
  }

  handleQuality() {}

  handleSellIn() {}
}

아이템 추가

Conjured 아이템을 추가하기 위해서는, 동일하게 클래스를 추가해주면 됩니다.

export class Conjured extends Item {
  constructor(sellIn, quality) {
    super("Conjured Mana Cake", sellIn, quality);
  }

  handleQuality() {
    if (this.quality >= 50 || this.quality === 0) return;
    if (this.sellIn === 0) {
      this.quality = this.quality / 4;
      return;
    }
    this.quality -= 2;
  }

  handleSellIn() {
    if (this.sellIn <= 0) return;
    this.sellIn--;
  }
}

Conjured는 마법에 걸린 아이템을 말하는데, 명세서에 따르면 마법에 걸린 아이템만의 특징이 있습니다. Mana Cake뿐아니라 다른 마법에 걸린 아이템이 나온다면, Conjured와 관련된 속성만을 담당하는 클래스를 분리해 상속받도록 구현할 수도 있으니 유지보수에서 더욱 장점을 가져갈 수 있을 거라 생각합니다.

사용할 때는 각각의 판매 아이템 클래스 인스턴스를 생성해 넘겨주기만 하면 됩니다.

    const gildedRose = new GildedRose([new Passes(1, 0)], [new Conjured(1, 0)]);
    const items = gildedRose.updateQuality();

정리하면,

export type ItemName =
  | "Aged Brie"
  | "Backstage passes to a TAFKAL80ETC concert"
  | "Conjured Mana Cake"
  | "Sulfuras, Hand of Ragnaros";

export class Item {
  name;
  sellIn;
  quality;

  constructor(name: ItemName, sellIn: number, quality: number) {
    this.name = name;
    this.sellIn = sellIn;
    this.quality = quality;
  }
}

export class AgedBrie extends Item {
  constructor(sellIn, quality) {
    super("Aged Brie", sellIn, quality);
  }

정리

이번 글에서는...

책만 읽고 예시만 보고서는 금방 잊어버리기도 하고, 어떻게 적용해야 할지 잘 알 수 없습니다.
공개되어 있는 코드로 로직을 리팩토링해보면서 적용하고 체득할 수 있는 건 좋은 경험이죠!
혼자서도 좋고, 함께 스터디하기에도 좋은 자료라고 생각합니다.

다음 글에서는...

이번 글에서는 로직 개선에 있어서 크게 가장 시급하다고 생각했던 2가지 포인트를 개선해보았습니다.

1. 조건문의 뎁스를 줄이기
2. 다양한 판매 아이템별 로직을 처리하기 위해 클래스로 다형성 구현하기

물론 이 코드 또한 아직 리팩토링할 여지가 많이 존재합니다.
다음 글에서 이 다음 단계의 리팩토링에 대해 정리해보겠습니다.