[리팩터링] Refactoring 2판 - 1장

 

1장에서 리팩터링 예시로 든 공연료 구하는 코드를 리팩토링 해보기로 했다.

 

➡️  공연료 청구서 출력하는 코드 리팩토링 하기

 

const plays = {
  "hamlet": {"name": "Hamlet", "type": "tragedy"},
  "as-like": {"name": "As you like it", "type": "comedy"},
  "othello": {"name": "Othello", "type": "tragedy"}
};

const invoices = {

  "customer": "Amy",
  "performances": [
    {
      "playID": "hamlet",
      "audience": 55
    },
    {
      "playID": "as-like",
      "audience": 35
    },
    {
      "playID": "othello",
      "audience": 40
    }
  ]
};

 

🖍️  책 기본 코드

function statement(invoices, plays) {
  let totalAmount = 0;
  let volumeCredits = 0;

  let results = `청구 내역 (고객명: ${invoices.customer}) \n`;
  const format = new Intl.NumberFormat("en-US", {
    style: "currency", currency: "USD", minimumFractionDigits: 2
  }).format;

  for(let perf of invoices.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 > 30) {
          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);

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

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

  return results;
}

console.log(statement(invoices, plays));

 

출력결과

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

 

 

🖍️  내 리팩토링

 

• 함수를 쪼갰다. 함수 추출하기가 이번 리팩토링의 9할 이상이었다.
• 제일 처음 필요한 데이터를 담은 객체를 만들어주는 함수를 먼저 짰다. (getPlayInfo). map을 통해 새로운 데이터를 담은 배열을 리턴해주었다(newPerformance)
• getPerformanceAmount 함수의 주석 부분이 1차 리팩터링한 코드였다. 타입이 두개뿐인데 switch문을 안쓰고 싶었다. case가 늘어날때도 보기 힘들 것 같았다. Map을 이용했다.(typeMap 함수), case가 늘어날때 직관적으로 확인하기 좋을 것 같았다. 타입에 따라 총액을 구하는 함수 자체가 줄어서 직관적이어 진 것 같다..(!)
• 다만 case가 없을때 오류처리하는 방식이 아쉽다. undefined일 경우 0을 return 하도록 ?? 연산자를 써서 처리했는데, 정확한 방법이 아니라 아쉽다. new Error를 처리하면 총 액이 NaN으로 나와서 0으로 return 하긴 했지만 오류를 반환하는 방식도 고민해봐야할 것 같다. 
• getOnePlayInfo 함수는 연극별 정보를 한 줄 한 줄 출력해주는 코드이다. reduce를 써서 문자열을 누적하고 싶었는데, 문자열 초기값 설정이 너무 잘 안되어서 결국 map을 썼다. 

 

const format = new Intl.NumberFormat("en-US", {
  style: "currency", currency: "USD", minimumFractionDigits: 2
}).format;


// 사용할 데이터를 가공할 함수
const getPlayInfo = (performances, plays) => {

  return performances.map((performance) => {
    let newPerformance = {};
    newPerformance.play = plays[performance.playID];
    newPerformance.audience = performance.audience;
    newPerformance.amount = getPerformanceAmount(newPerformance);
    newPerformance.point = getPerformanceVolumeCredit(newPerformance);
    return newPerformance;
  });

}


// "tragedy" 공연의 총액을 구하는 함수
const tragedyCalculator = (performance) => {
  let thisAmount = 40000;
  if(performance.audience > 30) {
    thisAmount += 1000 * (performance.audience - 30);
  }
  return thisAmount;
}

// "comedy" 공연의 총액을 구하는 함수
const comedyCalculator = (performance) => {
  let thisAmount = 30000;
  if(performance.audience > 30) {
    thisAmount += 10000 + 500 * (performance.audience - 20);
  }
  thisAmount += 300 * performance.audience;
  return thisAmount;
}

// getPerformanceAmount의 기본 switch문을 Map으로 리팩토링
// switch문을 써도 되지만, Map을 사용해 case가 추가될때 한눈에 보기 쉽게 변경
const getPerformanceAmount = (performance) => {
  // 금액 구하기
  const typeMap = {
    "tragedy": tragedyCalculator(performance),
    "comedy": comedyCalculator(performance)
  }

  return typeMap[performance.play.type] ?? 0;
}

const getPerformanceVolumeCredit = (performance) => {
  // 포인트 구하기
  let volumeCredits;
  volumeCredits = Math.max(performance.audience - 30, 0);

  if(performance.play.type === "comedy") {
    volumeCredits += Math.floor(performance.audience / 5);
  }
  return volumeCredits;
}

// 출력결과에서 원하는 play 하나의 정보를 print 하는 함수
const getOnePlayInfo = (performanceInfos) => {
  let result = "";
  performanceInfos.map((performance) =>
    result += `\n${performance.play.name}: ${format(performance.amount / 100)} (${performance.audience}석)`
  )
  return result;
}

//각 공연의 총합을 더하는 reduce 누적 함수
const getTotalAmount = (performanceInfos) => {
  return performanceInfos.reduce((total, performance) => total + performance.amount, 0)
}

//각 공연의 포인트를 더하는 reduce 누적 함수
const getTotalPoint = (performanceInfos) => {
  return performanceInfos.reduce((total, performance) => total + performance.point, 0)
}

// 최종 출력을 하는 호출 함수
const getStatementInfo = (invoices, plays) => {
  const performanceInfos = getPlayInfo(invoices.performances, plays);
  const getOnePlayInfos = getOnePlayInfo(performanceInfos);

  return `청구내역 (고객명: ${invoices.customer}) ${getOnePlayInfos}
총액: ${format(getTotalAmount(performanceInfos)/ 100)}
적립 포인트: ${getTotalPoint(performanceInfos)}점`;

}

const result = getStatementInfo(invoices, plays);
console.log(result);

 

처음에 각 play별로 총액을 구하는 함수는 if 조건문이었다.

조건문을 안쓰고 더 직관적으로 보이게 하고 싶어서 아래처럼 2차 리팩터링 했다.

 

기존 getPerformanceAmount 함수

// 기존함수

const getPerformanceAmount = (performance) => {
   let thisAmount;
	if(performance.play.type === "tragedy") {
     thisAmount = 40000;
     if(performance.audience > 30) {
       thisAmount += 1000 * (performance.audience - 30);
     }
   } else if(performance.play.type === "comedy") {
     thisAmount = 30000;
     if(performance.audience > 30) {
       thisAmount += 10000 + 500 * (performance.audience - 20);
     }
     thisAmount += 300 * performance.audience;
   } else {
     throw new Error('타입? 금액 계산? 여튼 오류입니다.')
   }

   return thisAmount;
 }

 

2차 리팩터링에서는 타입별로 총액을 구하는 함수를 나눴다. (tragedyCalculator, comedyCalculator)

그리고 typeMap으로 type별 key-value 값으로 묶었다. case가 늘어날 경우 Map에 함수와 type을 매칭해주면 getPerformanceAmount 코드가 간단해질 것 같았다. 

 

getPerformanceAmount 함수 refactoring

// "tragedy" 공연의 총액을 구하는 함수

const tragedyCalculator = (performance) => {
  let thisAmount = 40000;
  if(performance.audience > 30) {
    thisAmount += 1000 * (performance.audience - 30);
  }
  return thisAmount;
}


// "comedy" 공연의 총액을 구하는 함수

const comedyCalculator = (performance) => {
  let thisAmount = 30000;
  if(performance.audience > 30) {
    thisAmount += 10000 + 500 * (performance.audience - 20);
  }
  thisAmount += 300 * performance.audience;
  return thisAmount;
}


// getPerformanceAmount의 기본 switch문을 Map으로 리팩토링
// switch문을 써도 되지만, Map을 사용해 case가 추가될때 한눈에 보기 쉽게 변경

const getPerformanceAmount = (performance) => {
  // 금액 구하기
  const typeMap = {
    "tragedy": tragedyCalculator(performance),
    "comedy": comedyCalculator(performance)
  }

  return typeMap[performance.play.type] ?? 0;
}

 

 

 🖍️ 책 리팩토링 + 나의 코드 살짝 추가

 

• 함수 추출하기 & 클래스 사용이 두드러지는 책에서 제시한 리팩터링 코드이다. 
• 데이터를 먼저 만드는 과정은 비슷하다. (enrichPerformance 함수)
• 기존의 코드보다 코드량은 50줄 정도 늘어났지만, 가독성과 확장성이 더 좋다.(클래스)
•  performanceCalculator라는 클래스를 만들어 tragedy, comedy 타입에 따라 자식 클래스를 상속한 방식이 내 리팩터링과 달랐다. comedy 타입의 volumeCredit을 나는 if문으로 처리했는데, 책에서는 메서드 오버라이딩을 통해 comedyCalculator에서 처리한 방식이 깔끔했다. 

 

const format = new Intl.NumberFormat("en-US", {
  style: "currency", currency: "USD", minimumFractionDigits: 2
}).format;




function createStatementData (invoices, plays) {
  const result = {};
  result.customer = invoices.customer;
  result.performances = invoices.performances.map(enrichPerformance);
  result.totalAmount = totalAmount(result);
  result.totalVolumeCredits = totalVolumeCredits(result);


  return `청구내역 (고객명: ${result.customer})
   ${getPlayTotalAmount(result.performances)}
    총액: ${format(result.totalAmount / 100)}
    적립 포인트: ${result.totalVolumeCredits}점`;


  function getPlayInfo (performances) {
    let result = [];
    performances.map((performance) => result.push(`${performance.playID}: ${format(performance.amount / 100)} (${performance.audience}석)\n`));
    return result;
  }

  function getPlayTotalAmount (performances) {
    return getPlayInfo(performances).reduce((playInfos, performance) => playInfos + performance);
  }

  function enrichPerformance(aPerformance) {
    const calculator = createPerformanceCalculator(aPerformance, playFor(aPerformance));
    const result = Object.assign({}, aPerformance);
    result.play = calculator.play;
    result.amount = calculator.amount;
    result.volumeCredits = calculator.volumeCredits;
    return result;
  }

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

  function totalAmount(data) {
    return data.performances.reduce((total, p) => total + p.amount, 0);
  }

  function totalVolumeCredits(data) {
    return data.performances.reduce((total, p) => total + p.volumeCredits, 0);
  }
}



function createPerformanceCalculator(aPerformance, aPlay) {
  switch(aPlay.type) {
    case "tragedy": return new TragedyCalculator(aPerformance, aPlay);
    case "comedy" : return new ComedyCalculator(aPerformance, aPlay);
    default: throw new Error(`알 수 없는 장르: ${aPlay.type}`);
  }
}

class performanceCalculator {
  constructor(aPerformance, aPlay) {
    this.performance = aPerformance;
    this.play = aPlay;
  }

  get amount() {
    throw new Error('서브클래스에서 처리하도록 설계되었습니다.');
  }

  get volumeCredits () {
    return Math.max(this.performance.audience - 30, 0);
  }
}

class TragedyCalculator extends performanceCalculator {
  get amount () {
    let result = 40000;
    if(this.performance.audience > 30) {
      result += 1000 * (this.performance.audience - 30);
    }
    return result;
  }
}

class ComedyCalculator extends performanceCalculator {
  get amount () {
    let result = 30000;
    if(this.performance.audience > 20) {
      result += 10000 + 500 * (this.performance.audience - 20);
    }
    result  += 300 * (this.performance.audience);
    return result;
  }

  get volumeCredits () {
    return super.volumeCredits + Math.floor(this.performance.audience / 5);
  }
}

const result = createStatementData(invoices, plays);

 

 

성능 부분에 대한 고민 보다는 함수를 잘게 추출하여 직관성을 높이는 방향의 리팩터링을 진행했다.  여전히 맘에 들지는 않지만 기존 코드 보다는 확실히 나아졌다.  로직이 단순한 코드여서 리팩터링에 큰 어려움은 없었다. 다만 자바스크립트 언어를 쓰면서 클래스를 쓰는 것이 익숙지 않아 책의 리팩터링 코드가 낯설게 느껴지는 것이 좀 슬픈 점..? 

현업에서 회사 코드를 리팩터링 하는 일은 매우 매우 어려운 것 같다. 얽혀있는 로직이 복잡하고 많아서, 그냥 돌아가게만 하는 것도 힘들기 때문이다. 작은 피쳐 기능의 코드들을 리팩터링 하는 것 부터 연습하려 한다.  리팩터링하면서 reduce 메서드를 많이 써보려고 했는데, 초기값이 없으면 자꾸 배열의 첫번째 요소를 가지고 오기 때문에, 가공된 배열이 아니면 가공된 문자열로 reduce를 하는게 어려웠다. 초기값을 가공한 요소로 하드코딩하는 것은 더 최악이었기 때문에 map을 쓸 수 밖에 없었다. javascript map 짱... 

리팩터링 천재가 되는 날까지 화이팅.... ❤️‍🔥