Zenn
🏛️

神経質が設計にこだわりすぎた失敗談①　細分化しすぎ

2021/12/25に公開1件
C#
Unity
プログラミング
設計
tech

この記事を読んで欲しい人

  • 神経質な人
  • プログラム設計に、強いこだわりを持ってる人
  • 極端すぎるトンデモ思考を見て、笑顔になりたい人

設計とは？

簡単なアプリケーションを作るとき、

  1. 仕様を決定して
  2. どのツールを使うか選んで
  3. クラスや関数、それらの関係を定義して
  4. その中身を実装していく

というステップを踏む。
その中で、クラスや関数、それらの関係を定義する段階が設計と呼ばれる。

補足

大規模なアプリケーションだと、「設計」という工程はもっと広い意味を持ちます。
ここでは、分かりやすくするために便宜上「設計」を「クラスや関数を定義し、その関係を決定する工程」とします。

設計にこだわりすぎる問題

プログラムコードを書いていると、なんだか納得の行くコードが書けなくて、むず痒くなるタイミングがある。

「ここ、クラスや関数を分割したほうがいいかな......？」
「なんかコードが汚いから、もうちょっと綺麗にならないかな......？」
「このクラスの命名、こっちのほうがいいんじゃないかな......？」

これらは非常に良い。非常に良い気付き。なんだけど......
何事も、やりすぎは良くない。
設計を意識するのは良いことだけど、意識しすぎると本末転倒なことが起こる。

この記事では、私が経験した失敗パターンと、当時の思考を紹介しよう。
あまりに極端すぎて「作り話でしょこんなの笑」となるかもしれないけど、実話。マジ。
今、「設計に悩みすぎている」 と感じている人は、ぜひ読んでいって欲しい。

失敗エピソード「細分化しすぎ」

私が2年前、Unityでゲームを作っていたときの話。言語はC#。

Unityがわからない人向けのちょっとした説明

Unityでは、MonoBehaviourを継承したクラスを「コンポーネント」として、ゲーム上にあるオブジェクトに付与することで機能を実装していきます。
また、MonoBehaviourをオブジェクトに付与したり、パラメータや参照先を指定したりするときは、基本ドラッグ＆ドロップでの操作となります（重要）。

神の声「再利用できるところは再利用せよ」

😕「このクラスとあのクラスで同じことしてるな...」

😉「再利用できるように切り出して、コンポーネント化してしまおう！」

// 正の整数を保持するコンポーネント
public class PositiveIntegerHolder : MonoBehaviour {
    [SerializeField] private int value = 0;
    public int Value {
        get { return this.value; }
	set { this.value = Math.Max(0, value); }
    }
}

😄「よし！これでHPでもMPでも使い回せるぞー！」

変数1個だけやんけ！！！これ使うたびにドラッグ＆ドロップしてたら指がもげるわ！！！

😐「このクラスは、キャラクターのMPが0になったときに失敗アニメーションを再生するイベント処理をしてる。」

😨「他のイベント処理を同じクラスに混ぜるのも気持ち悪いから、1つのクラスとして分割してしまおう！」

public class Hoge : MonoBehaviour {
    // キャラクターの各パラメータを保持しているオブジェクト
    [SerializeField] private CharacterParamsHolder paramsHolder;
    // キャラクターの各アニメーションを再生するオブジェクト
    [SerializeField] private CharacterAnimatorPlayer animatorPlayer;
    
    public void Start() {
        // MPが0になったときに、失敗アニメーションを再生する
        paramsHolder
	    .OnMagicPointEmpty
	    .Subscribe(_ => animatorPlayer.PlayMagicFailedAnimation());
    }
}

嫌な予感が...

🤨「クラス名、どうしようかな。なるべく誤解を招かないものがいいな。」

class EmptyMagicPointFailedAnimationPlayer

😎「これなら誤解を招かない！最高だね！」

読めるかこんなもん！！！早口言葉かよ！！！

🥰「あとはこれを必要な分作って...完成！」

生成された大量のコンポーネント
当時のスクリーンショットの生き残り

おわりだおわり。

さらに最悪なのが、これらはUnityのMonoBehaviourコンポーネントなのである。
つまり、この後大量のモジュールをドラッグ＆ドロップする地獄のような作業が待っているのであった......

❌ 問題点① 共通部分をコンポーネント分割した。

「このクラスとあのクラスで同じことをしてる」から、コンポーネントとして分割する。

👆 解決策①「共通化をするべきか」を考えてみる。

私みたいに神経質な人間というのは、「同じ部分がある」とすぐに 「共通化しなきゃ！」 となる。というか、もはや無意識に手が動いてる。

待って。思いついて手を動かす前に深呼吸をして。10秒間、ゆーーーっくり息を吸うの。

その共通化、意味ある？
偶然同じだけだったりしない？
あとで片方だけ変更されたりしない？

特に、個人・少人数で作ってるゲームとかGUIだと、「実装 → 良い感じかチェック → 変更 → 再度チェック」という感じに仕様が良く変わるので、
「共通化したけど、結局後から変更加えたから意味なかった」
というケースが多い。

今回の例は、無意識に共通化をしていたため危なかったが、その部分が偶然共通化をするべき部分だった。
では、他に何が問題だったのだろうか。

👆 解決策②「コンポーネント化」以外の選択肢を考えてみる。

今回の例だと、コンポーネント化してしまったのが良くなかった。
なぜなら、機能が「正の整数を保持する」しかないものを、コンポーネントとして扱う意味がないからだ。

今の私だったら、このようにする。

public readonly struct PositiveInteger {
    public readonly int value;
    public PositiveInteger(int value) {
        this.value = Math.Max(0, value);
    }
}

正の整数を、読み取り専用オブジェクトとしてstructで作成する。
こうすれば、ドラッグ＆ドロップの作業なんて必要なく、コード内に型として記述するだけで良い。

public class Player {
    public PositiveInteger HP;
    public PositiveInteger MP;
    /* something... */
}

このように、共通化をする場合は手段を選ぶことが大事。
「継承」「ヘルパー関数」「コンポーネント」「委譲」「読み取り専用オブジェクト」など......
それぞれを、目的に合わせて上手く使い分けよう。

❌ 問題点② イベント処理1個ごとにクラスを作った。

「他のイベント処理を同じクラスに混ぜるのが気持ち悪い」から、イベント処理1個ごとにクラスを分割する。

イベント処理とは、「プレイヤーのHPが変化したとき、HPゲージの値を変化させる」のように、「何かのイベントが発生したときに、他の要素に影響を与える」処理のこと。

これは特に、MVPでいう「Presenter」などの「橋渡し」のクラスを書こうとすると起こりやすい問題。
それぞれのイベント処理があまり関連性を持ってない場合があり、上手く同じクラスにまとめられない。

MVPの構造
MVPの構造

👆 解決策①「イベントの発行元」に紐付ける

一番簡単な解決法。
「イベントの発行元クラスが同じ」イベント処理ごとにクラスを分けるのだ。

プレイヤーについての機能を持つPlayerというコンポーネントがあったとき、

  • 「プレイヤーのHPが変化したとき、HPゲージの値を変化させる」
  • 「プレイヤーがジャンプに成功したとき、ジャンプのアニメーションを再生する」
  • 「プレイヤーが死亡したとき、ゲームシーンを切り替える」

といったイベント処理を、すべて同じクラス（例えばPlayerPresenter）に記述する。

Playerに紐づけられたイベント処理
Playerに紐づけられたイベント処理

「えっ、なんか単一責任原則に反してそうだし、気持ち悪い......」と思うかもしれない。というか現に当時の私は思った。
そこで、私が自分で考え、自分で納得した解釈が2つある。

🙄 1. 責務は？

このとき、PlayerPresenterは「Playerの発行したイベントに対して、外部コンポーネントに副作用を与える」という責務を持つ。

🙄 2. イベント駆動じゃなかったら？

今回はイベントを購読する実装だが、もしイベント駆動じゃない場合を想像してみよう。

イベント駆動じゃなかった場合の構造
イベント駆動じゃなかった場合の構造

Modelである Player が 「Modelの状態変化を、外部へ伝達するためのインタフェースIPlayerPresenter」を用意 し、それを Presenterである PlayerPresenter が実装することになる。
つまり、イベント処理を行う Presenter は、Model に紐づいているのだ。

しかし、これでも実際に実装をしていると文句が出てくる。

😰 1. 外部クラスが多すぎる！

「複数のイベント処理を同じクラスにまとめたせいで、操作しなきゃいけない外部クラスが多すぎる！」
「コンストラクタの引数が多すぎる！」

public class PlayerPresenter {
    private Player player;
    private HPGauge hpGauge;
    /* 外部クラスの参照を保持する大量のプロパティ */
    
    public PlayerPresenter(
        Player player, HPGauge hpGauge, ...
	/* 大量の外部クラス */
    ) {
        this.player = player;
	this.hpGauge = hpGauge
        /* 大量の代入処理 */
    }
    
    public void Start() {
        /* 各イベント処理 */
    }
}

確かにそう。
この場合、次のような基準を考えて、このクラスをさらに分割する必要がある。

  • 外部クラスを「UI」「アニメーション」「ステータス効果」のように分類する。
    そして、それぞれを使用するイベント処理ごとにクラスを分ける。
    こうすることで、各クラスが使用する外部クラスの数を減らすことができる。

......こんなこと言いたくはないけど、この工程はセンスが必要だと思う。多くのコードを書いているうちに、臨機応変に上手い分け方ができるようになっていく。

ちなみに、私はこういうとき 「密凝集・疎結合」 を第一に意識している。参考になれば嬉しい。

😰 2. 依存関係が引っかかる！

「プレイヤーが死亡したときに、ゲームシーンを切り替えるようにしよう！」
「...待てよ。プレイヤーがゲームシステムに依存していいのか...？」

ゲームシステムに依存するプレイヤー
ゲームシステムに依存するプレイヤー

よくない。なぜなら、プレイヤーを他の部分で再利用したりできなくなるから。

かと言って、ゲームシステム側にイベント処理を紐づけると、
ゲームシステム側にイベント処理を書いたとき
ゲームシステム側にイベント処理を書いたとき

「イベント処理が『イベントの発行元』に紐づいてる場合と、『イベントの影響を受ける側』に紐づいてる場合が共存してて気持ち悪い！」

と思ったりする。「面倒くさいなオイ！」と思うかもしれないが、神経質とはそういうもの。
そこで、神経質な私が納得した方法がこれ。

😊 「外部からイベントを引っ張ってくる」クラスを作る

この場合だと

  • 「Player の中にあるもの」からは、「Player の中にあるもの」しか見えない。
  • 「System の中にあるもの」からは、「System の中にあるもの」と「Player の中にあるもの」が見える。

そこで、
System 側に、Player内のイベントを引っ張ってくる専用のクラスを作る。

Player内のイベントをSystem内に引っ張ってくる
Player内のイベントをSystem内に引っ張ってくる

こうすることで、先程の

「イベントの購読元クラスが同じ」イベント処理ごとにクラスを分ける

というルールを適用できる。

❌ 問題点③ クラス名が非常に長くなった。

「クラス名は誤解を招かない方が良い」から、長いクラス名を付ける。

👆 解決策①「細分化をしすぎてないか」をチェックしてみる。

今回の例では、クラスを細分化しすぎたために、クラスの役割が

「キャラクターのMPが0になったときに失敗アニメーションを再生する」

という、かなり特殊化されたものになってしまった。

クラスの命名は、そのクラスの責務を表現するものであるため、「クラス名は誤解を招かない方が良い」というのは間違ってない。

つまり、命名が

class EmptyMagicPointFailedAnimationPlayer

となってしまったのは、起こるべくして起こったものと言える。

そのため、「クラス名が長くなってしまう！」という場合は、そのクラスが細分化されすぎていないかをチェックしてみよう。

👆 解決策② 名前空間を活用してみる。

名前空間は、その中にあるクラスや関数のコンテキスト、つまり「前提」を表す。
つまり、「クラス名に含めていた情報の一部」を、名前空間に含めてしまえばいいのだ。

今回の例では、「ゲームのプレイヤー」についての処理を記述している。
そのため、Game.Playerという名前空間を作れば、クラスの命名から「Player」の文字を省くことができ、その分他の意味を含めることができる。

しかし、このようなことをすると、私のような神経質な人間はむず痒くなる部分がある。
それらを私なりに克服したので、その時の考え方や対処法を書く。

🤮 1.名前空間の名前とクラス名が衝突する！

namespace Player;
public class Player { /* something */ }

名前空間Playerの外からPlayerクラスを利用する場合、Player.Playerとする必要があってなんだか気持ち悪い、という問題。
私は、名前空間を複数形にすることで解決している。

namespace Players;
public class Player { /* something */ }

🤮 2.他の名前空間にあるクラス名と衝突する！

namespace Players;
public class AnimationPlayer { /* something */ }

namespace Enemies;
public class AnimationPlayer { /* something */ }

名前空間Players Enemiesの「外」から、クラスAnimationPlayerを使用する場合、クラス名が衝突するのが気持ち悪い、という問題。
「使うときは Players.AnimationPlayer Enemies.AnimationPlayer と書けばいいじゃん！」と思うかもしれないが、それが気持ち悪い。

まず解決法の一つとして、使用する側で型名にエイリアスを使用する方法がある。

using PlayerAnimationPlayer = Players.AnimationPlayer;
using EnemyAnimationPlayer = Enemies.AnimationPlayer;

正直、当時の私はこれではまだ納得がいかない。
そこで、

  • 名前空間内でしか使わないプライベートなクラスは、クラス名から名前空間の情報を除く。
  • 名前空間外で使われるパブリックなクラスは、クラス名にも名前空間の情報を含める。
  • 名前空間外で使われるパブリックなクラスは、クラスは、なるべく少なくする。
    そう、「密凝集・疎結合」 さ！

という対応をして、解決した。
特に最後のが重要。名前空間内でも「密凝集・疎結合」を意識する。
こうすることで、冗長なクラス名を記述する回数が最小限で済む。

🤮 3.エディタの補完機能が使いづらくなる！

先程のように Players.AnimationPlayer と Enemies.AnimationPlayer を定義していたとする。

Players の名前空間にあるコードを編集しているときに、 Players.AnimationPlayer を使用するため、「Anim」まで入力した。
すると、エディタの補完機能が働いて、クラス名の候補がいくつか出てきた。

AnimationPlayer
AnimationPlayer

ｷﾞｬｰｯ！！同じクラス名が並んでる！！気持ち悪い！！
という問題。（同じ気持ちになった頃がある人がいれば良いんだけど......）

一回補完機能を切ってみよう。 設定を開いて、機能オフ。
補完機能がなくなった今、コンパイラだけが正義。

そう、補完機能はあくまで 「こんな候補があるんだけど、どうかな？」と聞いてるだけに過ぎない。
さらに、補完機能はエディタ固有の機能であり、それによってコードが影響を受けるというのはおかしな話である。

この問題に関しては、残念ながら具体的な解決法は提案できない。
しかし、私はこれを頭の片隅においておくだけでも、結構役に立っているので共有しておく。

改善後の実装

変数1個だけのコンポーネント分割

Before

// 正の整数を保持するコンポーネント
public class PositiveIntegerHolder : MonoBehaviour {
    [SerializeField] private int value = 0;
    public int Value {
        get { return this.value; }
	set { this.value = Math.Max(0, value); }
    }
}

After

public readonly struct PositiveInteger {
    public readonly int value;
    public PositiveInteger(int value) {
        this.value = Math.Max(0, value);
    }
}

1つのイベント処理だけが記述されたコンポーネント

Before

public class EmptyMagicPointFailedAnimationPlayer : MonoBehaviour {
    // キャラクターの各パラメータを保持しているオブジェクト
    [SerializeField] private CharacterParamsHolder paramsHolder;
    // キャラクターの各アニメーションを再生するオブジェクト
    [SerializeField] private CharacterAnimatorPlayer animatorPlayer;
    
    public void Start() {
        // MPが0になったときに、失敗アニメーションを再生する
        paramsHolder
	    .OnMagicPointEmpty
	    .Subscribe(_ => animatorPlayer.PlayMagicFailedAnimation());
    }
}

After

namespace Players;

public class AnimationPresenter {
    private readonly Player player;
    private readonly AnimationPlayer animationPlayer;

    public AnimationPresenter(Player player, AnimationPlayer animationPlayer) {
        this.player = player;
	this.animationPlayer = animationPlayer;
    }
    
    public void Start() {
        // MPが0になったときに、失敗アニメーションを再生する
        player
	    .OnMagicPointEmpty
	    .Subscribe(_ => animationPlayer.PlayMagicFailed());
        /* アニメーションを使用する他のイベント処理 */
    }
}

※ネストを増やしたくないので C# 10 の機能 file-scoped namespace を使用しています。

まとめ

「共通化できる部分がある！」と思ったときは...

  1. 本当に共通化する必要があるか、考えてみよう！
  2. 共通化の方法を、適切に選ぼう！

「イベント処理が上手くまとめられない！」と思ったときは...

  1. 「イベントの発行元」ごとにクラスを分けよう！
  2. 依存関係で 1. が適用できないときは、「外部からイベントを引っ張ってくるクラス」を作ろう！

「クラス名が長くなってしまう！」と思ったときは...

  1. クラスを細分化しすぎてないか、確認してみよう！
  2. 名前空間を活用しよう！

さいごに

この記事では、「細分化しすぎ」なケースについて書いた。
みんなも、同じような経験はあったかな。
設計に悩んだとき、頭の片隅でもいいから、この内容を思い出してくれると嬉しい。

さて、次はどのケースを書こうかな。

Discussion

フシハラフシハラ

処理をクラスに切り出ししまくったらコンストラクタの引数が無限に増えるけどこれ絶対間違ってるだろ…と悩んでいたらこの記事にたどり着きました。
参考にさせて頂きます！