[依存性逆転の原則失敗]ロジックを抽象化したけど抽象化しきれていなかった
みなさんご存知のSOLIDの原則に依存性逆転の原則というものがありますね。
具体に依存せず抽象に依存しようねみたいな考え方のやつです。
今回意識的に処理を抽象化したのですが、なんかあんまいい感じじゃなかったのでその話をしようと思います。
実装内容
「Hogeというデータが外部システムAに連携済かどうかを判定する機能」を実装していました。
判定ロジックは現時点で確実なものではなく、今後変更される可能性の高いものでした。
「変更される可能性高いならまあ抽象化しとくか〜」といった気持ちで実装してみました🐈
やったことは以下です。
- 変更される可能性の高いロジックをインタフェース化
- ロジックに必要な要素を値オブジェクト化
実装内容は以下です。
// 呼び出し元のクラス
class getUrlService
{
public function __construct(
// 依存性注入⭐️
private HogeCooperationCheckerInterface $hoge_cooperation_checker
) {
}
// 呼び出し元のメソッド
// 外部システムAに連携しているかどうかをチェックして、URLを返却するメソッド
public function getUrl(HogeModel $hoge_model): string
{
// 判定に必要な項目のみを持つ値オブジェクトを生成
$cooperation_state = new CooperationState(
$hoge_model->getCooperateOnceFlag(),
$hoge_model->getHogetatus(),
$hoge_model->getWebPublishedAgainstStatus()
);
// 外部システムAに連携しているかどうかの判定
if (!$this->hoge_cooperation_checker->isCooperated($cooperation_state)) {
// 連携済でない場合は空文字を返す
return '';
}
// 後続処理が続く...
}
}
// 外部システムAに連携済かどうかをチェックするインフェース
interface HogeCooperationCheckerInterface
{
public function isCooperated(CooperationState $cooperation_state): bool;
}
// HogeCooperationCheckerInterfaceを実装した、判定ロジックの具体を持つクラス
readonly class HogeCooperationChecker implements HogeCooperationCheckerInterface
{
public function isCooperated(CooperationState $cooperation_state): bool
{
// 判定ロジックがここに入る
}
}
// 外部システムAに連携しているかどうかの判定に必要な項目のみを持つ値オブジェクト
readonly class CooperationState
{
public function __construct(
private ?int $cooperate_once_flag,
private ?int $hoge_status,
private ?int $web_published_against_status
) {
}
// 以降に各プロパティのgetterが定義される...
}
意識した点
- 単体テストしやすくしたい
- HogeModelはクソデカクラスだったので、インスタンス生成やモック生成などのデータ準備がかなり手間になります。
- そうならないように必要な項目だけを持つ値オブジェクトを作成しました。
- 無事単体テストを簡単に書くことができました✌️
- 判定ロジックは変わる可能性があるので、抽象化しておきたい
- 呼び出し元は具象クラスではなくインターフェースに依存する形にしました。
微妙だった点
ロジックに変更があった場合、呼び出し元も変更必要にならない？
「外部システムAに連携済かどうかの判定ロジック」に変更が入った場合、判定に必要な要素が変わる可能性も高いと思うんですよね。
今の実装だと、呼び出し元(getUrlService)で判定用の値オブジェクト(CooperationState)を生成しているため、呼び出し元の変更も必要になってしまいます。
意味ないじゃん！
インターフェースした意味ある？
今時点ではインターフェース化した利点はわからなかった。
他の実装案を考える
値オブジェクトを生成するファクトリーを追加する
判定に必要な要素も具体の実装の一部です。
これも一緒に抽象化しないと意味がないですね。
HogeModelを渡してCooperationStateを返すメソッドを持つクラスがあれば、呼び出し元から具体のロジックが完全になくなりそうです。
// HogeCooperationStateを生成するファクトリークラス
class HogeCooperationStateFactory
{
public static function createFromHogeModel(HogeModel $hoge_model): HogeCooperationState
{
return new CooperationState(
$hoge_model->getCooperateOnceFlag(),
$hoge_model->getHogetatus(),
$hoge_model->getWebPublishedAgainstStatus()
);
}
}
// 呼び出し元のクラス
class getUrlService
{
public function __construct(
private HogeCooperationCheckerInterface $hoge_cooperation_checker
) {
}
public function getUrl(HogeModel $hoge_model): string
{
// ファクトリーを使って$cooperation_stateを生成
$cooperation_state = HogeCooperationStateFactory::createFromHogeModel($hoge_model);
if (!$this->hoge_cooperation_checker->isCooperated($cooperation_state)) {
// 連携済でない場合は空文字を返す
return '';
}
// 後続処理が続く...
}
}
抽象化せずそのまま直で実装する
今回実装した「Hogeというデータが外部システムAに連携済かどうかの判定」は、複雑なロジックではなかったので、直で実装するのも良いと思います。
具体のロジックが直で書いてあるので読みやすく、変更ファイル数を少量にとどめることができます。
システムの規模や状況によっては、こういった単純な実装も選択肢としてはありだと思います( ◠‿◠ )
ただし単体テストを書くのが少々手間になるデメリットはあります。
class getUrlService
{
public function __construct(
) {
}
public function getUrl(HogeModel $hoge_model): string
{
if (!$this->isCooperated($hoge_model)) {
// 連携済でない場合は空文字を返す
return '';
}
// 後続処理が続く...
}
private function isCooperated(HogeModel $hoge_model): bool
{
// 判定ロジックがここに入る
}
}
感想
ロジックが変更されたときに、どのファイルのどこを修正する必要が出てくるか？を考えながら実装したいと思ったり思わなかったりしました。
あと抽象化って普通に手間なので、やるべきところとそうでないところのめりはり？をつけないと冗長コードになりそう。(めりはり？)
YAGNIとバランスとりながらいい感じに実装してきましょうね🐈
Discussion