Refactoring 2nd EditionはJavaScriptで書かれている
- コードを読める人口がもっとも多いであろう、という理由から
- 「JavaScriptのリファクタリングの本」ではない
最近の普段遣いの言語はPHPなので、サンプルコードをPHPで書き直し、実際に手を動かしてリファクタリング実習してみた
GitHubリポジトリ

The Starting Point

第1版では「レンタルビデオ店」のサンプルコードだったらしい
第2版では「演劇の料金計算」のサンプルコード
- 今日びでは「『レンタルビデオ店』ってなに？」ってなることうけあいだから
repos

<?php
function statement($invoice, $plays)
{
    $totalAmount = 0;
    $volumeCredits = 0;
    $result = "Statement for ${invoice['customer']}";
    $format = '$%.2f';
    foreach ($invoice['performances'] as $perf) {
        $play = $plays[$perf['playID']];
        $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 += max($perf['audience'] - 30, 0);
        // add extra credit for every ten comedy attendees
        if ('comedy' === $play['type']) $volumeCredits += floor($perf['audience'] / 5);
        // print line for this order
        $result .= "  ${play['name']}: " . sprintf($format, $thisAmount / 100) . "(${perf['audience']} seats)" . PHP_EOL;
        $totalAmount += $thisAmount;
    }
    $result .= 'Amount owed is ' . sprintf($format, $totalAmount / 100) . PHP_EOL;
    $result .= "You earned ${volumeCredits} credits" . PHP_EOL;
    return $result;
}

やりたいリファクタリングはいろいろある
- 中間データ生成とレンダリングを2-phaseに分ける
- Polymorphismを使ってif文やswitch-case文を排除する

【補】テスト書く

何より先に、まずテスト書く
repos

<?php
declare(strict_types=1);
use PHPUnit\Framework\TestCase;
class ExampleTest extends TestCase
{
    /**
     * @test
     * @dataProvider dataProvider
     */
    public function statement_正しい結果を得る(
        array $invoice,
        array $plays,
        string $statementExpected
    ) {
        $output = statement($invoice, $plays);
        $this->assertSame(
            $statementExpected,
            $output
        );
    }
    function dataProvider()
    {
        return [
            [
                [
                    'customer' => 'BigCo',
                    'performances' => [
                        [
                            'playID' => 'hamlet',
                            'audience' => 55,
                        ],
                        [
                            'playID' => 'as-like',
                            'audience' => 35,
                        ],
                        [
                            'playID' => 'othello',
                            'audience' => 40,
                        ],
                    ],
                ],
                [
                    'hamlet' => [
                        'name' => 'Hamlet',
                        'type' => 'tragedy',
                    ],
                    'as-like' => [
                        'name' => 'As You Like It',
                        'type' => 'comedy',
                    ],
                    'othello' => [
                        'name' => 'Othello',
                        'type' => 'tragedy',
                    ],
                ],
                <<< EOL
Statement for BigCo  Hamlet: $650.00(55 seats)
  As You Like It: $580.00(35 seats)
  Othello: $500.00(40 seats)
Amount owed is $1730.00
You earned 47 credits
EOL
            ]
        ];
    }
}

本当は例外処理のテストも書かなきゃダメ

Decomposing the `statement` Function

まず一枚岩の巨大な関数を分割し構造化する
repos

<?php
function statement($invoice, $plays)
{
    $playFor = function ($perf) use ($plays) {
        return $plays[$perf['playID']];
    };
    $amountFor = function ($aPerformance) use ($playFor) {
        $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;
    };
    $volumeCreditsFor = function ($aPerformance) use ($playFor) {
        $result = 0;
        $result += max($aPerformance['audience'] - 30, 0);
        if ('comedy' === $playFor($aPerformance)['type']) $result += floor($aPerformance['audience'] / 5);
        return $result;
    };
    $usd = function ($aNumber) {
        $format = '$%.2f';
        return sprintf($format, $aNumber / 100);
    };
    $totalVolumeCredits = function () use (
        $invoice,
        $volumeCreditsFor
    ) {
        $volumeCredits = 0;
        foreach ($invoice['performances'] as $perf) {
            $volumeCredits += $volumeCreditsFor($perf);
        }
        return $volumeCredits;
    };
    $totalAmount = function () use ($invoice, $amountFor) {
        $result = 0;
        foreach ($invoice['performances'] as $perf) {
            $result += $amountFor($perf);
        }
        return $result;
    };
    // ----------------------------------------
    $result = "Statement for ${invoice['customer']}";
    foreach ($invoice['performances'] as $perf) {
        // print line for this order
        $result .= '  ' . $playFor($perf)['name'] . ': ' . $usd($amountFor($perf)) . "(${perf['audience']} seats)" . PHP_EOL;
    }
    $result .= 'Amount owed is ' . $usd($totalAmount()) . PHP_EOL;
    $result .= 'You earned ' . $totalVolumeCredits() . ' credits' . PHP_EOL;
    return $result;
}

適用したリファクタリングパターン
- Extract Function
  - 処理を関数に切り出す
  - 例
- Replace Temp With Query
  - 関数の結果を一時変数に格納していたのを、関数を逐一呼び出すように置換
  - 例
  - パフォーマンス面で物議を醸す
  - 著者は「パフォーマンスチューニングしやすくなるから、先に構造化する」という立場
    - 【補】ドナルド・クヌース先生の「早すぎる最適化は諸悪の根源」というやつの真意
    - 最終的には可読性・パフォーマンスともに優れるコードになる予定
- Inline Variable
  - 式を一時変数に受けず、右辺値のまま使う
  - 例
- Change Function Declaration
  - 名前変更、引数削除等
  - 例
  - 適切な名前にすることで、中身が変わることも
    - この例では、「セントを受け取ってUSドル形式にフォーマットする」usd関数
    - 「100で割る」処理も含まれるようになった
- Split Loop
  - アキュムレーションごとにループを水平分割
  - 例
- Slide Statements
  - アキュムレータ変数の宣言・初期化をループ近くに持っていく
  - 例
  - Split Loopとともに行い、Extract Functionにつなげる

勉強日記

チラ裏

Refactoring ch1 A First Example -- PHPで書き直してみた (1/3)

The Starting Point

【補】テスト書く

Decomposing the `statement` Function

The Starting Point

【補】テスト書く

Decomposing the statement Function

Decomposing the `statement` Function