スマレジのテックファーム（SES 部門）で Web 系エンジニアとして働いているやまて（@r_yamate）と申します。

2023 年 4 月からは、スマレジの関連アプリの開発業務を担当しています。

業務でのコーディングは初めてで、日々奮闘中です。Flutter（Dart、Kotlin）をメインで担当していて、React、Laravel の一部機能を担当しています。

開発しているアプリがついにリリースされ、利用していただく日が楽しみです。

はじめに

今回は、 #ミノ駆動本こと『良いコード／悪いコードで学ぶ設計入門―保守しやすい成長し続けるコードの書き方』の「第1章悪しき構造の弊害を知覚する」を読んで学んだ内容について記事にします。

gihyo.jp

まずは悪い例を知りましょう、という位置付けであるのが第 1 章です。

記事の書き進め方

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

qiita.com

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

また、本を網羅的に記事にするのではなく、できていなかったところや主観的に特に気をつけたいと思ったところを抜粋して、書き進めたいと思います。

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

ちなみにミノ駆動本の「第1章悪しき構造の弊害を知覚する」の目次は、以下の通りです。

1.1 意味不明な命名

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

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

1.4 悪魔退治の基本

1. 意味不明な命名

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

技術駆動命名

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

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

以下のコードは自分が書いた、プレイヤーとディーラーの得点を比較して、勝敗を返すコードです。

<?php
// ~~~
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;
}

…さすがに実務未経験の頃の私もここまでの技術駆動命名はしておらず、上記が下記のコードをあえて技術駆動命名に置き換えてみたものです。

<?php
// ~~~
/**
 * プレイヤーとディーラーの得点を比較して、勝敗を返す
 *
 * @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;
}

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

連番命名

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

<?php
// ~~~
class Class001 extends Class002
{
    public function method001()
    {
    }
    public function method002()
    {
    }
    public function method003()
    {
    }
}

確かに意味不明で自分が書いたコードでも分からなくなるでしょうね…。

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

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

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

何重にもネストしたロジック は、私はダメだと分かっていても初学者の頃はどのように書けばネストしないか解決方法がよく分からず、ブラックジャックのソースコードでも書いてしまいがちでした。以下は if 文が三重にネストしています。

<?php
// ~~~
/**
 * 勝敗を判定する
 *
 * @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 文もあるので、四重でしょうか…。

このコードもあえてネストさせました。...と言いたいところですが何重にもネストさせたロジックを私が書きました。

巨大なネスト

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

<?php
// ~~~
/**
 * 勝敗を判定する
 *
 * @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. さまざまな悪魔を招きやすいデータクラス

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

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

<?php
// ~~~
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 クラスなど別のクラスに書かれている状態です。

<?php
// ~~~
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();
        }
...

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

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

仕様変更時に牙を剥く悪魔

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

悪魔は以下の 5 つです。

重複コード … 別のクラスに同じようなロジックが複数書いてある
修正漏れ … 重複したコードがあると発生しやすい
可読性低下 … 関連するコードが離れていて、重複コードが多いと生じる
未初期化状態（生焼けオブジェクト） … 初期化しないと利用できないことを利用する側が知らずにバグが生じてしまう状態。データクラスであることで、初期化が必要となっていると生じる
不正値の混入 … 仕様として正しくない値が入ってしまうこと。データクラスは不正値を簡単に受け取ってしまい、生じる

悪魔たちとはめちゃくちゃ面識があります。

4. 悪魔退治の基本

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

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

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

おわりに

今回は、#ミノ駆動本こと『良いコード／悪いコードで学ぶ設計入門―保守しやすい成長し続けるコードの書き方』の「第1章悪しき構造の弊害を知覚する」を読んで学んだ内容について記事にしました。

まずは悪い例を知ることで、その状態を避けようと考えられるので、大切なステップでした。

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

昨日の夜は漫画を楽しみました😌 pic.twitter.com/DxyI5MCh10
— やまて｜Web系エンジニア3年目 (@r_yamate) 2024年2月23日

保守しやすい成長し続けるコードを書けるようになるには、息抜きも必要です。

転職したらスマレジだった件

スマレジのエンジニアやまてのテックブログです。マジレス大歓迎です。

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