Vapor Trail

明るく楽しく元気よく

『リファクタリング』第15章 部品から全体へ

リファクタリング 技術書

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)

第15章 ｢部品から全体へ｣ Kent Beck

｢リファクタリングを実際に体得したと感じるのはどんなときでしょうか｡それは､みなさんが諦観を身につけ始めたときでしょう｡それは､誰かの書いたコードがどんなにぐちゃぐちゃでも､それを改善し､進化し続けるようにできるという､絶対的な自信を感じるようになるときです｡｣

｢自身を持ってリファクタリングをやめられるようになれば､それを体得したと実感できるでしょう｡やめることは､リファクタリングのレパートリーの中で最も強力な一手なのです｡コードが多少でも改善されていれば､それまでにやったことを統合してリリースしましょう｡改善されていなければ､手を引きましょう｡きっぱりなかったことにします｡達成できなかったことは残念ですが､勉強させてもらったことに満足しましょう｡｣

｢リファクタリングは学習可能な技能なのです｡｣

  • 目標の選別に慣れる

｢リファクタリングは､真実や美を追求するために行うもの(少なくともそれだけのもの)ではありません｡プログラムをわかりやすくし､ガタが来ているプログラムに喝を入れ直すために行うのです｡｣

  • 引き返す

｢私は､2つか4つのリファクタリングを､その間のテストケースを流さずに連続して行います｡だいじょうぶ､逃げ切れるさ｡自身もある｡経験も積んできたし･･･｡｢ドカーン｣｡あるテストが上手く行かず､どの変更がこの問題の原因なのかわかりません｡｣

  • 制御可能な状態に戻す｡

｢この自分のアドバイスに背いた報いの自己最高記録は､4時間で3回やり直したときです｡制御不能になり､一旦引き返し､はじめはゆっくりと前進し直したのですが､また制御不能になり､そしてもう1度やり直したのです｡｣

｢ゴミの山がどんなに醜く映ったとしても､問題を少しずつ減らすのだと自戒してください｡新たな機能を追加しようとするときは､まずちょっと時間をとって､それを整理してからにしましょう｡｣

｢リファクタリングしていると､必ずコードが正しく動かないケースに出会うはずです｡絶対に間違いありません｡そのときに､誘惑に負けないでください｡リファクタリングしているときの目標は､コードの計算結果を元のままに保つことなのです｡それ以外の何ものでもありません｡｣

Kent Beckも自分と同じミスをしていると思うと励まされる｡がんばろう｡

『リファクタリング』第7章 オブジェクト間での特性の移動

リファクタリング

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)

第7章 オブジェクト間での特性の移動

オブジェクトの設計の根幹は､責務をどこに配置するか｡責務をはじめから正しい場所に配置することは難しいため､リファクタリングを通して適切な場所に配置する｡

メソッドの移動

あるクラスのメソッドが他のクラスから多用されているとき､そのメソッドを多用するクラスに移動･作成する｡

  • 動機
    1. クラスの振る舞いが多い
    2. クラス間のやり取りが多く結合度が高いとき
<?php
class Account
{
    /**
     * 口座の種類
     * @var AccountType
     */
    private $type;
    /**
     * 当座貸越手数料
     * @var
     */
    private $daysOverdrawn;

    /**
     * @return float
     */
    public function overdraftCharge(): float
    {
        if ($this->type->isPremium()) {
            $result = 10;
            if ($this->daysOverdrawn > 7) $result += ($this->daysOverdrawn - 7) * 0.85;
            return $result;
        } else {
            return $this->daysOverdrawn * 1.75;
        }
    }

    /**
     * @return float
     */
    public function bankCharge(): float
    {
        $result = 4.5;
        if ($this->daysOverdrawn > 0) $result += $this->overdraftCharge();
        return $result;
    }
}

新たな口座の種類ができるとして口座の種類ごとに手数料を計算しなければならない場合､overdraftCharge()がAccountTypeクラスにへの結合度が高くなってしまうのでリファクタリングをする｡ 口座の種類はAccountTypeで判断させるため､overdraftCharge()をAccountTypeクラスに移動する｡

<?php
class Account
{
    /**
     * 口座の種類
     * @var AccountType
     */
    private $type;
    /**
     * 当座貸越手数料
     * @var
     */
    private $daysOverdrawn;

    /**
     * @return AccountType
     */
    public function getType(): AccountType
    {
        return $this->type;
    }

    /**
     * @return mixed
     */
    public function getDaysOverdrawn()
    {
        return $this->daysOverdrawn;
    }

    public function overdraftCharge()
    {
        return $this->type->overdraftCharge($this->daysOverdrawn);
    }

    /**
     * @return float
     */
    public function bankCharge(): float
    {
        $result = 4.5;
        if ($this->daysOverdrawn > 0) $result += $this->overdraftCharge();
        return $result;
    }
}

<?php
class AccountType
{
    /**
     * @param Account $account
     * @return float
     */
    public function overdraftCharge(Account $account): float
    {
        if ($this->isPremium()) {
            $result = 10;
            if ($account->getdaysOverdrawn() > 7) $result += ($account->getdaysOverdrawn() - 7) * 0.85;
            return $result;
        } else {
            return $account->getdaysOverdrawn() * 1.75;
        }
    }
}

フィールドの移動

ある週では正しく適正であった設計判断も､次の週にはそうではなくなる｡それ自体は問題ではなく､それについて何もしないことが問題｡リファクタリングを通して常に設計を修正する｡

  • 動機
  • そのクラスのメソッドよりも別のクラスのメソッドのほうがそのフィールドを多く使っているとわかったとき｡
  • クラスの抽出を行うため｡クラスの抽出を行う場合､先にフィールドを移動してからメソッドを移動させる｡
<?php
class Account
{
    /**
     * @var AccountType
     */
    private $type;
    /**
     * 利率
     * @var
     */
    private $interestRate;

    /**
     * @param float $amount
     * @param int $days
     * @return float|int
     */
    public function interestForAmountDays(float $amount, int $days): float
    {
        return $this->interestRate * $amount * $days / 365;
    }
}

Accountクラスの$interestRateのフィールドをAccountTypeクラスに移動する｡interestForAmountDaysメソッドがこのフィールドを利用している｡

<?php
class AccountType
{
    /**
     * @var float
     */
    private $interestRate;

    /**
     * @return mixed
     */
    public function getInterestRate(): float
    {
        return $this->interestRate;
    }

    /**
     * @param mixed $interestRate
     */
    public function setInterestRate(float $interestRate): void
    {
        $this->interestRate = $interestRate;
    }
}

<?php
class Account
{
    /**
     * @var AccountType
     */
    private $type;

    /**
     * @param float $amount
     * @param int $days
     * @return float|int
     */
    public function interestForAmountDays(float $amount, int $days): float
    {
        return $this->type->getInterestRate() * $amount * $days / 365;
    }
}

Accountクラスの$interestRateフィールドを削除する｡

クラスの抽出

｢クラスはきっちりと抽象化されたものであり､少数の明確な責務を担うべきである｣ 実際には操作やデータを追加していくため､クラスは成長する｡ クラスに責務を少しずつ追加していき､やがて手におえないほどクラスは複雑に成長する｡｡｡

  • 動機
  • クラスに大量のメソッドとデータがある｡
<?php
class Person
{
    /**
     * @var string
     */
    private $name;
    /**
     * @var string
     */
    private $officeAreaCode;
    /**
     * @var string
     */
    private $officeNumber;

    /**
     * @return string
     */
    public function getName(): string
    {
        return $this->name;
    }

    /**
     * @param string $name
     */
    public function setName(string $name): void
    {
        $this->name = $name;
    }

    /**
     * @return string
     */
    public function getOfficeAreaCode(): string
    {
        return $this->officeAreaCode;
    }

    /**
     * @param string $officeAreaCode
     */
    public function setOfficeAreaCode(string $officeAreaCode): void
    {
        $this->officeAreaCode = $officeAreaCode;
    }

    /**
     * @return string
     */
    public function getOfficeNumber(): string
    {
        return $this->officeNumber;
    }

    /**
     * @param string $officeNumber
     */
    public function setOfficeNumber(string $officeNumber): void
    {
        $this->officeNumber = $officeNumber;
    }

    public function getTelephoneNumber(): string
    {
        return "({$this->officeAreaCode}){$this->officeNumber}";
    }
}

電話番号の振る舞いをクラスとして抽出する｡

<?php
class Person
{
    /**
     * @var string
     */
    private $name;

    /**
     * @var TelephoneNumber
     */
    private $officeTelephone;

    /**
     * @return string
     */
    public function getName(): string
    {
        return $this->name;
    }

    /**
     * @param string $name
     */
    public function setName(string $name): void
    {
        $this->name = $name;
    }

    /**
     * @return TelephoneNumber
     */
    public function getOfficeTelephone(): TelephoneNumber
    {
        return $this->officeTelephone;
    }

    /**
     * @return string
     */
    public function getTelephoneNumber(): string
    {
        return $this->officeTelephone->getTelephoneNumber();
    }
}

<?php
class TelephoneNumber
{
    /**
     * @var string
     */
    private $areaCode;

    /**
     * @var string
     */
    private $number;

    /**
     * @return string
     */
    public function getNumber(): string
    {
        return $this->number;
    }

    /**
     * @param string $number
     */
    public function setNumber(string $number): void
    {
        $this->number = $number;
    }

    /**
     * @return string
     */
    public function getAreaCode(): string
    {
        return $this->areaCode;
    }

    /**
     * @param string $areaCode
     */
    public function setAreaCode(string $areaCode): void
    {
        $this->areaCode = $areaCode;
    }

    public function getTelephoneNumber(): string
    {
        return "({$this->areaCode}){$this->number}";
    }
}

委譲の隠蔽

人クラス

<?php
class Person
{
    /**
     * @var Department
     */
    private $department;

    /**
     * @return Department
     */
    public function getDepartment(): Department
    {
        return $this->department;
    }

    /**
     * @param Department $department
     */
    public function setDepartment(Department $department): void
    {
        $this->department = $department;
    }
}

部門クラス

<?php
class Department
{
    /**
     * @var string
     */
    private $chargeCode;
    /**
     * @var Person
     */
    private $manager;

    /**
     * Department constructor.
     * @param $manager
     */
    public function __construct(Person $manager)
    {
        $this->manager = $manager;
    }

    /**
     * @return Person
     */
    public function getManager(): Person
    {
        return $this->manager;
    }
}

例えば､会計部門の人を作成して､その部門のマネージャを取得したいとき､

<?php
$person = new Person($accountingDepartment);
$manager = $person->getDepartment()->getManager();

とする必要がある｡

<?php
    public function getManager(): Person
    {
        return $this->department->getManager();
    }

委譲メソッドをPersonクラスに作り､Departmentクラスをクライアントから隠蔽することで結合度を低くする｡

<?php
$person = new Person($accountingDepartment);
$manager = $person->getManager();

『リファクタリング』第1章 最初の例

技術書 リファクタリング

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)

リファクタリングをする上でテストは頼みの綱｡
テストの結果を見てバグが入ってしまわなかったを判断する｡
リファクタリングの成功は良いテストを用意できるかで決まる｡

メソッドの分割･再配置

  • 抜き出そうとする部分でローカルなスコープを持つ変数に着目し､それらが新規に作られるメソッドの一時変数かパラメータにならないか検討する｡

  • 変更されない変数についてはパラメータで渡すことができる｡変更される変数は注意を払う必要がある｡1つだけなら戻り値にできる｡

  • 変数名はコードのキーポイント｡わかりやすくするために変数名の変更を躊躇してはいけない｡

    コンパイラが理解できるコードは誰にでも書ける｡優れたプログラマは､人間にとってわかりやすいコードを書く｡

  • 一時変数は､不必要なパラメータをたくさん受け渡してしまう原因になるのでできるだけ取り除く｡ 一時変数はメソッドの中だけで有効なので､長くて複雑なルーチンの原因となる｡

他のオブジェクトの属性を調べるswitch文を書くことは､常に間違いである｡switch文は他のオブジェクトについてではなく､自分自身について行うべき｡

<?php
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::CHILDRENS:
                $result += 1.5;
                if ($this->getDaysRented() > 3) {
                    $result += ($this->getDaysRented() - 3) * 1.5;
                }
                break;
        }

RentalクラスからMovieクラスに移動

<?php
    public function getCharge(int $daysRented): float
    {
        $result = 0;

        switch ($this->getPriceCode()) {
            case Movie::REGULAR:
                $result += 2;
                if ($daysRented > 2) {
                    $result += ($daysRented - 2) * 1.5;
                }
                break;
            case Movie::NEW_RELEASE:
                $result += $daysRented * 3;
                break;
            case Movie::CHILDRENS:
                $result += 1.5;
                if ($daysRented > 3) {
                    $result += ($daysRented - 3) * 1.5;
                }
                break;
        }
        return $result;
    }

なぜ貸出料金の計算をMovieクラスで行うのか?
→映画の分類が追加される変更が予想されるから｡分類を変えたときの影響範囲をできるだけ小さくする｡
そのため映画の分類をRentalオブジェクトに渡すのではなく､貸出期間をMovieオブジェクトに渡す
getCharge()とgetTotalFrequentRenterPoints()をMovieクラスで処理をすることで､分類の変更によって影響を受ける2つの要素(料金とレンタルポイント)をまとめて当事者に任せることができる｡

Switch文のポリモーフィズムへの置き換え

diagram-3147769721547182912

Movieクラスは抽象クラスであるPriceに依存させる｡
利用する側はPriceのサブクラスを知る必要はない｡

料金体系が変わった場合でも､Priceのサブクラスを追加するか､各サブクラスで処理を変えればよいだけであり､変更が非常に簡単｡