PHPで『リファクタリング―プログラムの体質改善テクニック』を実装してみる (2) メソッドの分割

nda-desu.hatenablog.com

メソッドの分割

statementメソッドを分割していきます。

小さく分割していくことで管理が楽になるし、他のクラスへの移動も楽になる。

リファクタリング前のstatementメソッド

<?php

namespace App;

class Customer
{
   
   ~省略~

    public function statement
    {
        $totalAmount = 0;
        $frequentRenterPoint = 0;

        $result = 'Rental Point for ' . $this->getName() . "\n";

        foreach ($this->rental as $rental) {
            $thisAmount = 0;
            switch ($rental->getMovie()->getPriceCode()) {
                case Movie::REGULAR:
                    $thisAmount += 2;
                    if ($rental->getDaysRented() > 2) {
                        $thisAmount += ($rental->getDaysRented() - 2) * 1.5;
                    }
                    break;
                case Movie::NEW_RELEASE:
                    $thisAmount += $rental->getDaysRented() * 3;
                    break;
                case Movie::CHILD:
                    $thisAmount += 1.5;
                    if ($rental->getDaysRented() > 3) {
                        $thisAmount += ($rental->getDaysRented() - 3) * 1.5;
                    }
                    break;
            }
            // レンタルポイントを加算する
            $frequentRenterPoint++;

            // 新作を２日以上借りた場合は、ボーナスポイント
            if (($rental->getMovie()->getPriceCode() === Movie::NEW_RELEASE) && $rental->getDaysRented() > 1) {
                $frequentRenterPoint++;
            }
            // 貸し出しに関する数値の表示
            $result .= "\t" . $rental->getMovie()->getTitle() . "\t" . $thisAmount . "\n";
            $totalAmount += $thisAmount;
        }
        $result .= 'Amount owed is ' . $totalAmount . "\n";
        $result .= 'You earned ' . $frequentRenterPoint . ' frequent renter points' . "\n";

        return $result;
    }
}

いろいろやり過ぎている感があります。長くて読みづらい。

長すぎるメソッドのリファクタリングには

・メソッドの抽出
・問い合わせによる一時変数の置き換え
・メソッドオブジェクトによるメソッドの置き換え
・条件記述の分解

で対処できないかを考えてみましょうと本書にはあります。

そこで「メソッドの抽出」

まずは、switch文を抽出してみます。

switch文を抽出する

switch文は、amountForメソッドに移動させます。

<?php

namespace App;

class Customer
{
  
　~省略~

    public function statement()
    {
        $totalAmount = 0;
        $frequentRenterPoint = 0;

        $result = 'Rental Point for ' . $this->getName() . "\n";

        foreach ($this->rental as $rental) {
            $thisAmount = $this->amountFor($rental);
            // レンタルポイントを加算する
            $frequentRenterPoint++;
            // 新作を２日以上借りた場合は、ボーナスポイント
            if (($rental->getMovie()->getPriceCode() === Movie::NEW_RELEASE) && $rental->getDaysRented() > 1) {
                $frequentRenterPoint++;
            }
            // 貸し出しに関する数値の表示
            $result .= "\t" . $rental->getMovie()->getTitle() . "\t" . $thisAmount . "\n";
            $totalAmount += $thisAmount;
        }
        $result .= 'Amount owed is ' . $totalAmount . "\n";
        $result .= 'You earned ' . $frequentRenterPoint . ' frequent renter points' . "\n";

        return $result;
    }

    public function amountFor(Rental $rental)
    {
        $result = 0;
        switch ($rental->getMovie()->getPriceCode()) {
            case Movie::REGULAR:
                $result += 2;
                if ($rental->getDaysRented() > 2) {
                    $result += ($rental->getDaysRented() - 2) * 1.5;
                }
                break;
            case Movie::NEW_RELEASE:
                $result += $rental->getDaysRented() * 3;
                break;
            case Movie::CHILD:
                $result += 1.5;
                if ($rental->getDaysRented() > 3) {
                    $result += ($rental->getDaysRented() - 3) * 1.5;
                }
                break;
        }
        return $result;
    }
}

クラスの移動

amountForを移動できました。

ただ、amountForがCustomerクラスに定義されていていいのかを考えてみます。

多くの場合、使用するデータを持つオブジェクトにメソッドは定義される
p.17

amountForメソッドは、Rentalクラスのメソッドやデータを使っているので、Rentalクラスに移動します。

Rental.php

メソッド名をamountForからgetChargeに変更しています

<?php

namespace App;

use App\Movie;

class Rental
{
    
    ~省略~ 
　　
    public function getCharge()
    {
        $result = 0;
        switch ($this->getMovie()->getPriceCode()) {
            case Movie::REGULAR:
                $result += 2;
                if ($this->getDaysRented() > 2) {
                    $result += ($this->getDaysRented() - 2) * 1.5;
                }
                break;
            case Movie::NEW_RELEASE:
                $result += $this->getDaysRented() * 3;
                break;
            case Movie::CHILD:
                $result += 1.5;
                if ($this->getDaysRented() > 3) {
                    $result += ($this->getDaysRented() - 3) * 1.5;
                }
                break;
        }
        return $result;
    }
}

Customer.phpも変更します。

Customer.php

<?php

namespace App;

class Customer
{
   ~省略~

    public function statement()
    {
        $totalAmount = 0;
        $frequentRenterPoint = 0;

        $result = 'Rental Point for ' . $this->getName() . "\n";

        foreach ($this->rental as $rental) {
            $thisAmount = $rental->getCharge();

            // レンタルポイントを加算する
            $frequentRenterPoint++;
            // 新作を２日以上借りた場合は、ボーナスポイント
            if (($rental->getMovie()->getPriceCode() === Movie::NEW_RELEASE) && $rental->getDaysRented() > 1) {
                $frequentRenterPoint++;
            }
            // 貸し出しに関する数値の表示
            $result .= "\t" . $rental->getMovie()->getTitle() . "\t" . $thisAmount . "\n";
            $totalAmount += $thisAmount;
        }
        $result .= 'Amount owed is ' . $totalAmount . "\n";
        $result .= 'You earned ' . $frequentRenterPoint . ' frequent renter points' . "\n";

        return $result;
    }
}