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

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

ミノ駆動本を読んで、保守しやすい成長し続けるコードを書きたい。 - 第2章 設計の初歩

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

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

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

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

はじめに

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

gihyo.jp

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

ryamate.hatenablog.com

第 2 章は設計の初歩について、ということで、変数は省略しない、メソッド化しよう、というような初歩についてです。

記事の書き進め方

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

qiita.com

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

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

目次

ちなみにミノ駆動本の「第2章 設計の初歩」の目次は、以下の通りです。

2.1 省略せずに意図が伝わる名前を設計する

2.2 変数を使い回さない、目的ごとの変数を用意する

2.3 ベタ書きせず、意味のあるまとまりでメソッド化

2.4 関係し合うデータとロジックをクラスにまとめる

そのまま。

1. 省略せずに意図が伝わる名前を設計する

名前を省略することで、記述の量は減らせるけど、意図を読み解くのが難しくなってしまうので、意図が伝わる名前にしよう、ということです。

以下のコードは自分が書いた、勝敗の結果別でチップを計算するコードです。

<?php
//~~~
private function calcChipsByResult(Player $p): int
{
    $c = $p->getChips();

    switch ($p->getStatus()) {
        case $p::WIN:
            $c = $c + $p->getBets();
            break;
        case $p::LOSE:
        case $p::BURST:
            $c = $c - $p->getBets();
            break;
        case $p::DRAW:
            break;
    }
    return $c;
}

(あえて、$player$p に、$chips$c に、名前を省略して置き換えてみたものです。)

記述量は少なくてスッキリしている印象ですが、関数名や型があるから分かる状態でコードの量も短いので理解はできますが、省略すればするほど意図は伝わりにくくなるのは感じられます。

2. 変数を使い回さない、目的ごとの変数を用意する

また、変数 $c について、再代入(変数)して変数を使い回していることも問題です。同じ変数について、コードの途中で用途が変わっていくことで、読み手が混乱して(書いた自分自身含め)、バグを埋め込んでしまう可能性があります。

以下は省略せずに意図が伝わる名前にして、再代入せずに、計算後であることがわかる変数に代入しているコードです。

<?php
//~~~
private function calcChipsByResult(Player $player): int
{
    $chips = $player->getChips();

    switch ($player->getStatus()) {
        case $player::WIN:
            $chipsByResult = $chips + $player->getBets();
            break;
        case $player::LOSE:
        case $player::BURST:
            $chipsByResult = $chips - $player->getBets();
            break;
        case $player::DRAW:
            $chipsByResult = $chips;
            break;
    }
    return $chipsByResult;
}

3. ベタ書きせず、意味のあるまとまりでメソッド化

ブラックジャックのコードを書いているときは、実装していくときにはベタ書きでコードを書き出していくのですが、リファクタリングで意味のあるまとまりでメソッド化する意識は持ちながら実装しました。

<?php
//~~~
 /**
 * ベットする額を決める
 *
 * @return void
 */
private function placeYourBets(): void
{
    foreach ($this->players as $player) {
        $player->bet();
    }
}

/**
 * ブラックジャックを開始する
 *
 * @return void
 */
private function startGame(): void
{
    echo Message::getGameStartMessage();
    sleep(Message::SECONDS_TO_DISPLAY);

    $this->deck->initDeck();
    foreach ($this->players as $player) {
        $this->dealer->dealOutFirstHand($this->deck, $player);
    }
    $this->dealer->dealOutFirstHand($this->deck, $this->dealer->getDealerPlayer());

    foreach ($this->players as $player) {
        foreach ($player->getHand() as $card) {
            echo Message::getPlayerFirstHandMessage($player, $card);
            sleep(Message::SECONDS_TO_DISPLAY);
        }
        echo PHP_EOL;
    }

    echo Message::getDealerFirstHandMessage($this->dealer->getDealerPlayer());
    sleep(Message::SECONDS_TO_DISPLAY);
    echo Message::getDealerSecondHandMessage();
    sleep(Message::SECONDS_TO_DISPLAY);
}

これらのメソッドを以下のように呼び出す形にしています。

<?php
//~~~
 /**
 * ブラックジャックをプレイする
 *
 * @return void
 */
public function play(): void
{
    $this->set();
    while ($this->status === self::GAME_STATUS[self::CONTINUE]) {
        $this->placeYourBets();
        $this->startGame();
        $this->action();
        $this->resultGame();
        $this->resultCalcChips();
        $this->selectContinueOrStop();
    }
    $this->end();
}

これらが全部ベタ書きだったら、きっと読みにくいはずです。

4. 関係し合うデータとロジックをクラスにまとめる

ミノ駆動本では、戦闘を伴うゲームの主人公のHP(ヒットポイント)を例に、単なる変数としてHPを用意すると、その変数を操作するロジックがバラバラに書かれてしまう、という問題が書かれています。

ブラックジャックについて考えると、チップや点数(カードの合計値)のようなデータがあります。

現状のコードは以下のようなチップ計算のクラスを作成していますが、チップ自体のデータのクラスは作成していません。

<?php

namespace Blackjack;

require_once('Player.php');

use Blackjack\Player;

class ChipCalculator
{
    /**
     * 勝敗、特殊ルールに応じたプレイヤーのチップ残高を算出する
     *
     * @param Game $game
     * @param Player $player
     * @return void
     */
    public function calcChips(Game $game, Player $player): void
    {
        switch ($player->getSplitStatus()) {
            case $player::NO_SPLIT:
                $chipsByResult = $this->calcChipsByResult($player);
                $player->changeChips($chipsByResult);
                $this->displayMessageByResult($player);
                $player->reset();
                break;
            case $player::SPLIT_FIRST:
                echo Message::getSplitDeclarationMessage($player);
                $chipsByResult = $this->calcChipsByResult($player);
                $player->changeChips($chipsByResult);
                $this->displayMessageByResult($player);
                $player->reset();
                break;
            case $player::SPLIT_SECOND:
                $this->calcChipsSplitSecondHand($game, $player);
                $this->resetStatusSplitPlayer($game, $player);
                $this->removePlayerSecondHand($game, $player);
                break;
        }
    }

    /**
     * 結果別でチップを計算する
     *
     * @param Player $player
     * @return integer $chipsByResult
     */
    private function calcChipsByResult(Player $player): int
    {
        $chips = $player->getChips();

        switch ($player->getStatus()) {
            case $player::WIN:
                $chipsByResult = $chips + $player->getBets();
                break;
            case $player::LOSE:
            case $player::BURST:
                $chipsByResult = $chips - $player->getBets();
                break;
            case $player::DRAW:
                $chipsByResult = $chips;
                break;
        }
        return $chipsByResult;
    }

    /**
     * 結果別でメッセージを表示する
     *
     * @param Player $player
     * @return void
     */
    private function displayMessageByResult(Player $player): void
    {
        switch ($player->getStatus()) {
            case $player::WIN:
                echo Message::getWinAndGetChipsMessage($player);
                break;
            case $player::LOSE:
            case $player::BURST:
                echo Message::getLoseAndLoseChipsMessage($player);
                break;
            case $player::DRAW:
                echo Message::getDrawAndKeepChipsMessage($player);
                break;
        }
        sleep(Message::SECONDS_TO_DISPLAY);
        echo Message::getChipBalanceMessage($player);
        sleep(Message::SECONDS_TO_DISPLAY);
    }

    /**
     * スプリット宣言プレイヤーの2手目(splitStatus: 2) について、チップ残高を算出する
     *
     * @param Game $game
     * @param Player $playerSecondHand
     * @return void
     */
    private function calcChipsSplitSecondHand(Game $game, Player $playerSecondHand): void
    {
        foreach ($game->getPlayers() as $player) {
            if ($this->isSplitFirstHand($player, $playerSecondHand)) {
                $chipsByResult = $this->calcChipsByResult($player);
                $player->changeChips($chipsByResult);
                $this->displayMessageByResult($player);

                $game->removeSplitPlayer($playerSecondHand);
                break;
            }
        }
    }

    /**
     * スプリット宣言したプレイヤーのステータスをリセット削除する
     *
     * @param Game $game
     * @param Player $playerSecondHand
     * @return void
     */
    private function resetStatusSplitPlayer(Game $game, Player $playerSecondHand): void
    {
        foreach ($game->getPlayers() as $player) {
            if ($this->isSplitFirstHand($player, $playerSecondHand)) {
                $player->changeSplitStatus($player::NO_SPLIT);
                break;
            }
        }
    }

    /**
     * スプリット宣言したプレイヤーの 2 手目を削除する
     *
     * @param Game $game
     * @param Player $playerSecondHand
     * @return void
     */
    private function removePlayerSecondHand(Game $game, Player $playerSecondHand): void
    {
        foreach ($game->getPlayers() as $player) {
            if ($this->isSplitFirstHand($player, $playerSecondHand)) {
                $game->removeSplitPlayer($playerSecondHand);
                break;
            }
        }
    }

    /**
     * $player が、スプリット宣言プレイヤーの 1 手目か否かを判定する
     *
     * @param Player $player
     * @param Player $playerSecondHand
     * @return boolean
     */
    private function isSplitFirstHand(Player $player, Player $playerSecondHand): bool
    {
        if (
            $player->getName() === $playerSecondHand->getName() &&
            $player->getSplitStatus() === $playerSecondHand::SPLIT_FIRST &&
            $playerSecondHand->getSplitStatus() === $playerSecondHand::SPLIT_SECOND
        ) {
            return true;
        }
        return false;
    }
}

Chip クラスを新たに作成して、チップ自体のデータ構造や状態管理をさせて、関係し合うデータとロジックをクラスにまとめることも可能なのかな、と考えています。

おわりに

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

第 2 章は設計の初歩について、ということで、変数は省略しない、メソッド化しよう、というような初歩についてでした。

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

#本日の自習 ポストをすることで、日々の学習にハリが出る。