Refactoring第2版のサンプルコードをTypeScriptで実習する part.2 Replace Temp with Query #TypeScript

前回に引き続き、statement.tsのリファクタリングを進めていきます。

今回は、一時変数(Temp)を関数呼び出し(Query)で置き換えるリファクタリングです。

Removing the play Variable

前回作成した amountFor() 関数は、perfとplayという2つの引数を取ります。このうち、playはperfがもつ値を元に取得できることがわかります。

statement.ts

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

このような一時変数は、関数呼び出しに置き換えることができます。

statement.ts

  const playFor = (aPerformance: Performance) => plays[aPerformance.playID];

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

ここでちょっと想定外の事態が発生。amountFor関数はグローバルだと思ってたんですが、statement関数の関数内関数だったようです（そういえばそういう話も書いてあったような…）。

個人的に、関数内関数にはやや否定的で、playForのような1行関数くらいならともかく、amountForのように本格的なロジックを持つ関数は単独の関数として定義すべきだと思っています（実は、後の章でこの状況を改善するリファクタリングがあります）。

それはともかく、amountForをstatementの中に移し、次にamountForの中のplay変数をplayFor関数の呼び出しで置き換えます。これによって、playパラメータが不要になるので削除します。

  function amountFor(aPerformance: Performance): number {
    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;
  }

このリファクタリングによって、元のコードよりもパフォーマンスが劣化しているのでは？　と気になるかもしれませんが、その点については、(1) 実際には誤差の範囲 (2) 綺麗なコードになればあとで簡単にチューニングできるという2点を補足しておきます。

一時変数のplayと同様、thisAmountもamountFor関数の呼び出しで置き換えます。

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

Extracting Volume Credits

次に、volumeCreditsの計算を関数として抽出します。先ほどのリファクタリングでplayが関数呼び出しに置き換えられたので、volumeCreditsForの抽出は簡単です。

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

  for (let perf of invoice.performances) {
    // add volume credits
    volumeCredits += volumeCreditsFor(perf);

Removing the format Variable

一時変数はよくないコードの兆候です。ここでは、format変数を関数に置き換えます。

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

また、formatという関数名は一般的すぎて何をするかわかりづらいので、受け取った数値をドルとして文字列整形する関数usdに名前を変更し、これにともなって処理も一部変更します。

function usd(aNumber: number): string {
  return new Intl.NumberFormat("en-US", {
    style: "currency",
    currency: "USD",
    minimumFractionDigits: 2
  }).format(aNumber / 100);
}
...
    result += `  ${playFor(perf).name}: ${usd(amountFor(perf))} (${
      perf.audience
    } seats)\n`;
    totalAmount += amountFor(perf);
  }
  result += `Amount owed is ${usd(totalAmount)}\n`;

今回はここまで。次回はvolume creditsの計算をもう一段階リファクタリングします。