본문 바로가기

SW Engineering

Refactoring 예제

연극 해주고 돈 받는 회사

  • 관객 수, 연극 장르에 따라 요금이 다르다.
  • 현재 연극 종류는 비극과 희극 두 종류
  • 나중에 할인 해 주는 크레딧 제공

plays.json…

  {
    "hamlet": {"name": "Hamlet", "type": "tragedy"},
    "as-like": {"name": "As You Like It", "type": "comedy"},
    "othello": {"name": "Othello", "type": "tragedy"}
  }

The data for their bills also comes in a JSON file:

invoices.json…

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

The code that prints the bill is this simple function:

  function statement (invoice, plays) {
    let totalAmount = 0;
    let volumeCredits = 0;
    let result = `Statement for ${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(`unknown type: ${play.type}`);
      }
  
      // add volume credits
      volumeCredits += Math.max(perf.audience - 30, 0);
      // add extra credit for every ten comedy attendees
      if ("comedy" === play.type) volumeCredits += Math.floor(perf.audience / 5);
  
      // print line for this order
      result += `  ${play.name}: ${format(thisAmount/100)} (${perf.audience} seats)\n`;
      totalAmount += thisAmount;
    }
    result += `Amount owed is ${format(totalAmount/100)}\n`;
    result += `You earned ${volumeCredits} credits\n`;
    return result;
  }

Running that code on the test data files above results in the following output:

  Statement for BigCo
    Hamlet: $650.00 (55 seats)
    As You Like It: $580.00 (35 seats)
    Othello: $500.00 (40 seats)
  Amount owed is $1,730.00
  You earned 47 credits

 

  • 위 코드는 조금만 생각하면 쉽게 이해가능한 코드다. 코드가 짧아서 딱히 깊은 구조가 필요가 없다. 하지만 프로그램이 커질수록 위와 같은 하나의 함수로 쓰여진 코드뭉치는 이해하기 힘들어질 것이다.
  • 프로그램은 계속 변경된다. 요구사항이 변경되고 새로운 기능이 추가되면서 처음의 디자인은 점점 희미해진다. 구조 또한 계속 유지보수 필요
  • When you have to add a feature to a program but the code is not structured in a convenient way, first refactor the program to make it easy to add the feature, then add the feature.

The First Step in Refactoring

  • 리팩토링을 위해서는 테스트가 필수적이다.
  • 테스트는 반드시 self-checking이 가능해야 한다. (테스팅 프레임워크가 제공함)

Decomposing the statement Function

function statement (invoice, plays) {
  let totalAmount = 0;
  let volumeCredits = 0;
  let result = `Statement for ${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;

//-----------------------extract function------------------------------------
    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(`unknown type: ${play.type}`);
    }
//----------------------------------------------------------------------
      
    // add volume credits
    volumeCredits += Math.max(perf.audience - 30, 0);
    // add extra credit for every ten comedy attendees
    if ("comedy" === play.type) volumeCredits += Math.floor(perf.audience / 5);

    // print line for this order
    result += `  ${play.name}: ${format(thisAmount/100)} (${perf.audience} seats)\n`;
    totalAmount += thisAmount;
  }
  result += `Amount owed is ${format(totalAmount/100)}\n`;
  result += `You earned ${volumeCredits} credits\n`;
  return result;
}
  • 위 코드 덩어리를 이해하는데는 약간 시간이 걸린다.
  • 빠르고 쉽게 이해할 수 있는 코드를 만들기 위해서는 적절한 단위로 함수를 나누고 이름을 잘 지어줘야 한다.
  • 코드 덩어리를 자신의 함수로 바꿔보자. amountFor(aPerformance) 이런식으로
  • 이렇게 함수로 빼서 쉽게 참조되도록 하는 리팩토링 기법을 Extract Function

function statement…

  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(`unknown type: ${play.type}`);
    }
    return thisAmount;
  }

top level…

  function statement (invoice, plays) {
    let totalAmount = 0;
    let volumeCredits = 0;
    let result = `Statement for ${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);
  
      // add volume credits
      volumeCredits += Math.max(perf.audience - 30, 0);
      // add extra credit for every ten comedy attendees
      if ("comedy" === play.type) volumeCredits += Math.floor(perf.audience / 5);
  
      // print line for this order
      result += `  ${play.name}: ${format(thisAmount/100)} (${perf.audience} seats)\n`;
      totalAmount += thisAmount;
    }
    result += `Amount owed is ${format(totalAmount/100)}\n`;
    result += `You earned ${volumeCredits} credits\n`;
    return result;
  • 이렇게 변경을 하면, 즉시 컴파일 후 테스트를 수행하여 문제가 없는지 확인한다.
  • Refactoring changes the programs in small steps, so if you make a mistake, it is easy to find where the bug is.

function statement…

  function amountFor(aPerformance, play) {
    let result = 0;
    switch (play.type) {
    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(`unknown type: ${play.type}`);
    }
    return result;
  }
  • 변수의 의미를 명확하게 하기 위해서 thisAmount => result 로 변경
  • 타입이 파라미터명에 포함하도록 perf => aPerformance 로 변경
  • Any fool can write code that a computer can understand. Good programmers write code that humans can understand.

Removing the play Variable

  • Loop내에서 amountFor 함수 파라미터로 performance로 계산되는 play는 전달할 필요 없다.
  • play와 같은 임시 변수는 메소드 추출을 힘들게 하기 때문에 Replace Temp with Query로 리팩토링 ㄱㄱ

function statement…

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

top level…

  function statement (invoice, plays) {
    let totalAmount = 0;
    let volumeCredits = 0;
    let result = `Statement for ${invoice.customer}\n`;
    const format = new Intl.NumberFormat("en-US",
                          { style: "currency", currency: "USD",
                            minimumFractionDigits: 2 }).format;
    for (let perf of invoice.performances) {
        //---- repace temp with query ---- 
      const play = playFor(perf);
      let thisAmount = amountFor(perf, play);
  
      // add volume credits
      volumeCredits += Math.max(perf.audience - 30, 0);
      // add extra credit for every ten comedy attendees
      if ("comedy" === play.type) volumeCredits += Math.floor(perf.audience / 5);
  
      // print line for this order
      result += `  ${play.name}: ${format(thisAmount/100)} (${perf.audience} seats)\n`;
      totalAmount += thisAmount;
    }
    result += `Amount owed is ${format(totalAmount/100)}\n`;
    result += `You earned ${volumeCredits} credits\n`;
    return result;

 

  • 다시 Inline Variable로 리팩토링

top level…

  function statement (invoice, plays) {
    let totalAmount = 0;
    let volumeCredits = 0;
    let result = `Statement for ${invoice.customer}\n`;
    const format = new Intl.NumberFormat("en-US",
                          { style: "currency", currency: "USD",
                            minimumFractionDigits: 2 }).format;
  
    for (let perf of invoice.performances) {
      //----------  Inline Variable ------------------
      let thisAmount = amountFor(perf, playFor(perf));
  
      // add volume credits
      volumeCredits += Math.max(perf.audience - 30, 0);
      // add extra credit for every ten comedy attendees
      if ("comedy" === playFor(perf).type) volumeCredits += Math.floor(perf.audience / 5);
  
      // print line for this order
      result += `  ${playFor(perf).name}: ${format(thisAmount/100)} (${perf.audience} seats)\n`;
      totalAmount += thisAmount;
    }
    result += `Amount owed is ${format(totalAmount/100)}\n`;
    result += `You earned ${volumeCredits} credits\n`;
    return result;

 

  • amountFor 에 Change Function Declaration 적용 (play 파라미터를 제거)

top level…

  function statement (invoice, plays) {
    let totalAmount = 0;
    let volumeCredits = 0;
    let result = `Statement for ${invoice.customer}\n`;
    const format = new Intl.NumberFormat("en-US",
                          { style: "currency", currency: "USD",
                            minimumFractionDigits: 2 }).format;
    for (let perf of invoice.performances) {
      let thisAmount = amountFor(perf, playFor(perf));
  
      // add volume credits
      volumeCredits += Math.max(perf.audience - 30, 0);
      // add extra credit for every ten comedy attendees
      if ("comedy" === playFor(perf).type) volumeCredits += Math.floor(perf.audience / 5);
  
      // print line for this order
      result += `  ${playFor(perf).name}: ${format(thisAmount/100)} (${perf.audience} seats)\n`;
      totalAmount += thisAmount;
    }
    result += `Amount owed is ${format(totalAmount/100)}\n`;
    result += `You earned ${volumeCredits} credits\n`;
    return result;

function statement…

  function amountFor(aPerformance) {
    let result = 0;
    switch (playFor(aPerformance).type) {
    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(`unknown type: ${playFor(aPerformance).type}`);
    }
    return result;
  }

 

  • 로컬 변수를 제거하면 추출하기 훨씬 쉬워진다!
  • thisAmount 로컬 변수 제거 위해 Inline Variable사용하자

top level…

  function statement (invoice, plays) {
    let totalAmount = 0;
    let volumeCredits = 0;
    let result = `Statement for ${invoice.customer}\n`;
    const format = new Intl.NumberFormat("en-US",
                          { style: "currency", currency: "USD",
                            minimumFractionDigits: 2 }).format;
  
    for (let perf of invoice.performances) {
      
      // add volume credits
      volumeCredits += Math.max(perf.audience - 30, 0);
      // add extra credit for every ten comedy attendees
      if ("comedy" === playFor(perf).type) volumeCredits += Math.floor(perf.audience / 5);
  
      // print line for this order
      result += `  ${playFor(perf).name}: ${format(amountFor(perf)/100)} (${perf.audience} seats)\n`;
      totalAmount += amountFor(perf);
    }
    result += `Amount owed is ${format(totalAmount/100)}\n`;
    result += `You earned ${volumeCredits} credits\n`;
    return result;

 

Extracting Volume Credits

  • 앞에서play 로컬 변수를 제거해서 volume credit 계산 하는 부분을 추출하기 쉬워졌다.

top level…

  function statement (invoice, plays) {
    let totalAmount = 0;
    let volumeCredits = 0;
    let result = `Statement for ${invoice.customer}\n`;
    const format = new Intl.NumberFormat("en-US",
                          { style: "currency", currency: "USD",
                            minimumFractionDigits: 2 }).format;
    for (let perf of invoice.performances) {
      volumeCredits += volumeCreditsFor(perf);
  
      // print line for this order
      result += `  ${playFor(perf).name}: ${format(amountFor(perf)/100)} (${perf.audience} seats)\n`;
      totalAmount += amountFor(perf);
    }
    result += `Amount owed is ${format(totalAmount/100)}\n`;
    result += `You earned ${volumeCredits} credits\n`;
    return result;

 

  function volumeCreditsFor(aPerformance) {
    let result = 0;
    result += Math.max(aPerformance.audience - 30, 0);
    if ("comedy" === playFor(aPerformance).type) result += Math.floor(aPerformance.audience / 5);
    return result;
  }

 

Removing the format Variable

  • 계속해서 임시 변수를 제거하자. 이번에 format이다.

function statement…

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

top level…

  function statement (invoice, plays) {
    let totalAmount = 0;
    let volumeCredits = 0;
    let result = `Statement for ${invoice.customer}\n`;
      
    for (let perf of invoice.performances) {
      volumeCredits += volumeCreditsFor(perf);
  
      // print line for this order
      result += `  ${playFor(perf).name}: ${format(amountFor(perf)/100)} (${perf.audience} seats)\n`;
      totalAmount += amountFor(perf);
    }
    result += `Amount owed is ${format(totalAmount/100)}\n`;
    result += `You earned ${volumeCredits} credits\n`;
    return result;

 

  • format 이란 이름으로는 의미를 충분히 전달 못한다.
  • "formatAsUSD" 은 이렇게 좁은 범위에서 너무 길다. 화폐 단위를 강조하여 "usd" 로 바꾸자

Removing Total Volume Credits

  • 다음 타겟은volumeCredits 이다. 이놈은 루프안에 있으므로 Split Loop를 통해 분리시킨다.

top level…

  function statement (invoice, plays) {
    let totalAmount = 0;
    let volumeCredits = 0;
    let result = `Statement for ${invoice.customer}\n`;
    
    for (let perf of invoice.performances) {
  
      // print line for this order
      result += `  ${playFor(perf).name}: ${usd(amountFor(perf))} (${perf.audience} seats)\n`;
      totalAmount += amountFor(perf);
    }
    for (let perf of invoice.performances) {
      volumeCredits += volumeCreditsFor(perf);
    }
    
    result += `Amount owed is ${usd(totalAmount)}\n`;
    result += `You earned ${volumeCredits} credits\n`;
    return result;

 

  • 그 다음 Slide Statement를 통해 변수 선언 위치를 옮긴다.

top level…

  function statement (invoice, plays) {
    let totalAmount = 0;
    let result = `Statement for ${invoice.customer}\n`;
    for (let perf of invoice.performances) {
  
      // print line for this order
      result += `  ${playFor(perf).name}: ${usd(amountFor(perf))} (${perf.audience} seats)\n`;
      totalAmount += amountFor(perf);
    }
    let volumeCredits = 0;
    for (let perf of invoice.performances) {
      volumeCredits += volumeCreditsFor(perf);
    }
    result += `Amount owed is ${usd(totalAmount)}\n`;
    result += `You earned ${volumeCredits} credits\n`;
    return result;

 

  • volume credit 총합 구하는 부분을 Extract Function

function statement…

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

top level…

  function statement (invoice, plays) {
    let totalAmount = 0;
    let result = `Statement for ${invoice.customer}\n`;
    for (let perf of invoice.performances) {
  
      // print line for this order
      result += `  ${playFor(perf).name}: ${usd(amountFor(perf))} (${perf.audience} seats)\n`;
      totalAmount += amountFor(perf);
    }
    let volumeCredits = totalVolumeCredits();
    result += `Amount owed is ${usd(totalAmount)}\n`;
    result += `You earned ${volumeCredits} credits\n`;
    return result;

 

  • 다시 Inline Variable 적용

top level…

  function statement (invoice, plays) {
    let totalAmount = 0;
    let result = `Statement for ${invoice.customer}\n`;
    for (let perf of invoice.performances) {
  
      // print line for this order
      result += `  ${playFor(perf).name}: ${usd(amountFor(perf))} (${perf.audience} seats)\n`;
      totalAmount += amountFor(perf);
    }
   
    result += `Amount owed is ${usd(totalAmount)}\n`;
    result += `You earned ${totalVolumeCredits()} credits\n`;
    return result;

 

 

  • 이런식으로 계산한 값을 변수에 저장하여 사용하지 않고 사용될 때마다 함수가 호출된 계산하는 방법이 성능에 영향을 미칠것이라 생각 할 수 있다.

  • 일반적인 경우 성능에 대한 영향은 미미하다. 더 중요한 것은 클린하고 명확한 코드이다.

  • 여기서 4단계로 리팩토링을 진행했다.

  • 사실 마틴형도 항상 저렇게 야금야금 리팩토링 하는건 아니다. 하지만 테스트 실패 시 저렇게 하나씩 해보면 문제 찾기 쉬움


Status: Lots of Nested Functions

function statement (invoice, plays) {
    let result = `Statement for ${invoice.customer}\n`;
    for (let perf of invoice.performances) {
      result += `  ${playFor(perf).name}: ${usd(amountFor(perf))} (${perf.audience} seats)\n`;
    }
    result += `Amount owed is ${usd(totalAmount())}\n`;
    result += `You earned ${totalVolumeCredits()} credits\n`;
    return result;
  
    function totalAmount() {
      let result = 0;
      for (let perf of invoice.performances) {
        result += amountFor(perf);
      }
      return result;
    }
  
    function totalVolumeCredits() {
      let result = 0;
      for (let perf of invoice.performances) {
        result += volumeCreditsFor(perf);
      }
      return result;
    }
    function usd(aNumber) {
      return new Intl.NumberFormat("en-US",
                          { style: "currency", currency: "USD",
                            minimumFractionDigits: 2 }).format(aNumber/100);
    }
    function volumeCreditsFor(aPerformance) {
      let result = 0;
      result += Math.max(aPerformance.audience - 30, 0);
      if ("comedy" === playFor(aPerformance).type) result += Math.floor(aPerformance.audience / 5);
      return result;
    }
    function playFor(aPerformance) {
      return plays[aPerformance.playID];
    }
    function amountFor(aPerformance) {
      let result = 0;
      switch (playFor(aPerformance).type) {
      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(`unknown type: ${playFor(aPerformance).type}`);
      }
      return result;
    }
  }
  • 코드 구조가 좋아졌다.
  • top-level statement function은 statement를 출력하는 7줄 밖에 없다.
  • 전체적인 흐름 뿐만 아니라 각각의 계산이 무엇을 의미하는지 이해하기 쉽다.

Splitting the Phases of Calculation and Formatting

  • statement를 text뿐만 아니라 HTML로 출력해야할 필요가 생겼다.
  • 기존에 구현한 text 내용을 바탕으로 복붙하여 HTML을 구현하면 중복 생긴다.
  • Extract Function을 적용하여 Split Phase를 해보자.

top level…

  function statement (invoice, plays) {
    const statementData = {};
    statementData.customer = invoice.customer;
    statementData.performances = invoice.performances;
    return renderPlainText(statementData, plays);
  }

 

  function renderPlainText(data, plays) {
    let result = `Statement for ${data.customer}\n`;
    for (let perf of data.performances) {
      result += `  ${playFor(perf).name}: ${usd(amountFor(perf))} (${perf.audience} seats)\n`;
    }
    result += `Amount owed is ${usd(totalAmount())}\n`;
    result += `You earned ${totalVolumeCredits()} credits\n`;
    return result;

 

 

 

Status: Separated into Two Files (and Phases)

statement.js

  import createStatementData from './createStatementData.js';
  function statement (invoice, plays) {
    return renderPlainText(createStatementData(invoice, plays));
  }
  function renderPlainText(data) {
    let result = `Statement for ${data.customer}\n`;
    for (let perf of data.performances) {
      result += `  ${perf.play.name}: ${usd(perf.amount)} (${perf.audience} seats)\n`;
    }
    result += `Amount owed is ${usd(data.totalAmount)}\n`;
    result += `You earned ${data.totalVolumeCredits} credits\n`;
    return result;
  }
  function htmlStatement (invoice, plays) {
    return renderHtml(createStatementData(invoice, plays));
  }
  function renderHtml (data) {
    let result = `<h1>Statement for ${data.customer}</h1>\n`;
    result += "<table>\n";
    result += "<tr><th>play</th><th>seats</th><th>cost</th></tr>";
    for (let perf of data.performances) {
      result += `  <tr><td>${perf.play.name}</td><td>${perf.audience}</td>`;
      result += `<td>${usd(perf.amount)}</td></tr>\n`;
    }
    result += "</table>\n";
    result += `<p>Amount owed is <em>${usd(data.totalAmount)}</em></p>\n`;
    result += `<p>You earned <em>${data.totalVolumeCredits}</em> credits</p>\n`;
    return result;
  }
  function usd(aNumber) {
    return new Intl.NumberFormat("en-US",
                                 { style: "currency", currency: "USD",
                                   minimumFractionDigits: 2 }).format(aNumber/100);
  }

createStatementData.js

  export default function createStatementData(invoice, plays) {
    const result = {};
    result.customer = invoice.customer;
    result.performances = invoice.performances.map(enrichPerformance);
    result.totalAmount = totalAmount(result);
    result.totalVolumeCredits = totalVolumeCredits(result);
    return result;
  
    function enrichPerformance(aPerformance) {
      const result = Object.assign({}, aPerformance);
      result.play = playFor(result);
      result.amount = amountFor(result);
      result.volumeCredits = volumeCreditsFor(result);
      return result;
    }
    function playFor(aPerformance) {
      return plays[aPerformance.playID] 
    }
    function amountFor(aPerformance) {
      let result = 0;
      switch (aPerformance.play.type) {
      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(`unknown type: ${aPerformance.play.type}`);
      }
      return result;
    }
    function volumeCreditsFor(aPerformance) {
      let result = 0;
      result += Math.max(aPerformance.audience - 30, 0);
      if ("comedy" === aPerformance.play.type) result += Math.floor(aPerformance.audience / 5);
      return result;
    }
    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);
    }

 

  • 모듈화되어 코드 부분을 이해하기 쉬워졌다.
  • 계산 코드의 중복 없이 HTML 지원이 가능해졌다.
  • Brevity is the soul of wit, but clarity is the soul of evolvable software.
  • When programming, follow the camping rule: Always leave the code base healthier than when you found it.

Reorganizing the Calculations by Type

  • 연극의 종류가 더 생길 수 있다. 각 연극 종류는 자신만의 요금과 크레딧 계산이 존재한다.
  • 상속을 통해 계산로직을 가지는 희극과 비극 subclass를 만들자
  • 핵심은 Conditional Code를 다형성으로 변환하는 Replace Conditional with Polymorphism 적용

createStatementData.js…

  export default function createStatementData(invoice, plays) {
    const result = {};
    result.customer = invoice.customer;
    result.performances = invoice.performances.map(enrichPerformance);
    result.totalAmount = totalAmount(result);
    result.totalVolumeCredits = totalVolumeCredits(result);
    return result;
  
    function enrichPerformance(aPerformance) {
      const result = Object.assign({}, aPerformance);
      result.play = playFor(result);
      result.amount = amountFor(result);
      result.volumeCredits = volumeCreditsFor(result);
      return result;
    }
    function playFor(aPerformance) {
      return plays[aPerformance.playID] 
    }
    function amountFor(aPerformance) {
      let result = 0;
      switch (aPerformance.play.type) {
      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(`unknown type: ${aPerformance.play.type}`);
      }
      return result;
    }
    function volumeCreditsFor(aPerformance) {
      let result = 0;
      result += Math.max(aPerformance.audience - 30, 0);
      if ("comedy" === aPerformance.play.type) result += Math.floor(aPerformance.audience / 5);
      return result;
    }
    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);
    }

 

  • performance에 관해서 intermediate data를 채우는 enrichPerformance 가 핵심이다.
  • performance 계산을 하는 Performance Calculator를 만든다.

function createStatementData…

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

class PerformanceCalculator…

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

 

  • 계산 로직을 calculator로 옮긴다. Move Function 리팩토링

class PerformanceCalculator…

  get amount() {
    let result = 0;
    switch (this.play.type) {
      case "tragedy":
        result = 40000;
        if (this.performance.audience > 30) {
          result += 1000 * (this.performance.audience - 30);
        }
        break;
      case "comedy":
        result = 30000;
        if (this.performance.audience > 20) {
          result += 10000 + 500 * (this.performance.audience - 20);
        }
        result += 300 * this.performance.audience;
        break;
      default:
        throw new Error(`unknown type: ${this.play.type}`);
    }
    return result;
  }

 

Making the Performance Calculator Polymorphic

  • 적절한 Calculator의 subclass를 얻기 위해서 생성자 대신에 subclass를 리턴하는 함수를 만든다.

    Replace Constructor with Factory Function 적용

function createStatementData…

  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;
  }

top level…

  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(`unknown type: ${aPlay.type}`);
      }
  }
  class TragedyCalculator extends PerformanceCalculator {
  }
  class ComedyCalculator extends PerformanceCalculator {
  }

 

Status: Creating the Data with the Polymorphic Calculator

createStatementData.js

  export default function createStatementData(invoice, plays) {
    const result = {};
    result.customer = invoice.customer;
    result.performances = invoice.performances.map(enrichPerformance);
    result.totalAmount = totalAmount(result);
    result.totalVolumeCredits = totalVolumeCredits(result);
    return result;
  
    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(`unknown type: ${aPlay.type}`);
      }
  }
  class PerformanceCalculator {
    constructor(aPerformance, aPlay) {
      this.performance = aPerformance;
      this.play = aPlay;
    }
    get amount() {
      throw new Error('subclass responsibility');
    }
    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);
    }
  }

 

  • 새로운 종류의 연극이 추가되도 subclass를 추가하여 유연하게 확장 가능

Final Thoughts

  • 명확한 코드는 쉽게 이해할 수 있게 하고, 이는 더 깊은 통찰력 가지게 하므로 긍정적인 피드백의 선순환을 만든다.
  • The true test of good code is how easy it is to change it.
  • 효율적인 리팩토링의 핵심은 작은 단계로 수행할 때 더 빠르게 진행되고 코드가 망가지지 않으며, 그 단계들이 실질적인 변경이 될 수 있다는 것이다.

'SW Engineering' 카테고리의 다른 글

Refactoring - Encapsulation  (0) 2019.12.07
Shotgun surgery vs. Divergent Change  (0) 2019.09.08