More than 1 year has passed since last update.

完走賞をとって Qiitan を自分にクリスマスプレゼントするAdvent Calendar 2022Day 10

ミノ駆動本を読んで、保守しやすい成長し続けるコードを書ける自分になる。 - 第1章悪しき構造の弊害を知覚する

Last updated at 2022-12-10Posted at 2022-12-09

はじめに

現在、 #ミノ駆動本 こと 『良いコード／悪いコードで学ぶ設計入門―保守しやすい成長し続けるコードの書き方』 を読み進めています。

今回は、 「第1章悪しき構造の弊害を知覚する」 を読んで学んだ内容について記事にします。
第1章でまずは悪い例を知りましょう、という位置付けの章です。

ただ読むだけでなく、以前取り組んでいたブラックジャックのソースコードを練習の舞台（良い具合に修正が必要なコードに仕上がっているはず…）として、ミノ駆動本で学んだ内容を元にチェック、修正するというアウトプットをしていこうと思っています。

ブラックジャックの PHP で書いたソースコードを修正したものを、記事の中では例文にします。

自己紹介

私は公務員からWebエンジニアに転向して、 2 年目に入ったところです。
1 年目は、研修 3 ヶ月、実務 9 ヶ月という内訳で、実務は設計メインの SES 出向先であるため、コードの実装は実務では未経験です。
そのため自己研鑽として、継続的にコードと向き合い技術を向上させるための課題を設定して取り組んでいます。

目的

記事を書いている現在、第 5 章を読み進めていますが、この本を理解するために時間を投下しまくって、良いコードを書けるようになるまで読み込みたい！という意気込みになっています。

とは言え、本を読むだけではあまり頭に残らないので、
手を動かしてコードに実際に反映してみて、それを記事としてアウトプットする過程で頭に刷り込みたいです。

1. 意味不明な命名
　 1-1. 技術駆動命名
　 1-2. 連番命名
 2. 理解を困難にする条件分岐のネスト
　 2-1. 何重にもネストしたロジック
　 2-2. 巨大なネスト
 3. さまざまな悪魔を招きやすいデータクラス
　 3-1. データクラス
　 3-2. 仕様変更時に牙を剥く悪魔
 4. 悪魔退治の基本

1. 意味不明な命名

「意味不明な命名」としては、「技術駆動命名」と「連番命名」の２つについて、説明されています。

1-1. 技術駆動命名

技術駆動命名 は、クラス名やメソッド名、変数名について、型名など技術ベースの命名をすることです。

ミノ駆動本では、変数名に intValue01 を用いるなど、プログラミング用語で命名されている悪いコードの例が載っています。

× 技術駆動命名

private function compareValues(int $intValue1, int $intValue2): string
{
    $result = '';
    if ($intValue1 > $intValue2) {
        $result = 'win';
    } elseif ($intValue1 < $intValue2) {
        $result = 'lose';
    } elseif ($intValue1 === $intValue2) {
        $result = 'draw';
    }
    return $result;
}

上記のコードは、以下のプレイヤーとディーラーの得点を比較して、勝敗を返すコードを、技術駆動命名に置き換えてみたコードです。

○ 技術駆動命名ではない

/**
 * プレイヤーとディーラーの得点を比較して、勝敗を返す
 *
 * @param DealerPlayer $dealerPlayer
 * @param Player $player
 * @return string $result
 */
private function compareScoreTotal(DealerPlayer $dealerPlayer, Player $player): string
{
    $result = '';
    $playerScoreTotal = $player->getScoreTotal();
    $dealerScoreTotal = $dealerPlayer->getScoreTotal();
    if ($playerScoreTotal > $dealerScoreTotal) {
        $result = $player::WIN;
    } elseif ($playerScoreTotal < $dealerScoreTotal) {
        $result = $player::LOSE;
    } elseif ($playerScoreTotal === $dealerScoreTotal) {
        $result = $player::DRAW;
    }
    return $result;
}

悪いコードと比較した良いコード、と言える自信がないですが、少なくとも技術駆動命名はしていないコードです。（ご了承ください…）

その他ブラックジャックのソースコード全体で、技術駆動命名についてはしていなかったですが、今後も気をつけます。

1-2. 連番命名

連番命名 は、クラス名やメソッド名に対して、番号付けで命名することです。

× 連番命名

class Class001 extends Class002
{
    public function method001()
    {
    }
    public function method002()
    {
    }
    public function method003()
    {
    }
}

確かに意味不明で自分が書いたコードでも分からなくなるでしょうね…。さすがに連番命名はしてなかったですが、今後も気をつけます。

2. 理解を困難にする条件分岐のネスト

「理解を困難にする条件分岐のネスト」としては、「何重にもネストしたロジック」と「巨大なネスト」の２つについて、説明されています。

2-1. 何重にもネストしたロジック

何重にもネストしたロジック は、私はダメだと分かっていても書いてしまいがちで、ブラックジャックのソースコードでもありました。以下は if 文が三重にネストしています。

× 何重にもネストしたロジック

/**
 * 勝敗を判定する
 *
 * @param Game $game
 * @return void
 */
public function judgeWinOrLose(Game $game): void
{
    echo Message::getStandMessage($game->getDealer()->getDealerPlayer());
    sleep(Message::SECONDS_TO_DISPLAY);

    if ($this->hasStand($game->getPlayers())) {
        $game->getDealer()->getDealerPlayer()->action($game);

        echo Message::getScoreTotalResultMessage($game->getDealer()->getDealerPlayer());
        sleep(Message::SECONDS_TO_DISPLAY);

        if ($game->getDealer()->getDealerPlayer()->hasBurstStatus()) {
            echo Message::getDealerBurstMessage();
            sleep(Message::SECONDS_TO_DISPLAY);

            foreach ($game->getPlayers() as $player) {
                if ($player->hasStandStatus()) {
                    $player->changeStatus($player::WIN);
                    echo Message::getWinByBurstMessage($player);
                    sleep(Message::SECONDS_TO_DISPLAY);
                }
            }
        } else {
            foreach ($game->getPlayers() as $player) {
                if ($player->hasStandStatus()) {
                    $result = $this->compareScoreTotal($game->getDealer()->getDealerPlayer(), $player);
                    $player->changeStatus($result);
                    echo Message::getResultMessage($player);
                    sleep(Message::SECONDS_TO_DISPLAY);
                }
            }
        }
    }
}

foreach 文もあるので、四重でしょうか…。

2-2. 巨大なネスト

何重にもネストしたロジックに、今後も何か仕様変更したときにどんどん行数を書き足して複雑さが悪化すると 巨大なネスト になってしまい、理解が困難になってしまいます。

× 巨大なネスト

/**
 * 勝敗を判定する
 *
 * @param Game $game
 * @return void
 */
public function judgeWinOrLose(Game $game): void
{
    echo Message::getStandMessage($game->getDealer()->getDealerPlayer());
    sleep(Message::SECONDS_TO_DISPLAY);

    if ($this->hasStand($game->getPlayers())) {
        $game->getDealer()->getDealerPlayer()->action($game);

        echo Message::getScoreTotalResultMessage($game->getDealer()->getDealerPlayer());
        sleep(Message::SECONDS_TO_DISPLAY);

        if ($game->getDealer()->getDealerPlayer()->hasBurstStatus()) {
            echo Message::getDealerBurstMessage();
            sleep(Message::SECONDS_TO_DISPLAY);

            foreach ($game->getPlayers() as $player) {
                if ($player->hasStandStatus()) {
                    $player->changeStatus($player::WIN);
                    echo Message::getWinByBurstMessage($player);
                    sleep(Message::SECONDS_TO_DISPLAY);
                }
            } else {
			// 数十〜数百行の処理
							if (何かの条件){
			// 数十〜数百行の処理
...

確かに今の理解度でコードを書き足し続けたら、巨大なネストにしてしまうと思います。

何重にもネストしたロジックや巨大なネストを、どのように回避していくかをミノ駆動本を読み進めていくことで、紐解いていきたいです。

3. さまざまな悪魔を招きやすいデータクラス

データクラスとは何かと、データクラスがあることで生じる問題についてです。

3-1. データクラス

データクラスは、データを保持するだけのクラスで、ロジックを持たないクラスです。

× データクラス

class Player
{
    public const HIT = 'hit';
    public const STAND = 'stand';
    public const BURST = 'burst';

    public const WIN = 'win';
    public const LOSE = 'lose';
    public const DRAW = 'draw';

    public const NO_SPLIT = 0; // 宣言なし
    public const SPLIT_FIRST = 1; // スプリット宣言 1 手目
    public const SPLIT_SECOND = 2; // スプリット宣言 2 手目

    /** @var string プレイヤー名 */
    public string $name,
    /** @var int チップ残高 */
    public int $chips = 100,
    /** @var int ベットした額 */
    public int $bets = 0,
    /** @var array<int,array<string,int|string>> 手札 */
    public array $hand = [],
    /** @var int プレイヤーの現在の得点 */
    public int $scoreTotal = 0,
    /** @var int プレイヤーの引いた A の枚数 */
    public int $countAce = 0,
    /** @var string プレイヤーの状態 */
    public string $status = self::HIT,
    /** @var int スプリット宣言の状態 */
    public int $splitStatus = self::NO_SPLIT
}

例えば、プレイヤーの現在の得点 $scoreTotal を計算するメソッドが、 PlayerManager クラスなど別のクラスに書かれている状態です。

× PlayerManager クラスに書かれる計算ロジック

class PlayerManager {
...
		/**
		 * プレイヤーの現在の得点を計算する
		 *
		 * @param Player $player
		 * @return void
		 */
		public function calcScoreTotal(Player $player): void
		{
		    $player->scoreTotal = 0;
		    $player->countAce = 0;
		    foreach ($player->hand as $card) {
		        if ($card['num'] === 'A') {
		            ++$player->countAce;
		        }
		        $player->scoreTotal += $card['score'];
		    }
		    $player->calcAceScore();
		}
...

ブラックジャックのソースコードでは、明らかにデータクラスになっているクラスは作っていなかったのですが、ロジックを書く場所（クラス）が適切ではなく、データクラスと実質似たような状態になってる箇所はあるような気がしています。

class Judge
{
    /**
     * カードの合計値が 21 を超えているかを判定する
     *
     * @param Player $player
     * @return bool 21 を超えたら true
     */
    public function checkBurst(Player $player): bool
    {
        if ($player->getScoreTotal() > 21) {
            $player->changeStatus($player::BURST);
            return true;
        }
        return false;
    }
...

例えばこの Judge クラスに書いた checkBurst メソッドは、 Player クラスのデータしか扱わないから、Player クラスにあった方が良いのではないか、と思いました。

今後の章で改善方法を学んで、必要であれば修正していきます。

3-2. 仕様変更時に牙を剥く悪魔

ミノ駆動本では、データクラスがあることで生じる問題を 仕様変更時に牙を剥く悪魔 として説明されていました。悪魔は以下の 5 つです。

重複コード

重複コード は、別のクラスに同じようなロジックが複数書いてあることです。関連するコードが離れていることで、重複コードを複数書いてしまい、量産されてしまいます。

修正漏れ

あっちにもこっちにも重複したコードがあることで、仕様変更時に 修正漏れ が発生しやすくなります。

可読性低下

可読性低下 は、早く正確に読み解くのが難しくなっている状態です。関連するコードが離れていて、重複コードが複数量産されていくと可読性が低下してしまいます。

未初期化状態（生焼けオブジェクト）

未初期化状態（生焼けオブジェクト） は、初期化しないと利用できないことを利用する側が知らずにバグが生じてしまう状態です。データクラスであることで、初期化が必要となっていると生じてしまいます。

不正値の混入

不正値の混入 は、仕様として正しくない値が入ってしまうことです。データクラスは不正値を簡単に受け取ってしまい、生じてしまいます。不正値が混入しないようにするには、データクラスの利用側でバリデーションロジックを実装する方法がありますが、それが重複コードの量産につながります。

重複コードは、データクラスではなくても、書いてあるロジックを再度書いてしまっていることはありました…。

4. 悪魔退治の基本

「悪魔退治の基本」は、以下の2点とのことでした。

「悪しき構造の弊害を知ること」
「オブジェクト指向の基本であるクラスを、適切に設計すること」

悪魔がどのように悪いやつなのかを知って、その悪魔を倒す武器を手に入れて、悪魔退治しましょう、ということですね。強い武器を手に入れるため、本を読み進めるのが楽しみです。

おわりに

今回は、 「第1章悪しき構造の弊害を知覚する」 を読んで学んだ内容について記事にしました。

「4. 悪魔退治の基本」にあるように、まずは悪い例を知ることで、その状態を避けようと考えられるので、大切なステップだと思えました。

また、以前取り組んでいたブラックジャックのソースコードを練習台として、ミノ駆動本を元にチェック、修正しようとすること、記事としてアウトプットすることで、ただ読むだけよりも理解度が断然上がっている実感があります。

引き続き、アウトプットすることを前提にミノ駆動本を読み進めていきます。

ありがとうございました。

You get articles that match your needs
You can efficiently read back useful information
You can use dark theme

What you can do with signing up

ミノ駆動本を読んで、保守しやすい成長し続けるコードを書ける自分になる。 - 第1章 悪しき構造の弊害を知覚する