#あすみかんの上にあすみかん

#たのしいことしかかかないことをここに決意します

発表「失敗例から学ぶSOLID原則」から「単一責任の原則」を考えてみる

技術 登壇

はじめに

PHPカンファレンス北海道2024にて発表する「失敗例から学ぶSOLID原則」において、発表時間の都合上「単一責任の原則」「リスコフの置換原則」に触れることができませんでした。 そのため、本編拡張版として、ブログで「単一責任の原則」について書いていきます💪

▼ 発表のスライド（O・I・D）はこちら！

speakerdeck.com

▼ Lについての補足記事はこちら！

asumikam.com

「インターフェース分離の原則」中の「どうしたら良かった？」について

資料中で、以下の様に解決法を提示していました。

みているﾂﾖﾂﾖの方にとっては「えっこれでおわり？」って思う部分があったかと思います....！！！

様々なことが様々あり、資料中ではあの部分までしか載せることができませんでした。

  • 15分じゃ最後まで話持っていけなさそう
    • マジでこれごめんなさい
  • 「インターフェース分離の原則」の説明をシンプルに表現したいな
    • before/afterでUseCase層の実装が殆ど変わらない方がわかりやすそう
    • UseCase層で$ecSiteの引数1つを受け取る方法で見せたい
  • convertとuploadがEcSiteに内包されていた方が取り回しやすい
    • 一括バリエーションアップロードのUseCaseの実装の時とか
  • サービスの時期・成長度合いによっては「これで充分」と言っても問題なさそう
    • このままでもまあ及第点とするか...

はい。

色々書きましたが、何を伝えたいかと言うと実は、資料中のこの実装は単一責任の原則に違反する可能性があります......。（なんてこった！）

どこが「単一責任の原則」に違反？

  • バリエーション・カテゴリーの様な概念が増え始めた時
  • 1つのECサイトで複数のアップロード方法がある時
<?php

class KamabokoIchiba implements EcSite, VariationConvertable, CategoryConvertable, OptionConvertable, TagConvertable
{

    public function convert(MyItem $myItem): array
    {
        // ...
    }

    public function convertVariation(MyItem $myItem): array
    {
        // ...
    }

    public function convertOption(MyItem $myItem): array
    {
        // ...
    }

    public function convertTag(MyItem $myItem): array
    {
        // ...
    }

    public function upload(array $ecItem): void
    {
        if(/* APIの時 */) {
            $this->uploadApi(); // privateメソッド
            return;
        }

        if (/* S3の時 */) {
            $this->uploads3(); // privateメソッド
            return;
        }
        
        if (/* FTPの時 */) {
            $this->uploadFtp(); // privateメソッド
            return;
        }
    }
}

だいぶ簡略化したコードなのですが、ストーリーを進めてみると、こんな感じに膨らんでいっちゃいそうです。 普通になんかめっちゃしんどそうですね。問題が別の場所に移動しただけ感があります...。

そのため資料中に示した方法だけだと"あと一歩！"足りない感があります。

「単一責任の原則」に反さない様にしてみる

convertとuploadで分ける

「単一責任の原則」の各クラスが1つだけの責任を持つべきというルールを考えるとconvertとuploadが同じクラスにあるのがちょっと怪しそうです。

つまり、KamabokoIchibaConverterとKamabokoIchibaUploaderの様にクラスを分け、もう全く別のクラスにする、という方法です。

こうすることによって、単一責任の原則がだいぶ満たされた様に思います。

  • 変換処理が変わったとすると...
  • before
    • アップロード部分は何も変化がないはずなのに"クラスの修正"が入る
    • 違和感
  • after
    • アップロード部分は何も変化がないので"クラスの修正"が入らない
    • YOSASO〜！

YOSASO〜！YOSASO〜！YOSASO〜！

...と、思ったのですが、ただ、そのクラスだけだと「一括バリエーションアップロード」のユースケースの時に都合が悪くなるのです。

いま、ユースケースは以下の様に実装されています。

<?php

/**
 * @param MyItem $myItem
 * @param EcSite|VariationConvertable[] $ecSiteList
 * @return void
 */
public function uploadVariationsUseCase(MyItem $myItem, array $ecSiteList): void
{
    foreach ($ecSiteList as $ecSite) {
        $ecVariation = $ecSite->convertVariation($myItem);

        $ecSite->upload($ecVariation);
    }
}

たとえば、EcSiteUploaderとEcSicteConverterでクラスを分けたとして、一括バリエーションで受け取る時にforeachで回す時に2つのクラスの整合性を取るのがしんどそうです。 （クラスを分けておいてController側やその手前とかで"整合性"を表現するのはありかなのかもしれません。）

また、この2つだけだと、例えばKamabokoIchibaUploaderに「複数のアップロード方法が記述」されているのは変わらず、まだまだif文はありそうです。

つまり、convertとuploadで責任を分離し、別クラスにするのは良さそう。 ただ、もうちょっと工夫が要りそう。

「何を選択するか知っている」と「実際の処理を知っている」で責任を分ける

<?php

interface BasicConverter {
    public function convertBasic();
}

interface VariationConverter {
    public function convertVariation();
}

interface Uploader {
    public function upload();
}

interface EcSite {
    public function getBasicConverter();

    public function getUploader();
}

interface VariableEcSite {
    public function getVariationConverter();
}

class KamabokoIchiba implements EcSite
{
    public function getBasicConverter()
    {
        return new KamaBasicConverter();
    }

    public function getUploader()
    {
        return new KamaApiUploader();
    }
}
  • 具体の処理を表現する
    • インターフェース
      • BasicConverter
      • VariationConverter（必要な時だけ）
      • Uploader
    • 具象
      • KamaBasicConverter
      • KamaApiUploader
  • 何を選択するか知っているインターフェース
    • インターフェース
      • EcSite
      • VariableEcSite（必要な時だけ）
    • 具象
      • KamabokoIchiba

この様に分けると、先ほどの悩みを解決しつつ、「単一責任の原則」を満たせそうです！

この実装だとユースケース層がどうなるかをみてみましょう。

<?php

/**
 * @param MyItem $myItem
 * @param EcSite|VariationConvertable[] $ecSiteList
 * @return void
 */
public function uploadVariationsUseCase(MyItem $myItem, array $ecSiteList): void
{
    foreach ($ecSiteList as $ecSite) {
        $ecVariation = $ecSite->getVariationConverter()->convertVariation($myItem);

        $ecSite->getUploader()->upload($ecVariation);
    }
}

いい感じ！いい感じ？

.................本当？？？？

はい.....次はデメテルの法則にむんむんに違反していそうですね......。

単一責任の原則に関しては満たせたので、この記事内ではここまでにしますが、デメテルの法則に違反しない様にする記事は.......

次回に続く！！！！！！！！