Zenn
Open60

設計の知見や勉強

C#
設計
MakihiroMakihiro

Martinの定義

拡張に対して開いている

モジュールの振舞いを拡張できることを意味する。
アプリケーションの要求が変化したら、それらの変更内容を満たす新しい振舞いでモジュールを拡張することが可能である。言い換えれば、モジュールが実行することを変更できるのである。

変更に対して閉じている

モジュールの振舞いを拡張した結果として、モジュールのソースやバイナリコードで変更が発生しない。モジュールのバイナリコードは、リンク可能なライブラリなのか、DLLなのか、Javaの.jarなのかにかかわらず、変更されないままとなる。

MakihiroMakihiro

まぁC#のインターフェースがやっていることで合っていると思う

MakihiroMakihiro

昔作った重み付きランダムライブラリでも、外部からアルゴリズムを追加したりして拡張できるけど、それを使っているコードは壊れないようになっているよー
https://github.com/mackysoft/Choice/blob/main/Assets/MackySoft/MackySoft.Choice/Runtime/WeightedSelectMethod/IWeightedSelectMethod.cs

これ、外からはenumの感覚でアルゴリズムを選べる記法にするためにやったんだけど、実はgetterが呼ばれるたびにnewが呼ばれるからヤバいんだよな。

新しいインスタンスが生成されるのは意図した挙動なんだけど、getterが呼ばれるたびにGCが発生するのがヤバい。反省反省

MakihiroMakihiro

開発者は、既存のモジュールのソースコード、またはコンパイル済みのアセンブリを編集せずに、新しい機能をサポートしなければなりません。

とあるライブラリを拡張したくてソースコードを見漁ったけど、結局コードを直接いじるしか拡張する方法が無かったのを思い出した。

C#であれば基本的に、インターフェースや抽象クラスを定義しておけば外部から拡張できるので、どんどんやっていこう

MakihiroMakihiro

「変更に対して閉じている」ルールに対するさらに寛大な例外は、そのコードのクライアント側を変更する必要が無い限り、既存のコードに対する変更をすべて認める、というものです。

あるクラスを変更したら別のクラスを変更せざるを得ない場合、それら２つのクラスは「密結合」されています。
逆に、あるクラスを変更しても他のクラスを変更する必要が無い場合、それら２つのクラスは「疎結合」されています。

上のIWeightedSelectMethodで言うと、「IWeightedSelectMethodを実装したアルゴリズムのSelectIndex戻り値が規格化されていれば、IWeightedSelectMethod.SelectIndexを呼んだ場所自体を変更しなくても大丈夫だから疎結合だよねー」的な

MakihiroMakihiro

仮想メソッドかー

virtual使いたくない理由として、いとも容易く前後の条件をぶち壊すことができるというのがあって、

virtualをオーバーライドするときは、まず最初にここら辺を知る必要がある。

  • base.VirtualMethodを呼ぶ必要があるかどうか
  • base.VirtualMethodは関数の最初で呼び出すべきか、最後に呼び出すべきか、それとももっと別のタイミングか
public virtual void Execute () {
    // 前処理（overrideした時に消すことができてしまう）
    PreExecute();

    // 何かの処理
}

しかし、このExecuteが仮に「PreExecuteを必ず呼ぶ必要がある」というものだった場合に、overrideした場所でbase.PreExecuteを呼ばなかったらこの前提条件を簡単に外部から破壊できてしまう。

人間に優しくない。

なので、こういうことをしたい場合は抽象メソッドを使って以下のように実装すれば、外部から条件を破壊されずに済む。

public void Execute () {
    PreExecute();
    ExecuteCore();
}

protected abstract void ExecuteCore () {

}
MakihiroMakihiro

個人的には「必ずしも実装する必要のないコールバック関数を実装するとき」ぐらいしかvirtualは使っていないなー

そういうシチュエーションも大体、インターフェースか抽象メソッドでどうにかなる案件だったりするので、virtualを使う機会ってほとんどない

MakihiroMakihiro

継承を意図して設計、文書化する。そうでなければ、継承を禁止する。

クラスの継承は何かとやっかいです。新しいサブクラスによって既存のコードが予測不可能な方法で破壊されるかもしれません。

積極的にsealed使っていこうぜ～って話

MakihiroMakihiro

C#って、暗黙的なsealedにして、明示的に継承可能にする方が良かったんじゃないかって気がするな

MakihiroMakihiro

開放・閉鎖の原則をいつどこで適用するか

これ難しいよなー
いたるところに拡張ポイントを作っちゃうと、逆に生産性が下がる

MakihiroMakihiro

予想されるバリエーション

予想されるバリエーションのポイントを特定し、それらのまわりに安定したインターフェースを作成する。

どこで仕様が変更されるかをできる限り予測する、または変更可能性ポイントを聞き出しておくなどして、そこを拡張ポイントにしようぜーってこと

僕のライブラリを例に出すと、

「線形アルゴリズムだけじゃなくて、二分探索アルゴリズムも使いたいよね。なんなら未知のアルゴリズムを他の人が実装するかもしれないよね」

ってなって、アルゴリズムを実装するためのインターフェースを作る感じ

MakihiroMakihiro

安定したインターフェース

インターフェースが変化した場合は、クライアントも変化しなければなりません。インターフェースに依存することの主な利点は、インターフェースの方が実装よりもずっと変化しにくいことです。

インターフェースに依存することになるので、作ったインターフェースはもちろん変化せずに安定してることが大事。インターフェースはちゃんと設計しないとね！

まぁそれが難しいんだな！

MakihiroMakihiro

ほどよい適応力

コードにも「ゴルディロックスゾーン」が存在します。ここでいうゴルディロックスゾーンは、適度な量の拡張ポイントが（適切な場所に）含まれているコードのことです。

やたらと拡張性を高くしたりすると複雑になって管理が大変になるよね。

ちなみにゴルディロックスはおとぎ話のヒロインらしい
https://www.fineza-col.com/product/1470

MakihiroMakihiro
  • 「予想されるバリエーション」を見極めたうえで、変更可能性が限りなく低い
  • 拡張ポイントを全然使わない

こうなったら手間がかかるし、可読性も低くなるしで、神オブジェクトが正しい場合もあるよねーという話

MakihiroMakihiro

ガイドラインとしての「予想されるバリエーション」と「投機的な一般化」

予想されるバリエーション

何が拡張出来て何が拡張できないのかを明確にすべきであることを意味します。

abstractやsealedのこと

投機的な一般化

クラスが一般的な問題に適用されることを見越して、漏れのある抽象化を作成しないように注意すべきであることを意味します。

ちょっとよく分からんから保留

MakihiroMakihiro

多分、「やたらと抽象化するなよ～」ってことだよな

理想的には、設計上の選択肢はすべて明示的な意図をもって行うべきです

virtualは暗黙的になってしまうから、基本的には避けていきたいところ

MakihiroMakihiro

「開放・閉鎖の原則」終わり。

virtualの話で「リスコフの置換原則」が頭に過ったので、次はそれをやる

MakihiroMakihiro

リスコフの置換原則のルールに従っていれば、クラス階層が変更されても、クライアントがそれを認識せずに済みます。
インターフェースが変更されない限り、既存のコードを変更する理由は何もないはずです。

インターフェースの変更以外を理由にコードを変更しなければならないなら、実装に対して「密結合」しているわけだから抽象化できてないよねー

MakihiroMakihiro

SがTの派生型であるとすれば、T型のオブジェクトをS型のオブジェクトと置き換えたとしても、プログラムは動作し続けるはずである。

「基底型のオブジェクトを、派生型のオブジェクトに置き換えても動くはずだよね」

というのに加えて、

「インターフェースを実装したオブジェクトを、そのインターフェースを実装した別のオブジェクトに置き換えても動作するよね」

という意味でもある。

MakihiroMakihiro

具体的にどの派生型を呼び出しているのかをクライアントが知るべきではなく、知る必要もありません。クライアントの振舞いは、与えられた派生型のインスタンスに関係なく、同じでなければなりません。

「基底型を知っていれば派生型を知っている必要はない」って話だけど、継承はあんまり使わないからインターフェースに置き換えて読んだ方が分かりやすいかもしれん

「インターフェースを知っていれば、実装を知っている必要はないよねー」という話になる

MakihiroMakihiro

コントラクトのルール

  • 事前条件を派生型で強化することはできない。
  • 事後条件を派生型で緩和することはできない。
  • 基底型の不変条件（常に満たされなければならない条件）は派生型でも維持されなければならない。

個人的にvirtualを使いたくない理由はまさにこれで、「リスコフの置換原則」を破壊できてしまうからなんよね

MakihiroMakihiro

僕のライブラリで例を挙げると、プールの実装かな。

プールでインスタンスを生成するFactoryは抽象メソッドなので拡張可能だけど、ファクトリーから生成されたインスタンスは必ずnullチェックが行われるようになっている。

つまり事後条件を派生型で緩和できない。

public T Rent () {
	T instance;
	if (m_Pool.Count > 0) {
		instance = m_Pool.Dequeue();
	}
	else {
		// 事後条件はファクトリから生成されたインスタンスがnullではないこと
		instance = Factory() ?? throw Error.FactoryMustReturnNotNull();
	}

	OnRent(instance);
	return instance;
}

protected abstract T Factory ();

virtualでもないので事後条件を壊せない。我ながらよくできている！

ザックリ言えば、これのパターンやね
https://zenn.dev/link/comments/dd2e4f92b0f446

MakihiroMakihiro

変性のルール

  • 派生型のメソッドの引数には反変性がなければならない。
  • 派生型の戻り値の型には共変性がなければならない。
  • 既存の例外階層に含まれていない新しい例外を派生型からスローすることはできない。

用語がちょっと難しくなった。

反変性は「引数の型がインターフェースだった場合に、そのインターフェースを実装した型を渡せる」
共変性は「インターフェースを返す関数で、そのインターフェースを実装した型を返せる」

ということだと認識してる。

MakihiroMakihiro

もしかしたら例外を強化するのはやってしまってるかもしれない

MakihiroMakihiro

開発者はよく、インターフェースに対してプログラムすべきであると言われます。
それに関連して、コントラクト（契約）に対してプログラムするという表現もあります。
ただし、メソッドのシグネチャ以外にインターフェースが伝えるものと言えば、コントラクトのかなり大まかな主旨だけです。

「シグネチャって何だ？」ということで調べました。

プログラミングの分野では、関数やメソッドの名前、引数の数やデータ型、返り値の型などの組み合わせのことをシグネチャという。
https://e-words.jp/w/シグネチャ.html

関数を外から見て、それが何をするのか得るための情報のことね

MakihiroMakihiro

このメソッドのクライアントは、float型でありさえすれば、負の値でもなんでも有効だろうと推測するかもしれません。このメソッドのコントラクトで、0よりも大きい重量値を共用すべきです。したがって、このメソッドは事前条件を実装しなければなりません。

どうやって事前条件を設定するか。思いつくのは２つかなー

  • ifで例外を投げる
  • 型で絞る
MakihiroMakihiro

事前条件

事前条件は、「メソッドを失敗させずに確実に実行するために必要なすべての条件」として定義されます。

ガード句（ifからの例外）

当然ながら、事前条件はクライアントからアクセスできる必要がある。

MakihiroMakihiro

事後条件

事後条件は、メソッドの終了時にオブジェクトが有効な状態のままであるかどうかをチェックするための条件です。

事前条件と同じく、ガード句で実装できる。
ただしオブジェクトの変更等が行われた後、メソッドの最後で行う必要がある。（ガード句のあとで変更しちゃダメだよねー）

MakihiroMakihiro

僕のライブラリで例を挙げると、UnityObject用のプール。
これは事後条件が少し危ない実装になっている。

public T Rent () {
	T instance = GetPooledInstance();
	if (instance == null) {
		instance = UnityObject.Instantiate(m_Original);
		OnCreate(instance);
	}
	OnRent(instance);
	return instance;
}

protected abstract void OnCreate (T instance);

protected abstract void OnRent (T instance);

プールから返されるオブジェクトは当然nullであってはいけないんだけど、抽象メソッドのOnCreateとOnRentの中でDestroyをしてしまえば、UnityObjectだからnullを返すことができてしまうんだよね。

具体的に言うと、「拡張ポイントであるコールバック関数の中で事前条件を強化して、事前条件にそぐわなければDestroyで破棄する」ということができてしまう。勿論そのような使い方は想定していないのでNG。既存のコードが動かなくなる。

nullチェックしてInstantiateしてるから絶対にnullでないオブジェクトを返しているつもりだったんだけど、詰めが甘かったなー

今回の場合であれば、OnRentの後にガード句を配置しないといけない。あとで修正する！

MakihiroMakihiro

データ不変条件

データ不変条件とは、オブジェクトのライフタイムにわたって変化しない述語のことです。
データ不変条件は、オブジェクトが作成された時点で満たされ、オブジェクトがスコープを外れるまでその状態が維持されなければなりません。

コンストラクタでもガード句どんどん使おうぜーであったり、プロパティのsetterでのガード句であったりの話

DIで似たような話になったな。コンストラクタで完全に条件を満たしておいて、あとはreadonlyにしておくと良い。

MakihiroMakihiro

よく使う事前条件は、値オブジェクトなどにするとミスの防止にもなる

MakihiroMakihiro

ifで例外を投げる（ガード句）

メソッドの事前条件

型で絞る（カプセル化）

型の不変条件

これを的確に判断できると良さそう

MakihiroMakihiro

事前条件は強化できない

クライアントがサブクラスのどのメソッドでも最も厳格な事前条件コンストラクタが定義されると想定していた場合、事前条件を強化すると、クライアントのコードが動作しなくなるおそれがあります。

まぁ抽象化しているのに派生先を知っている必要があって、それはもはや「密結合」になってしまうんだよね。

MakihiroMakihiro

どの型を操作しているのかをクライアントコードに推測させるべきではありません。それを許してしまうと、クラスどうしが強く結びついてしまい、要求の変化に適応できなくなってしまいます。

同じインターフェースを使っているのに、その派生先の実装次第でエラーが出たり出なかったりするの怖い

MakihiroMakihiro

事後条件は緩和できない

新しいサブクラスが作成されたときに既存のクライアントが動作しなくなる恐れがあることです。理論的には、リスコフの置換原則に準拠していれば、新たに作成されたサブクラスを既存のすべてのクライアントで使用できるはずであり、思わぬ方法で失敗することはないはずです。

事前条件と同じで、派生先を知っている必要が出てきてしまうんだよね

MakihiroMakihiro

不変条件は維持しなければならない

継承からのカプセル化でprotectedメンバをいじれるようにしたりすると不変条件が壊れちゃうからやめろよーって話

MakihiroMakihiro

「リスコフの置換原則」終わり

SOLID原則の話になったので、次は「単一責務の原則」をやる

MakihiroMakihiro

クラスを変更する理由が１つではないとしたら、そのクラスには複数の責務が割り当てられています。
複数の責務を持つクラスは、複数のより小さなクラスに分割すべきです。そして、分割されたクラスがそれぞれ責務と変更する理由を１つだけ持つようにします。

責務の粒度、難しいんだよなー

コンポーネント指向を徹底的に行った結果、１つのオブジェクトにアタッチするコンポーネントが多すぎて生産性が下がったのを思い出した。

MakihiroMakihiro

抽象化を行わなければ、無数の要求がビッグ・ボール・オブ・マッド、すなわち「大きな泥だんご」と化し、責務がほとんど描写されず、抽象化がまったく見当たらないクラスやアセンブリが作成されます。
結果として、ユニットテストが無く、保守や拡張が難しく、それでいて業務に必要不可欠なアプリケーションが出来上がってしまいます。

「リスコフの置換原則」の話を先にやってしまったから、だよねーって共感するぐらいしか無くなってしまっている

MakihiroMakihiro

単純に一つの関数を複数の関数を分けるだけではなく、インターフェースとして分離すること大事

MakihiroMakihiro

インターフェース（とそれらの実装）として抽象化するプロセスは、再帰的なプロセスです。各クラスを調べるときに、そのクラスの責務を洗い出し、そのクラスの責務が１つだけになるまで繰り返しリファクタリングする必要があります。

再帰的なプロセスかー、確かに

責務の粒度はこのプロセスで決まっていくね

MakihiroMakihiro

「単一責務の原則」終わり。
「インターフェース分離の原則」やる。

MakihiroMakihiro

インターフェース分離の原則は、インターフェースをより小さくすべきであることを表す原則です。
インターフェースのどのクライアントでもすべてのメンバーが必要になる場合を除いて、そうした大きな契約を満たすことをすべての実装に要求するのは合理的ではありません。

メンバーが必要なものと必要じゃないものに分かれてしまうなら、インターフェースを分けた方がいいよね

MakihiroMakihiro

そういえば重み付きランダムのアルゴリズムを実装するインターフェース、アルゴリズムと状態の実装を同じ場所に書く必要があるのは本当に良くなかった。

public interface IWeightedSelectMethod {
	int SelectIndex (TemporaryArray<float> weights,float value);
	void Calculate (TemporaryArray<float> weights);
}

アルゴリズムはシングルトンでもいいのに、状態は各インスタンスが必要だからここは分離するべきだった。

MakihiroMakihiro

AOPの例を見てて思ったけど、デコレーターはFluidで書けるようにしたいよねって感想がある。

hogehoge.
    .HogeDecorator()
    .FugaDecorator();
MakihiroMakihiro

業務でasync/await使わずにコルーチンを使わないといけないプロジェクトがちょくちょくあるのだけど、具体的になんで嫌なのかが分かった

MakihiroMakihiro

コルーチンで返り値を返そうとする場合、クロージャを使うことになるのだけど、

Hoge hoge = null;
yield return HogeFunc(x => hoge = x);

Debug.Log(hoge != null);

「hogeに対して返り値が代入されること」がコンパイル時点で保証されていないんや
タイミングがHogeFunc依存になっていて、結合性が無駄に高くなる

MakihiroMakihiro

もう最近はChatGPTが碌なコード出力を行わなくなってきたので、GitHub Copilotの方が仕事してる

MakihiroMakihiro

わぁ、AddressablesでIncludeInBuildを無効にした状態でSpriteAtlas使う場合、少し面倒なやり方をしないといけないんだな。SpriteAtlasManager.atlasRequestedで解決しないと、Addressables.LoadAssetAsyncでSpriteを読み込もうとしても、警告が出てデフォルトの白い画像が表示される。（事前にLoadAssetAsyncでSpriteAtlasをプリロードしても、atlasRequestedで解決しないと正しく読み込めない）

SpriteAtlasManager.atlasRequested += OnAtlasRequested;

void OnAtlasRequested (string tag, System.Action<SpriteAtlas> callback)
{
	OnAtlasRequestedCore(tag,callback).Forget();
}

async UniTaskVoid OnAtlasRequestedCore (string tag, System.Action<SpriteAtlas> callback)
{
	Debug.Log($"[{nameof(CommonInitializer)}] OnAtlasRequested: {tag}");
	var handle = Addressables.LoadAssetAsync<SpriteAtlas>(tag);
	await handle.ToUniTask();

	if (handle.Status == AsyncOperationStatus.Succeeded)
	{
		callback(handle.Result);
	}
}