본문 바로가기
javascript

리팩토링 해보자 with JS - 1

by 윤-찬미 2021. 12. 1.

공연료를 청구하는 시스템을 만든다고 해보자.

공연료를 청구하는 시스템 코드를 리팩토링 해보자

시스템의 코드는 아래와 같다.

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.platId];
    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 + 50 * (perf.audience - 20);
        }
        thisAmount += 300 * perf.audience;
        break;
      default:
        throw new Error(`알 수 없는 장르: ${play.type}`);
    }
    volumeCredits += Math.max(perf.audience - 30, 0);
    if ("comedy" === play.type) volumeCredits += Math.floor(perf.audience / 5);

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

위 코드를 보고 리팩토링을 할때 어떤 것 부터 봐야할지 감이 안잡힌다면, 아래 글의 순서 대로 차근 차근 리팩토링을 진행해보자.

테스트 코드 짜기

리팩토링할때 발생할 수 있는 실수를 수동으로 확인하는 것이 아닌,

테스트 코드를 짜고 시작하면, 리팩토링 후 코드의 정상 동작이 '보장' 된다.

함수 쪼개기

우선 코드를 읽으면서 로직별로 살펴보자.

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 + 50 * (perf.audience - 20);
      }
      thisAmount += 300 * perf.audience;
      break;
    default:
      throw new Error(`알 수 없는 장르: ${play.type}`);
}

위 코드가 무엇을 하는지 말해보자!한번의 공연에 대한 요금을 계산하고 있음을 알 수 있다.

워드 커닝햄이 말하길,
이런 식으로 파악한 정보는 휘발성이 높기로 악명높은 저장장치인 내 머릿속에
기록 되므로, 잊지 않으려면 제빨리 코드에 반영해야 한다. 그러면 다음번에 코드를 볼때, 다시 분석하지
않아도 코드 스스로가 자신이 하는일이 무엇인지 이야기 해줄 것 이다.

이름은 amountFor.js별도 함수로 꺼내면

perf, play의 변수 값은 유효범위를 벗어나는,

즉 새 함수에서 곧바로 사용할 수 없는 함수 이므로, 매개변수로 전달할 수 있도록한다.

단,thisAmount는 함수 내부에서 값의 변화를 일으킴으로 내부 변수로 두었다.

(이제 이부분에서 어? 할 수 있는데, 우선 함수추출하는 것에 집중하자)

자 그럼 이제 statement 함수는 아래와 같이 코드를 작성할 수 있다.

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.platId];
    let thisAmount = amountFor(perf, play);
    volumeCredits += Math.max(perf.audience - 30, 0);
    if ("comedy" === play.type) volumeCredits += Math.floor(perf.audience / 5);

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

함수를 추출 했다면, 이제 코드를 조금 더 명확하게 표현할 수 있는 방법을 진행해보자.

변수명 조금 더 명확하게 짓기

amountFor.js

-> thisAmount를 result로 변경해보자.

function amountFor(perf, play) {
  let result = 0; // thisAmount -> result 로 변경
  switch (play.type) {
    case "tragedy":
      result = 40000;
      if (perf.audience > 30) {
        result += 1000 * (perf.audience - 30);
      }
      break;
    case "comedy": 
      result = 30000;
      if (perf.audience > 20) {
        result += 10000 + 50 * (perf.audience - 20);
      }
      result += 300 * perf.audience;
      break;
    default:
      throw new Error(`알 수 없는 장르: ${play.type}`);
    }
  return result;
}

-> perf를 aPerformance로 변경해보자.

function amountFor(aPerformance, play) { // perf -> aPerformance
    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 + 50 * (aPerformance.audience - 20);
      }
      result += 300 * aPerformance.audience;
      break;
    default:
      throw new Error(`알 수 없는 장르: ${play.type}`);
    }
    return result;
}

변수 제거하기

매개변수가 어디서 오는지 알아보기
-> play 의 출처를 찾아서..

function statement(invoice, plays) {
  // ...

  for (let perf of invoice.performances) {
    const play = plays[perf.platId]; // <- perf에서 정보를 얻어옴
    let thisAmount = amountFor(perf, play);
    volumeCredits += Math.max(perf.audience - 30, 0);
    if ("comedy" === play.type) volumeCredits += Math.floor(perf.audience / 5);

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

play의 출처를 보니 우리가 매개변수에서 받은 perf로 알 수 있는 정보다.

"임시변수를 질의 함수로 바꾸기", "변수를 인라인 하기" 과정을 통해 리팩토링 해보자


function statement(invoice, plays) {
  // ...
    function amountFor(aPerformance) {
        let result = 0;
      switch (playFor(perf).type) { // play -> playFor(perf)
        case "tragedy":
          result = 40000;
          if (aPerformance.audience > 30) {
            result += 1000 * (aPerformance.audience - 30);
          }
          break;
        case "comedy": 
          result = 30000;
          if (aPerformance.audience > 20) {
            result += 10000 + 50 * (aPerformance.audience - 20);
          }
          result += 300 * aPerformance.audience;
          break;
        default:
          throw new Error(`알 수 없는 장르: ${playFor(perf).type}`); // play -> playFor(perf)
        }
        return thisAmount;
    }
    function playFor(aPerformance) {
        return plays[aPerformance.payID];
    }
  // playFor(perf)를 통해 play값 구할 수 있음
  for (let perf of invoice.performances) {
    let thisAmount = amountFor(perf);
    volumeCredits += Math.max(perf.audience - 30, 0);
    if ("comedy" === playFor(perf).type) volumeCredits += Math.floor(perf.audience / 5);

    result += `${playFor(perf).name}: ${format(totalAmount/100)} \n`;
    totalAmount += thisAmount
  }
  // ...
}

🤭 이렇게 코드를 짜니 분명 명확해진 느낌은 나는데.. 기존 루프를 한 번 돌 때마다 공연을 조회 했었는데,

이제 세 번이나 조회함. but 이정도 성능은 감안하더라도 일단, 코드를 보기 쉽게 만들자.

그럼 후에 성능 개선은 별게 아닐 수도 있다.

자 이제 다시 위 코드를 보고 변수를 인라인 할게 없는지 생각해보자.

thisAmount가 눈에 띄지 않나?

let thisAmount = amountFor(perf); // thisAmount를 인라인해보자
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) {
    // 포인트 적립
    volumeCredits += Math.max(perf.audience - 30, 0);
    if ("comedy" === playFor(perf.type) {
              // 희극 관계 5명 마다 추가 포인트 제공
                volumeCredits += Math.floor(perf.audience / 5);
        }
    result += `${playFor(perf).name}: ${format(amountFor(perf)/100)} \n`; // thisAmount -> amountFor(perf)
    totalAmount += amountFor(perf); // thisAmount -> amountFor(perf)
  }
  result += `총액: ${format(totalAmount/100)}\n`;
  result += `적립 포인트 : ${volumeCredits}점 \n`;
  return result;
}

현재까지의 결과물

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) {
    // 포인트 적립
    volumeCredits += Math.max(perf.audience - 30, 0);
    if ("comedy" === playFor(perf.type) {
                volumeCredits += Math.floor(perf.audience / 5);
        }
    result += `${playFor(perf).name}: ${format(amountFor(perf)/100)} \n`;
    totalAmount += amountFor(perf);
  }
  result += `총액: ${format(totalAmount/100)}\n`;
  result += `적립 포인트 : ${volumeCredits}점 \n`;
  return result;
}

중간 정리를 해보면 우리가 한 리팩토링은 아래와 같다

🐥 리팩토링하면서 버그가 생기지 않도록 테스트 코드를 먼저 짜놓는다.

🐥 한 함수에 몰려 있는 여러 로직을 분리시킨다.

🐥 변수명을 정확하게 짓자

🐥 변수를 제거하자 (변수를 인라인 하자) → 여기서 제거 하자 라는 말은 "너무 많은" 선언과 변수들 그리고 인자들은 코드를 오히려 복잡하게 만들 수도 있다는 말

자 마저 으쌰으쌰 작업을 해보자!

이번엔 아래 포인트 적립하면서 희극 5명 마다 추가 포인트 제공하는 부분을 함수로 추출해보자

기존코드

function statement(invoice, plays) {
  let totalAmount = 0;
  let volumeCredits = 0;
  // ...
  for (let perf of invoice.performances) {
    // 포인트 적립
    volumeCredits += Math.max(perf.audience - 30, 0);
        // 희극 5명 마다 추가 포인트 제공
    if ("comedy" === playFor(perf.type) {
                volumeCredits += Math.floor(perf.audience / 5);
        }
    result += `${playFor(perf).name}: ${format(amountFor(perf)/100)} \n`;
    totalAmount += amountFor(perf);
  }
  // ...
}

개선코드

function statement(invoice, plays) {
  // ...
    function volumeCreditsFor(aPerformance) {
        let result = 0;
        result += Math.max(perf.audience - 30, 0);
        // 희극 5명 마다 추가 포인트 제공
      if ("comedy" === playFor(aPerformance.type)) {
            result += Math.floor(aPerformance.audience / 5);
        }
        return result;
    }
  let totalAmount = 0;
  let volumeCredits = 0;
  // ...
  for (let perf of invoice.performances) {
    // 추출한 함수를 통해 값 누적
    volumeCredits += volumeCreditsFor(perf);
    result += `${playFor(perf).name}: ${format(amountFor(perf)/100)} \n`;
    totalAmount += amountFor(perf);
  }
  // ...
}

임시변수 줄이기

→ 임시 변수는 자신이 속한 루틴 에서만 의미가 있어서 루틴이 길고 복잡해 질 수 있다.

  1. format 함수 변수를 일반 함수로 변경해보자!
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;

  // ...
}

근데 format 이란 변수는 의미가 너무 장황하다.

이 함수가 하는 일은 화폐 단위를 맞추어 주는 역할을 한다.

따라서 usd정도가 적당할 수 있을 것 같다.

또, format을 사용할때마다 /100을 해줬는데, 해당 로직도 합쳐 주면 좋을 것 같다.

개선한 코드는 아래와 같다.


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

function statement(invoice, plays) {
  let totalAmount = 0;
  let volumeCredits = 0;
  let result = `청구내역 (고객명: ${invoice.customer})\n`;
  for (let perf of invoice.performances) {
    volumeCredits += volumeCreditsFor(perf);
    result += `${playFor(perf).name}: ${usd(amountFor(perf))} \n`; //format(amountFor(perf)/100) -> usd(amountFor(perf))
    totalAmount += amountFor(perf);
  }
  result += `총액: ${usd(totalAmount)}\n`; // format(totalAmount/100) -> usd(totalAmount)
  result += `적립 포인트 : ${volumeCredits}점 \n`;
  return result;
}
  1. volumeCredits 변수 제거하기

volumeCredits은 반복문 안에 있어서 제거하기가 까다롭다.

아래와 같은 순서로 차근 차근 분리해서 제거 하자!

🚀 반복문 쪼개기

function statement(invoice, plays) {
  let totalAmount = 0;
  let volumeCredits = 0;
  let result = `청구내역 (고객명: ${invoice.customer})\n`;
    // 아래와 같이 두개로 쪼개었다.
  for (let perf of invoice.performances) {
    result += `${playFor(perf).name}: ${usd(amountFor(perf))} \n`;
    totalAmount += amountFor(perf);
  }
  for (let perf of invoice.performances) {
    volumeCredits += volumeCreditsFor(perf);
  }
  result += `총액: ${usd(totalAmount)}\n`;
  result += `적립 포인트 : ${volumeCredits}점 \n`;
  return result;
}

🚀 문장 슬라이스 하기

volumeCredits 변수를 사용하는 곳 바로 앞으로 옮긴다.

function statement(invoice, plays) {
  let totalAmount = 0;
  let result = `청구내역 (고객명: ${invoice.customer})\n`;
  for (let perf of invoice.performances) {
    result += `${playFor(perf).name}: ${usd(amountFor(perf))} \n`;
    totalAmount += amountFor(perf);
  }
// volumeCredits 변수를 사용하는 곳 바로 앞으로 옮긴다.
  let volumeCredits = 0;
  for (let perf of invoice.performances) {
    volumeCredits += volumeCreditsFor(perf);
  }
  result += `총액: ${usd(totalAmount)}\n`;
  result += `적립 포인트 : ${volumeCredits}점 \n`;
  return result;
}

🚀 함수로 추출하기, 변수 인라인 하기

totalVolumeCredits 함수로 추출하고 나니, volumeCredits이라는 임시변수를 제거할 수 있게 되었다.

→ volumeCredits -> totalVolumeCredits()

function statement(invoice, plays) {
    // ...
    function totalVolumeCredits() {
      let result = 0;
      for (let perf of invoice.performances) {
        result += volumeCreditsFor(perf);
      }
      return result;
    }
  let totalAmount = 0;
  let result = `청구내역 (고객명: ${invoice.customer})\n`;
  for (let perf of invoice.performances) {
    result += `${playFor(perf).name}: ${usd(amountFor(perf))} \n`;
    totalAmount += amountFor(perf);
  }
  result += `총액: ${usd(totalAmount)}\n`;
  result += `적립 포인트 : ${totalVolumeCredits()}점 \n`; //volumeCredits -> totalVolumeCredits()
  return result;
}
  1. totalAmount 변수를 인라인 해보자!
  2. 아까와 똑같은 방법으로 진행한다.

function statement(invoice, plays) {
    // ...
    function totalAmount() {
      let result = 0;
      for (let perf of invoice.performances) {
        result += amountFor(perf);
      }
    }
  let result = `청구내역 (고객명: ${invoice.customer})\n`;
  for (let perf of invoice.performances) {
    result += `${playFor(perf).name}: ${usd(amountFor(perf))} \n`;
  }
  result += `총액: ${usd(totalAmount())}\n`;
  result += `적립 포인트 : ${totalVolumeCredits()}점 \n`; //volumeCredits -> totalVolumeCredits()
  return result;
}

지금 우리가 한 임시변수 제거 하는 방법은 아래와 같다.

🚀 반복문 쪼개기

🚀 문장 슬라이스 하기

🚀 함수로 추출하기

🚀 변수 인라인 하기

🐥 리팩토링 할 때 단계를 잘게 나누어 커밋 하고 테스트하면 테스트가 실패했을때 더 빠르게 파악이 가능하다!

최종 코드는 아래와 같다.

코드를 살펴 보면, 코드의 구조를 보강하여, 최종적인 statement의 코드는 단 7줄로 끝났다.

function statement(invoice, plays) {
  function totalAmount() {
    let result = 0;
    for (let perf of invoice.performances) {
      result += amountFor(perf);
    }
  } 
  function amountFor(aPerformance) {
    let result = 0;
    switch (playFor(perf).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 + 50 * (aPerformance.audience - 20);
        }
        result += 300 * aPerformance.audience;
        break;
      default:
        throw new Error(`알 수 없는 장르: ${playFor(perf).type}`);
    }
    return thisAmount;
  }

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

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

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

    // 🌈statement 로직의 코드는 오직 일곱줄!
  let result = `청구내역 (고객명: ${invoice.customer})\n`;
  for (let perf of invoice.performances) {
    result += `${playFor(perf).name}: ${usd(amountFor(perf))} \n`;
  }
  result += `총액: ${usd(totalAmount)}\n`;
  result += `적립 포인트 : ${totalVolumeCredits()}점 \n`;
  return result;
}

총 내용을 정리하면 아래와 같다.

  1. 테스트 코드짜기
  2. 리팩토링을 진행하다, 기능에 버그는 없는지 확인하기 위해 테스트 코드를 짠다.
  3. 함수로 쪼개기
  4. 여러 로직이 한곳에 몰려 있기보다, 각 기능에 맞추어 로직을 함수로 쪼갠다.
  5. 이름 잘 짓기
  6. 변수명을 잘 지어야 해당 코드의 의도를 파악하기 쉬워진다.
  7. 변수 제거 하기임시변수들은 특히 자신이 속한 루틴 에서만 의미가 있어서 루틴이 길고 복잡해 질 수 있다.🚀 반복문 쪼개기🚀 함수로 추출하기🐥 리팩토링 할 때 단계를 잘게 나누어 커밋 하고 테스트하면 테스트가 실패했을때 더 빠르게 파악이 가능하다!
  8. 🚀 변수 인라인 하기
  9. 🚀 문장 슬라이스 하기
  10. 임시변수는 아래와 같은 단계로 쪼개 분리할 수 있다.
  11. 여기서 제거 하자 라는 말은 "너무 많은" 선언과 변수들 그리고 인자들은 코드를 오히려 복잡하게 만들 수도 있다는 말

마무리

다음 포스팅 때는 "계산 단계와 포멧팅 단계를 분리" 하는 리팩토링을 해보겠다!

이번 파트를 읽고 정리하며, 내가 기존에 짜놓았던 코드의 복잡도를 줄일 수 있었다.

특히 변수 제거 하기에서 반복문에 포함되어있는 변수들은 줄이는게 너무 어려웠는데,

이번 파트를 통해 많은 부분을 깨달았다. 아직도 많이 어렵지만, 더 노력해야지.

성능이 나빠질까 미뤄뒀던 리팩토링들은 오히려 하고 나니 성능을 어떻게 개선하면 좋을지 더 잘 보였다.

저작자표시

'javascript' 카테고리의 다른 글

V8은 JS를 컴파일 할 때 Interpreter 의 느린 실행 속도를 어떻게 개선 했을까?  (0) 2022.01.12
추석하루 간단하게 본 NEST JS  (0) 2021.09.22
똑똑하게 외부코드 사용 고고  (1) 2021.08.07
lodash 유용하게쓰기 (응용해서 쓰기)  (0) 2021.06.25
롤업과 웹팩의 차이점 (rollup vs webpack)  (2) 2021.04.22

관련글

티스토리툴바