[리팩터링 2판 - ch.1] - statement() 함수 쪼개기

시작

미뤄두고 미뤄두던 리팩토링 2판을 읽기 시작했다..! Vue.js 공부하기도, 사이드 프로젝트 진행하기도, 일하기도 바쁜데 이건 언제하나? 싶지만 시간 내서 개발 서적을 읽지 않으면 당연히 실력을 늘릴 수 없겠지? 하는 생각에 산지 두달이나 됐지만 이제 읽기 시작한다. 예시 코드로 리팩토링을 시작해보자.

예시 1: 연극

다양한 연극을 외주로 받아서 공연하는 극단이 있다고 생각해보자. 공연 요청이 들어오면 연극의 장르와 관객 규모를 기초로 비용을 책정한다. 현재 이 극단은 두 가지 장르, 비극과 희극만 공연한다. 그리고 공연료와 별개로 포인트(volume credit)를 지급해서 다음 번 의뢰 시 공연료를 할인받을 수도 있다. 일종의 충성도 프로그램인 것이다.

먼저, 극단은 공연할 연극 정보를 play.json 파일에 저장한다.

// plays.json
{
	"hamlet": {
		"name": "hamlet",
		"type": "tragedy"
	},
	"as-like": {
		"name": "As You Like It",
		"type": "comedy"
	},
	"othello": {
		"name": "Othello",
		"type": "tragedy"
	}
}

공연료 청구서에 들어갈 데이터는 invoices.json에 저장한다.

// invoices.json
[
  {
    "customer": "BigCo",
    "performances": [
      {
        "playID": "hamlet",
        "audience": 55
      },
      {
        "playID": "as-like",
        "audience": 35
      },
      {
        "playID": "othello",
        "audience": 40
      }
    ]
  }
]

공연료 청구서를 출력하는 코드는 다음과 같이 구현한다. (리팩토링 이전 코드)

// Before refactoring
import plays from "./plays.json";
import invoices from "./invoices.json";

function statement(invoice, plays) {
  let totalAmount = 0;
  let volumeCredits = 0;
  let result = `청구내역 (고객명: ${invoice.customer})\n`;
  const format = new Intl.NumberFormat("en-US", {
    style: "currency",
    currency: "USD",
    minimumFractionDigits: 2,
  }).format;

  for (let perf of invoice.performances) {
    const play = plays[perf.playID];
    let thisAmount = 0;

    switch (play.type) {
      case "tragedy":
        thisAmount = 40000;
        if (perf.audience > 30) {
          thisAmount += 1000 * (perf.audience - 30);
        }
        break;

      case "comedy":
        thisAmount = 30000;
        if (perf.audience > 20) {
          thisAmount += 10000 + 500 * (perf.audience - 20);
        }
        thisAmount += 300 * perf.audience;
        break;
      default:
        throw new Error(`알 수 없는 장르: ${play.type}`);
    }

    // 포인트를 적립한다.
    volumeCredits += Math.max(perf.audience - 30, 0);
    // 희극 관객 5명마다 추가 포인트를 제공한다.
    if ("comedy" === play.type) volumeCredits += Math.floor(perf.audience / 5);

    // 청구 내역을 출력한다.
    result += `${play.name}: ${format(thisAmount / 100)} (${
      perf.audience
    }석)\n`;
    totalAmount += thisAmount;
  }

  result += `총액: ${format(totalAmount / 100)}\n`;
  result += `적립 포인트: ${volumeCredits}점\n`;
  return result;
}

invoices.map((invoice) => console.log(statement(invoice, plays)));

이 코드를 실행한 결과는 다음과 같다.

청구 내역 (고객명: BicCo)
  Hamlet: $650.00 (55석)
  As You Like It: $580.00 (35석)
  Othello: $500.00 (40석)
총액: $1,730.00
적립 포인트: 47점

예시 코드를 모두 읽고난 후, 어떤가? 리팩토링의 이유를 찾아보자.

일단 나는 꽤 쓸만하다 생각했다. 아니, 오히려 내가 코드를 작성한다면 저만큼 깔끔하게 작성할 수 있을까..? 라는 수준까지 생각했다. (이건 심지어 리팩토링을 하기 전의 코드다)

그런데 여기서 생각해보자. 만약 희극/비극 뿐만 아니라 고전극, 전원극, 자유극 등 더 많은 장르가 추가된다면? 이러한 변경은 공연료/적립 포인트 계산 로직에 분명 영향을 주게 될 것이다. 한번 작성하고 다시는 변경하지 않을 코드라면 상관이 없겠지만, 요구사항이 새롭게 추가, 혹은 변경될 때마다 개발자는 수많은 로직을 함께 변경해야 할 것이다.

이렇듯 리팩토링이 필요한 이유는 변경 때문이다.

리팩터링 첫 단계

리팩터링의 첫 단계는 항상 똑같다. 리팩터링할 코드 영역을 꼼꼼하게 검사해줄 테스트 코드들 먼저 마련해야 한다.

테스트를 작성하는 데에 시간이 오래 걸려서 오히려 테스트 코드를 작성하는 것이 개발 시간을 더 많이 잡아먹는 것 아닌가요? 라고 생각할 수 있다. 그러나 책의 저자는 신경 써서 만들어두면 디버깅 시간이 줄어서 전체 작업 시간은 오히려 단축된다고 말하고 있다.

개발 중 가장 많은 시간이 걸리는 부분이 어디일까? 신규 기능 개발? 환경 설정? 디버깅? (필자는 신규 기능 개발이라고 생각했었다) 그러나 대체로 가장 개발 과정 중 가장 오래 걸리는 부분이 바로 디버깅이라고 한다.

테스트 코드는 이후 4장에서 길게 다룰 예정이기 때문에 다른 부분을 먼저 리팩토링 해보자.

statement() 함수 쪼개기

statement()처럼 긴 함수를 리팩터링할 때는 먼저 전체 동작을 각각의 부분으로 나눌 수 있는 지점을 찾아야 한다. 아까 코드에서 중간에 swtich 문이 있었던 것을 기억하는가?

    switch (play.type) {
      case "tragedy":
        thisAmount = 40000;
        if (perf.audience > 30) {
          thisAmount += 1000 * (perf.audience - 30);
        }
        break;

      case "comedy":
        thisAmount = 30000;
        if (perf.audience > 20) {
          thisAmount += 10000 + 500 * (perf.audience - 20);
        }
        thisAmount += 300 * perf.audience;
        break;
      default:
        throw new Error(`알 수 없는 장르: ${play.type}`);
    }

이 swtich문은 한번의 공연에 대한 요금을 계산하고 있다. 이러한 사실은 코드를 분석해서 얻은 정보인데, 코드 분석을 통해 파악한 정보는 휘발성이 높기로 악명 높은 저장 장치인 내 머릿속에 기록되므로, 잊지 않으려면 재빨리 코드에 반영해야한다.(대표적인 방법: 함추 추출하기 )

이 switch 문을 확인했을 때, 유효범위를 벗어나는 변수, 즉 새 함수에서는 곧바로 사용할 수 없는 변수가 있는지 확인한다. 이번 예에서는 perf, play, thisAmount가 속한다.

perf와 play는 추출한 새 함수에서도 필요하지만 값을 변경하지 않기 때문에 매개변수로 전달하면 된다.

반면 thisAmount는 함수 안에서 값이 바뀌는데, 이런 변수는 주의를 해야 한다. 이번 예에는 이런 변수가 하나뿐이므로 이 값을 반환하도록 작성했다. 또한 이 변수를 초기화하는 코드도 추출한 함수에 넣었다. 결과를 보자.

function amountFor(perf, play) {
  	let thisAmount = 0;
  
     switch (play.type) {
      case "tragedy":
        thisAmount = 40000;
        if (perf.audience > 30) {
          thisAmount += 1000 * (perf.audience - 30);
        }
        break;

      case "comedy":
        thisAmount = 30000;
        if (perf.audience > 20) {
          thisAmount += 10000 + 500 * (perf.audience - 20);
        }
        thisAmount += 300 * perf.audience;
        break;
      default:
        throw new Error(`알 수 없는 장르: ${play.type}`);
    } 
  return thisAmount; // 함수 안에서 값이 바뀌는 변수 반환
}

이제 위쪽의 코드를 아까 추출한 amountFor() 함수를 호출하여 바꿔보자.

function statement(invoice, plays) {
  let totalAmount = 0;
  let volumeCredits = 0;
  let result = `청구내역 (고객명: ${invoice.customer})\n`;
  const format = new Intl.NumberFormat("en-US", {
    style: "currency",
    currency: "USD",
    minimumFractionDigits: 2,
  }).format;

  for (let perf of invoice.performances) {
    const play = plays[perf.playID];
    let thisAmount = amountFor(perf, play);
    // ...

이렇게 수정한 코드는 바로 테스트를 해야 한다. 이러면 오류가 생기더라도 변경 폭이 작고 살펴볼 범위도 좁아서 문제를 찾고 해결하기가 훨씬 쉽다.

이렇듯 함수를 추출하고 나면 추출된 함수 코드를 자세히 들여다보면서 지금보다 명확하게 표현할 수 있는 간단한 방법은 없는지 검토한다. thisAmount 또한 이해할만한 변수명이지만, result로 바꾼다면 더 명확해질 것이다.

play 변수 제거하기

이 책의 저자는 긴 함수를 잘게 쪼갤 대마다 play 같은 변수를 최대한 제거한다고 한다. 이런 임시 변수들 때문에 로컬 범위에 존재하는 이름이 늘어나서 추출 작업이 복잡해지기 때문이라고 함!

이를 해결해주는 리팩터링으로는 '임시 변수를 질의 함수로 바꾸기'가 있다고 한다.

먼저 대입문(=)의 우변을 함수로 추출한다. (이전에, 저자가 perf라는 변수를 aPerformance로 변경했기 때문에, 다음 예제에서는 모두 aPerformance 변수를 쓴다)

function playFor(aPerformance) {
	return plays[aPerformace.playID]
}

function statement(invoice, plays) {
  let totalAmount = 0;
  let volumeCredits = 0;
  let result = `청구내역 (고객명: ${invoice.customer})\n`;
  const format = new Intl.NumberFormat("en-US", {
    style: "currency",
    currency: "USD",
    minimumFractionDigits: 2,
  }).format;

  for (let perf of invoice.performances) {
    const play = playFor(perf); // 우변을 함수로 추출
    let thisAmount = amountFor(perf, play);
    // ...

이후 '변수 인라인하기'를 적용한다.

let thisAmount = amountFor(perf, playFor(perf));

function amountFor(aPerformance, play) {
  	let result = 0;
  
     switch (playFor(aPerformance).type) { // play를 playFor() 호출로 변경
      case "tragedy":
        result = 40000;
        if (aPerformance.audience > 30) {
          result += 1000 * (aPerformance.audience - 30);
        }
        break;

      case "comedy":
        result = 30000;
        if (aPerformance.audience > 20) {
          result += 10000 + 500 * (aPerformance.audience - 20);
        }
        result += 300 * aPerformance.audience;
        break;
      default:
        throw new Error(`알 수 없는 장르: ${playFor(aPerformance).type}`); // play를 playFor() 호출로 변경
    } 
  return result; // 함수 안에서 값이 바뀌는 변수 반환
}

마지막으로, 필요가 없어진 매개변수 play를 제거해준다.

let thisAmount = amountFor(perf);

function amountFor(aPerformance) // play매개변수 제거
// ...

방금 수행한 리팩터링에서, 이전 코드는 루프를 한번 돌 때마다 공연을 조회했는데 지금 코드에서는 세번이나 조회한다. 후에 리팩토링과 성능의 관계를 설명하겠지만, 현재 코드에는 성능에 그렇게 큰 차이가 없고, 실제로 느려지더라도 제대로 리팩터링된 코드베이스는 그렇지 않은 코드보다 성능을 개선하기가 훨씬 수월하다.

이렇게 지역변수를 제거해서 얻는 가장 큰 장점은 추출 작업이 훨씬 쉬워진다는 것이다. 유효범위를 신경 써야 할 대상이 줄어들기 때문이다. (실제로 저자는 추출 작업 전에 거의 항상 지역 변수부터 제거한다고 한다)

자, 그럼 지역변수가 두개 더 있는데, 나는 그 중 volumenCredits 변수만 리팩터링 해보도록 하겠다. 관련 코드는 다음과 같다.

// volumeCredits 관련 코드

// 포인트를 적립한다.
volumeCredits += Math.max(perf.audience - 30, 0);
// 희극 관객 5명마다 추가 포인트를 제공한다.
if ("comedy" === play.type) volumeCredits += Math.floor(perf.audience / 5);

이렇게 값을 누적해야 하는 코드는 어떤 식으로 리팩터링 해야할까? 저자가 말하는 최선의 방법은 추출한 함수에서 volumeCredits의 복제본을 초기화한 뒤 계산 결과를 반환토록 하는 것이다.

function volumenCreditsFor(perf) {
  let volumeCredits = 0;
  volumeCredits += Math.max(perf.audience - 30, 0);
  if ("comedy" === play.type) volumeCredits += Math.floor(perf.audience / 5);
  
  return volumeCredits;
}

for(let perf of invocies.performances) {
 volumeCredits += volumeCreditsFor(perf); // 추출한 함수를 이용해 값을 누적
 //...
}

한눈에 봐도 깔끔하다. 그럼, 다음에 할 일로 이제 리팩터링한 volumentCredits 변수를 제거해보자.(???)

volumentCredits 변수 제거하기

이 변수는 loop을 돌 때마다 값을 누적하기 때문에 리팩터링하기 더 까다로운데, 먼저 '반복문 쪼개기'로 volumentCredits값이 누적되는 부분을 따로 빼낸다.

for(let perf of invocies.performances) {
 result += `${playFor(blah blah..)}`
 // ...
}

for(let perf of invocies.performances) {
 volumeCredits += volumeCreditsFor(perf); // 추출한 함수를 이용해 값을 누적
}

이어서 '문장 슬라이드하기'를 적용해서 volumenCredits 변수를 선언하는 문장을 반복문 바로 앞으로 옮긴다.

let volumeCredits = 0; // 변수 선언(초기화)을 반복문 앞으로 이동
for(let perf of invocies.performances) {
 volumeCredits += volumeCreditsFor(perf); // 추출한 함수를 이용해 값을 누적
}

이렇게 volumenCredits 값 갱신과 관련한 문장들을 한데 모아두면 임시 변수를 질의 함수로 바꾸기가 수월해진다. 이번에도 volumenCredits값 계산 코드를 함수로 추출하는 작업을 해보자.

// volumeCredits의 토탈을 구하는 함수 

function totalVolumeCredits() {
 let volumeCredits = 0;
 for(let perf of invoice.performances) {
  	volumeCredits += volumeCreditsFor(perf); 
 }
 return volumeCredits;

// ...

let volumeCredits = totalVolumeCredits();

함수 추출이 끝났다면, 다음은 volumenCredits 변수를 인라인할 차례다.

// 변수 인라인 전 코드
result += `총액: ${volumeCredits}점`;

// 변수 인라인 후 코드
result += `총액: ${totalVolumeCredits()}점`;

여기서 잠시 생각을 해보자. 우리는 반복문을 쪼개서 성능이 느려지는 것을 원하지 않는다. 시간 복잡도를 중요하게 생각한다면 특히나 더 그럴 것이다. 그러나 이 정도 중복은 성능에 미치는 영향이 미미할 때가 많다.

물론 리팩터링 후 성능에 유의미한 악영향을 미치는 경우가 있다. 그러나 저자는 이러한 경우에도 개의치 않고 리팩터링을 진행한다고 한다.

잘 다듬어진 코드라야 성능 개선 작업도 훨씬 수월하기 때문이라고.

이러한 경우 리팩터링한 코드를 기반으로 시간을 내어 성능을 개선한다고 한다. 따라서 특별한 경우가 아니라면 일단 성능 문제는 무시하고 리팩터링을 진행하라고 저자는 조언한다.

결론

1. 프로그램이 새로운 기능을 추가하기에 편한 구조가 아니라면, 먼저 기능을 추가하기 쉬운 형태로 리팩터링하고 나서 원하는 기능을 추가한다.

2. 리팩터링하기 전에 제대로 된 테스트부터 마련한다. 테스트는 반드시 자가진단하도록 만든다.

3. 코드 분석을 통해 파악한 정보는 휘발성이 높기로 악명 높은 저장 장치인 내 머릿속에 기록되므로, 잊지 않으려면 재빨리 코드에 반영해야한다

4. 리팩터링은 프로그램 수정을 작은 단계로 나눠 진행한다. 그래서 중간에 실수하더라도 버그를 쉽게 찾을 수 있다.

5. 특별한 경우가 아니라면 일단 성능 문제는 무시하고 리팩터링을 진행하라.

이 글은 '리팩터링 2판(코드 구조를 체계적으로 개선하여 효율적인 리팩터링 구현하기) - 마틴 파울러 저'을 읽고 책의 내용 + 제 생각을 함께 정리한 글입니다. 문제가 될 시 삭제하겠습니다.