리팩터링2 1장 리뷰

배경

리팩터링2라는 책을 예전부터 알고 있었다. 개발바닥의 이동욱님을 통해서 알게되었고 꼭 스터디를 열어서 공부해야겠다고 생각했다. (클린코드, 함께 자라기, 객체지향의 사실과 오해, 오브젝트 등도 기회가 있다면 하고싶다.) 친구랑 우연히 얘기하다가 둘다 이 책으로 스터디를 하고 싶어한다는 것을 알게 되었고 스터디를 시작하게 되었다. 그리고 회사에서도 이 얘기를 하자 같이 하고 싶다고 한 두분이 있었고 회사 동료분의 친구까지 합쳐서 5명이 스터디를 하게 되었다.

 

이 책을 읽는 이유

사실 할 공부도 책도 너무 많다. 그 많은 책중에서 리팩터링이라는 책을 선택했다. 그 이유는 모두가 리팩터링이라는 것이 꼭 필요하다고 생각하기 때문이다. 특히 회사를 다니면 엄청 공감하게 된다. 작은 프로젝트에서는 그냥 하면 되지만 코드규모도 커지고 회사가 바쁘게 돌아가면 리팩터링할 여유가 없다. 그로인해서 발생하는 사이드이펙트도 문제고 말이다. 이런 상황에서 어떻게 리팩터링을 해야할지 잘 모르겠다. 물론 지금도 조금씩 하고 있지만 최소한 책 한권정도는 보는게 좋다고 생각했다. 

 

1장 정리

1장은 실제 리팩터링 하는 모습을 보여준다. 

export function statement(invoice: Invoice, plays: 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;
}

 

얼핏봐도 리팩터링할부분이 많이 보인다. 근데 사실 이정도 코드는 실무에서 자주 보게되는 코드일것같다. 잠깐만 이성을 놓고 손이 가는데로 코딩하면 이런 코드가 만들어지게 된다. 이 코드를 리팩터링하면서 여러가지 기법들을 소개한다. 전체적인 맥락에서 정리한 내용을 가져와봤다.

 

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

사실 테스트코드없는 리팩터링은 위험성이 너무나도 크다. 단, 프론트에서는 약간 논란이 있을 수도 있지만... 프론트에서도 최소한 코어적인 테스트 코드는 필요하다. 그리고 백엔드는 무조건 필수적이다. db 마이그레이션을 하는데 테스트코드가 없으면 도저히 결과를 예측할 수 없을것이다. 클린코드는 아마 커버리지100이 필수라고 한다. 우리는 100은 아니더라도 조금씩 테스트코드를 추가하면서 리팩터링을 해야만한다.

- 함수 쪼개기(함수 추출하기)

사실 가장 기본이 되는 내용이다. 함수 나눠라. 함수 나눠라. 하는 얘기를 엄청 많이 들었다. 역할 책임을 분리해서 함수를 쪼개는건 기본이지만 가장 중요한 작업이다.

- 수정은 작은 단계로 나눠 진행

한번에 너무 많이 수정해버리면 어디서부터 잘못했는지 알 수가 없다. 수정, 테스트, 커밋을 아주 작은 단계로 나누어서 진행해야만한다. 이 간단한 예제를 리팩터링하면서도 많이 느꼈다. 어느순간 잘못됬다고 느끼면 그냥 커밋을 버리고 이전 커밋으로 돌아가면 차라리 훨씬 편하다.

- 변수명 명확히 바꾸기

변수명은 엄청 중요하다. 동시에 엄청 어렵다. 과제리뷰를 받을때 변수명으로 지적을 받은적도 있었다. 왜 중요할까?? 그 이유는 변수명으로 함축된 정보를 파악해야하기 때문이다. 변수명이 명확하지 않으면 모든 코드를 읽어야한다. 함수를 쪼개는 의미가 절반으로 줄어든다.

- 임시 변수 제거하기(임시 변수를 질의 함수로 바꾸기)

이건 사실 처음들었다. 보통 객체 내부의 값에 접근할 때 지역변수를 만들고 사용하는데 질의 함수로 바꾸라고 했다. 이게 경우에 따라 좋을수도 있지만 항상 옳은건가? 라는 생각이 든다. 

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

  > 컴퓨터가 이해하는 코드는 바보도 작성할 수 있다. 사람이 이해하도록 작성하는 프로그래머가 진정한 실력자다. - 캔트 백

- 리팩터링으로 인한 성능 문제는 무시하자. 만약 성능이 크게 문제가 있다면 리팩터링을 끝내가 난 뒤에 성능을 개선하자

매우 동의한다. 특히 프론트에서는 정말 성능이 필요한 일부 로직을 제외하고는 가독성이 더 중요하다. 단, 성능을 극한으로 끌어올리기 위해서 코드가 조금 지저분해지는 경우도 있다. 결국 중간을 잘 찾아야한다. 그래도 일단은 리팩터링할때만큼은 성능을 조금 내려놓자.

- 단계 쪼개기: 데이터, ui(텍스트, html)로 분리.(중간 구조체) > 코드스피츠의 테트리스 강의와 유사

이것과 똑같은 얘기를 테트리스 강의를 들으면서 들었었다. 결국 도메인의 분리다. 도메인적인 코드와 네이티브적인 코드가 분리되면 유지보수가 쉬워진다.

- 프로그래밍에서는 간결함x, 명료함o

코드가 짧다고 무조건 좋은 코드일까?? 그렇지 않다. 짧은 코드와 가독성 좋은 코드를 놓고 봤을 때 조금 길더라도 가독성 좋은 코드가 훨씬 좋은 코드다. 즉 명료함이 중요하다. 코드의 수를 줄일려고 명료함을 해치는 행위를 해서는 안된다.

  > 항시 코드베이스를 작업 시잔 전보다 건강하게 만들어놓고 떠나야한다.

- 리팩터링과 기능 추가 사이의 균형을 맞추자.

이게 진짜 어렵다. 리팩터링만 잡고 있을 수는 없다. 기능 추가가 사실 더 우선적이기는 하다. 리팩터링을 하는 이유는 기능 추가하는데 걸리는 시간을 줄이기 위해서다. 그런데 기능 추가를 해야될때 하지 못한다면 의미가 없다. 이 중간지점을 잘 잡아야한다. 명확히 어떤 기준은 없다. 예전에 우테캠할때 멘토님들한테 들었던 말은 `일단 가장 중요한 기능을 먼저 만들고 그 이후에 코드 퀄리티를 신경써라. 그 이후에 적절히 기능과 코드 퀄리티를 신경쓰면 된다` 였다. 이 말에 동감하는데 회사에서 이렇게 하기는 쉽지 않다. 왜냐하면 우리는 개발자끼리 일하지 않는다. 이 균형을 비개발자와 얘기해서 조절해야한다. 2장을 슬쩍봤는데 리팩터링할때 관리자에게 알리지 말라는 말도 있다. ㅎㅎㅎ

- 리팩토링할때는 정말 간단한 것만 바꿔도 `테스트 => 커밋`을 하는 습관을 들이자(git hook도 괜찮은 선택)

위에서 동일한 설명을 했다.

- 조건부 로직을 다형성으로 바꾸기

다형성(polymorphism)이란 하나의 객체가 여러 가지 타입을 가질 수 있는 것을 의미합니다. 

http://www.tcpschool.com/java/java_polymorphism_concept

 

/*
변경점
핵심 : 조건부 로직을 다형서으로 바꾸기
1. 공연료 계산기 만들기(PerformanceCalculator 클래스)
2. 클래스의 생성자에 함수 선언 바꾸기 적용(aPlay)
3. 함수 옮기기를 통해 함수를 클래스로 이전(amount, volumeCredits)
공연료 계산기를 다형성 버전으로 만들기
4. 생성자를 팩터리 함수로 바꾸기
5. 조건부 로직을 다형성으로 바꾸기(amount, volumeCredits)
*/

abstract class PerformanceCalculator {
  performance: Performance;
  play: Play;

  constructor(aPerformance: Performance, aPlay: Play) {
    this.performance = aPerformance;
    this.play = aPlay;
  }

  abstract get amount(): number;

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

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

export default function createStatementData(
  invoice: Invoice,
  plays: Plays
): StatementData {
  const enrichPerformances = invoice.performances.map(enrichPerformance);
  return {
    customer: invoice.customer,
    performances: enrichPerformances,
    totalAmount: totalAmount(enrichPerformances),
    totalVolumeCredits: totalVolumeCredits(enrichPerformances),
  };

  function enrichPerformance(aPerformance: Performance): EnrichPerformance {
    const calculator = createPerformanceCalculator(
      aPerformance,
      playFor(aPerformance) // 함수선언바꾸기
    );
    return {
      ...aPerformance,
      play: calculator.play,
      amount: calculator.amount,
      volumeCredits: calculator.volumeCredits,
    };
  }

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

  function totalAmount(enrichPerformances: EnrichPerformance[]) {
    return enrichPerformances.reduce((acc, cur) => acc + cur.amount, 0);
  }

  function totalVolumeCredits(enrichPerformances: EnrichPerformance[]) {
    return enrichPerformances.reduce((acc, cur) => acc + cur.volumeCredits, 0);
  }
}

 

추상 클래스를 만들어서 상속, 오버라이딩하고 생성자를 팩터리 함수로 바꿨다. 코드는 길어졌지만 유지보수하기는 수월해졌다. 절대 짧은 코드가 좋은 코드는 아니다.

  > 좋은 코드를 가늠하는 확실한 방법은 `얼마나 수정하기 쉬운가`다.

 

https://github.com/yoonminsang/refactoring-2

GitHub - yoonminsang/refactoring-2: 리팩토링2 스터디