はじめに
前回、社内勉強会にて上記の記事を発表させてもらい、
後日コメントで、
「次は、SOLID原則（要検索）の理解を目指すと、良い感じ」といただいたので
気になり、このテーマにした背景です。
またその原則の中でも
特に、オープン・クローズドの原則が
SOLIDの原則が目指している
「良い設計」をもっともよく反映している原則とあったので
深ぼって勉強してみました。
また今回の記事はほぼこちらの動画を参考に
文字を起こし、ご理解のほどお願いします🙇♂️
登壇者：後藤 秀宣（@hidenorigoto）さん
SOLIDの原則とは
SOLID原則とは、オブジェクト指向プログラミングにおいて、変更しやすい・理解しやすい・再利用しやすいモジュール（クラスや関数などの、ソフトウェアのを構成する部品）を設計・開発するための原則のことです。ソフトウェアエンジニアRobert C. Martinに提唱された多くの設計原則を5つにまとめたものの頭文字をとって命名されました。
S … Single Responsibility Principle: 単一責任の原則
O … Open-Closed Principle: 開放閉鎖の原則
L … Liskov Substitution Principle: リスコフの置換原則
I … Interface Segregation Principle: インターフェイス分離の原則
D … Dependency Inversion Principle: 依存性逆転の原則
なぜSOLIDの原則
提唱されたのは、1990年代後半〜2000年代くらい。（ロバート・C・マーチン）
→ちょっと古臭い。目新しさみたいなのはない。
・ひと昔に流行っただけの技術ではない
・今のソフトウェア開発においても、とてもよく効果を発揮する設計原則
・PHPの言語機能で普通に使える
・息の長い技術を身につけると、お得💰
なぜオープン・クローズドの原則
SOLIDの原則が目指している
「良い設計」をもっともよく反映している原則だから。
どこにでもある、ソフトウェア開発チームのある日のお話
以下から、どこにでもあるような、ソフトウェア開発チームのある日のお話をします。
登場人物
・後輩
そろそろ新人ではなくなってきた。
コード設計についてもっと強くなりたい。
・先輩
設計がちょっと出来る。
後輩：「先輩お久しぶりっす！後輩っす！」
先輩：「お、後輩くん、お久しぶり。」
こんな風に2人が会話しています。
この後輩がとある機能の実装を担当することになりました。
後輩が担当する機能
・TODOアプリ
・TODO情報の出力機能（コマンド）を実装する
・出力は、人間が読むためのテキストファイル（フォーマットは別途）
・通常のTODOと、外部カレンダーから取り込んだ予定をTODOとして扱うカレンダーTODOとがある（フォーマットが異なる）
ユーザーから聞いた出力の要件
ユーザーの説明によると以下のような要件でした。
通常TODO
・１つのtodoの表示は2行になっていて、1行目がタイトル行、2行目がURL行。
・タイトル行には、ステータスがあって、タイトルがある。
[未着手]〇〇について調査 ← タイトル行
https://example.com/todo/1 ← URL行
[完了]Aの実装
https://example.com/todo/1
カレンダーTODO
・１つのtodoの表示は2行になっていて、1行目がタイトル行、2行目がURL行。（共通）
・タイトル行には、未来の場合は日時が表示され、完了の場合は完了の表示。タイトルの右に場所の情報が追加されている。
[3/15 10時]B社訪問（B社）
https://example.com/todo/1
[完了]C仕様打ち合わせ（会議室）
https://example.com/todo/1
実装（全体像）
全体像はこんな感じです。
左上にあるのが、出力コマンド用のExportAllTodoCommandクラス
中央にあるのが、TODOの情報、カレンダーTODOの情報を保持するエンティティクラス
右上にあるのが、エンティティの情報を取り出すリポジトリである、TodoRepositoryクラス
TodoRepositoryクラスのfindAll()メソッドで通常のTODOの一覧とカレンダーTODOの一覧をマージして返すように設定しました。
後輩：「今回の処理のほとんどは、出力コマンド用のExportAllTodoCommandクラスに書いてあるっす！」
出力コマンドクラスExportAllTodoCommand
コンストラクターや初期化コードは省略しています
今回の処理の本体はexecuteメソッドの中に書かれています。
class ExportAllTodoCommand extends Command {
// コンストラクターや初期化コードは省略しています
// ...
// 実行コマンド
protected function execute(InputInterface $input, OutputInterface $output)
{
// コマンド引数で指定した出力先のパスを取得
$path = $input->getArgument('output-path');
// リポジトリからTODOの一覧を取得
$todoList = $this->todoRepository->findAll();
// そのTODOの一覧に対してarray_mapで何か処理をした後、ファイルに出力する文字列を作って、それを出力先のパスに書き出している。
$contentArray = array_map(function($todo)
{
// formatTitleLineっていうメソッドの結果を文字列に格納（タイトル行）
$buf = $this->formatTitleLine($todo);
// $todoのgetUrlの結果をその文字列に結合（URL行）
$buf .= $todo->getUrl();
return $buf;
}, $todoList);
$content = implode(PHP_EOL . '----' . PHP_EOL, $contentArray);
file_put_contents($path, $content);
$output->writeln($content);
}
}
後輩：「ユーザーさんがイメージしていることに合わせて、タイトル行のフォーマット、URL行のフォーマットという風にコードにしてみたっす！」
後輩：「タイトル行のフォーマットはコード量が多くなりそうなので、formatTitleLine()という別メソッドに切り出してあるっす！」
出力コマンドクラスのタイトル行用メソッドformatTitleLine()
なんだかごちゃっとしていますが、
カレンダーTODOの場合と、通常TODOの場合とで異なる部分を分岐させながら文字列を組み立てている。
// タイトル行フォーマットメソッド
private function formatTitleLine($todo): string
{
// カレンダーTODOの場合と、通常TODOの場合とで異なる部分を分岐させながら文字列を組み立てている
$now = new \DateTime();
if($todo instanceof CalendarTodo &&
($todo->getEndDateTime() === null || $todo->getEndDateTime() > $now)){
$status = '[' . $todo->getTargetDateTime()->format('n/j H時') . ']';
}else{
$statusLabel = [
'pending' => '未着手', 'working' => '進行中', 'complete' => '完了'
];
$status = '[' . $statusLabel[$todo->getStatus()] . ']';
}
return $status . $todo->getTitle()
. (($todo instanceof CalendarTodo) ? '(' . $todo->getPlace() . ')' : '');
}
後輩：「最初は、ステータスの部分の文字列を作ってるっす！カレンダーTODOで特定条件の場合は日時になるので、そのように分岐してるっす。」
後輩：「で、ステータス以外はそんなに複雑じゃないんで、returnのところでまとめたっす！！」
後輩：「しかし、あたらめて見ると、このメソッドはごちゃごちゃだな💦うまくやれてたつもりなんだけど・・・」
後輩くんが、こんな思いを抱くのには訳があります。実は後輩くんが以前のプロジェクトで担当した作業はまさに今回このformatTitleLineメソッドのように、なんだかごちゃっとしたコードの修正作業だったんですね。
そして、その時後輩くんは修正にすごく苦労した苦い思い出がありました。そういう苦労の元凶を今度は自分の手で作ってしまうっていうのは辛いですよね。。。
後輩：「これは先輩にアドバイスしてもらうしかっ！！！」
先輩に相談
先輩：「なるほどなるほど。状況を知りたいので要件のところから教えてください。」
後輩：「おっけーっす。ばっちりメモしてあるっす！」
【改めて】ユーザーから聞いた出力の要件【を確認】
通常TODO
・１つのtodoの表示は2行になっていて、1行目がタイトル行、2行目がURL行。
・タイトル行には、ステータスがあって、タイトルがある。
[未着手]〇〇について調査 ← タイトル行
https://example.com/todo/1 ← URL行
[完了]Aの実装
https://example.com/todo/1
カレンダーTODO
・１つのtodoの表示は2行になっていて、1行目がタイトル行、2行目がURL行。（共通）
・タイトル行には、未来の場合は日時が表示され、完了の場合は完了の表示。タイトルの右に場所の情報が追加されている。
[3/15 10時]B社訪問（B社）
https://example.com/todo/1
[完了]C仕様打ち合わせ（会議室）
https://example.com/todo/1
後輩：「通常のTODOとカレンダーTODOがあるんすよ。で、どちらもタイトル行、URL行というふうに出力する感じっす。タイトル行の出力の中見が、通常とカレンダーとで少し違うみたいっす。」
先輩：「ふむふむ。丁寧なメモでいいですね。」
【改めて】実装（全体像）【を確認】
後輩：「全体の構造はこんな感じっす。」
先輩：「オーソドックスな全体構造で良いですね。」
後輩：「このあたりは上手くできてる気がするっす！問題はここからなんすよ。コマンドクラスのコードを見てほしいっす！」
【改めて】コマンドクラス【を確認】
class ExportAllTodoCommand extends Command {
// コンストラクターや初期化コードは省略しています
// ...
// 実行コマンド
protected function execute(InputInterface $input, OutputInterface $output)
{
// コマンド引数で指定した出力先のパスを取得
$path = $input->getArgument('output-path');
// リポジトリからTODOの一覧を取得
$todoList = $this->todoRepository->findAll();
// そのTODOの一覧の、、に対してarray_mapで何か処理をした後、ファイルに出力する文字列を作って、それを出力先のパスに書き出している。
$contentArray = array_map(function($todo)
{
// formatTitleLineっていうメソッドの結果を文字列に格納（タイトル行）
$buf = $this->formatTitleLine($todo);
// $todoのgetUrlの結果をその文字列に結合（URL行）
$buf .= $todo->getUrl();
return $buf;
}, $todoList);
$content = implode(PHP_EOL . '----' . PHP_EOL, $contentArray);
file_put_contents($path, $content);
$output->writeln($content);
}
}
先輩：（何かに気づいたように、眼光が鋭くなった）
「おっ、なるほどー。以下のあたりがポイントになりそうですね。」
// そのTODOの一覧の、、に対してarray_mapで何か処理をした後、ファイルに出力する文字列を作って、それを出力先のパスに書き出している。
$contentArray = array_map(function($todo)
{
// formatTitleLineっていうメソッドの結果を文字列に格納（タイトル行）
$buf = $this->formatTitleLine($todo);
// $todoのgetUrlの結果をその文字列に結合（URL行）
$buf .= $todo->getUrl();
return $buf;
}, $todoList);
$content = implode(PHP_EOL . '----' . PHP_EOL, $contentArray);
file_put_contents($path, $content);
後輩：「いやいや先輩、ここまでは結構キレイに書けてると思うんすよ！この先がダメなんすよ💦」
先輩：「お、おぅ、そうなんですね。では、そちらを見てみましょうか。」
【改めて】formatTitleLine()【を確認】
// タイトル行フォーマットメソッド
private function formatTitleLine($todo): string
{
// カレンダーTODOの場合と、通常TODOの場合とで異なる部分を分岐させながら文字列を組み立てている
$now = new \DateTime();
if($todo instanceof CalendarTodo &&
($todo->getEndDateTime() === null || $todo->getEndDateTime() > $now)){
$status = '[' . $todo->getTargetDateTime()->format('n/j H時') . ']';
}else{
$statusLabel = [
'pending' => '未着手', 'working' => '進行中', 'complete' => '完了'
];
$status = '[' . $statusLabel[$todo->getStatus()] . ']';
}
return $status . $todo->getTitle()
. (($todo instanceof CalendarTodo) ? '(' . $todo->getPlace() . ')' : '');
}
後輩：「ここっす！ユーザーの要件をコードにするって言うことを意識して書いてみたんすけど、間違ってる感がハンパないっす💦」
先輩：「あー、そういうことですね。状況はよく理解できました。ありがとう！確かにこのメソッドのコードは、困った感じになってますね。」
後輩：「ですよね。それでどうしたらいいんすかね💦」
先輩：「答えをゆってしまうと、このコードはバリエーションを捉えそこねているんですよ。」
後輩：「バリエーション・・・ 通常TODOとカレンダーTODOのところっすかね・・・。ぼやっとしか分からないっす。」
先輩：「そうですねー、これはコード設計を考える上でとても大事なことなんですよ。今日は時間がありますので、じっっっっくりお話しましょう！！！」
後輩：（先輩がノッテきたぞ）
変更要求に対する、コードの変化とバリエーション
今のコードの問題を見極めるために、
変更要求に対する、コードの変化とバリエーションについて考えてみましょう。
この雑な図は、後輩くんが書いたコードの構造を簡略に表した図です。
右側に、通常のTODOと、カレンダーTODOがあって、
それらをまとめて処理するコマンドクラスというのが左側にあります。
コマンドクラスのコードは、
array_mapでループ処理しながら個々のTODOを文字列にフォーマットするっていう部分と、
左下のさらにタイトル行をフォーマットするformatTitleLineメソッドという部分に分かれていました。
で、
今ここには明示的には表示していませんが、
このコードを書いた後輩くんの背後には、何らかのポリシーというか、隠れた構造、意図みたいなものがあります。
そのような背後に隠れたものを探るために
今から変更要求があった場合に、コードがどうなるのかを
４つのパターンに分けて検討してみます。
４つのパターンは以下です。
・通常TODOのタイトルのみ変更
・TODOの種類が増えた時
・出力フォーマットの行が増えた時
・別の出力フォーマットが増えた時
先輩：「それぞれのケースについて
・何らかのバリエーションの変化なのかどうか
・そのバリエーションはどう扱われているのか
も考えてみましょう。」
通常TODOのタイトルのみ変更
先輩：「通常TODOのフォーマットの処理は、ほとんどformatTitleLine()メソッドの中に書かれているので、変更箇所はこのメソッドの中身になりますね。」
後輩：「そうっすね。メソッドの中身がごちゃっとしちゃってるのであまり触りたくないっすけど、、、でもまあココだけなので、大変じゃないっす。」
先輩：「このケースでは、バリエーションの変化はなさそうですね。」
まとめ
通常TODOのタイトルのみ変化の場合は、
バリエーションの変化はなく、コードとしては単にformatTitleLineメソッドの中を変更する程度でした。
TODOの種類が増えた時
TODOが一種類増えています。
当然この新しいTODOの出力も行わなければなりません。
このコードを書いた後輩くんだったらどのあたりを触るでしょうか。
やはり、formatTitleLineメソッドです。
先輩：「新しいTODOのフォーマット次第ですが、おそらくformatTitleLine（）メソッドの中を、かなり書き換えないといけなさそうですね。」
後輩：「そうっすね・・・。２種類でもごちゃっとしてたのが、３種類になったらもうダメかもっす💦」
先輩：「まあ、今のコードだと大変そうですよね。ちなみにこのケースでは、TODOはすでにコードの中でバリエーションとして見出だされていました。 というのは、TODOの種類ごとにエンティティクラスが別々になっているからです。なので既知のバリエーションです。そこに追加が発生したというケースですね。」
まとめ
TODOの種類が増えた時の場合は、
既知のバリエーションに追加が発生し、コードとしてはタイトル行用のメソッドに結構大変な修正が必要でした。
出力フォーマットの行が増えた時
出力フォーマットは今の所、通常TODOもカレンダーTODOも、
タイトル行、URL行って言うふうに2行でした。
これに、どちらのTODOにも3行目の出力を追加する場合です。
先輩：「ところでこの【行】というのは、バリエーションなんでしょうか？」
後輩：「クラスにはしてないんすけど・・・【タイトル行】【URL行】って呼んでいて、メソッドも【タイトル行用】という単位で作ったりしているので、ひょうっとするとこれは・・・」
先輩：「そう、後輩くん、そしてユーザーさんも、頭の中で、出力の行をバリエーションとして捉えているんですよ。」
後輩：「そうだったんすか！！！」
先輩：「はい。でも、扱い方が不完全なんですよね。明確な形では扱われていない。そういう隠れたバリエーションってあるんですよ。これは不完全な、あるいは隠れたバリエーションですね。そこへの追加が発生したケースです。」
まとめ
出力フォーマットの行が増えた時の場合は、
TODOのバリエーションとは別の「行」っていう不完全なバリエーションに追加が発生し、
コードとしては行のフォーマット処理を新たに追加するって言う感じでした。
別の出力フォーマットが増えた時
今は出力コマンドで１種類の出力フォーマットのことしか想定していません。
CSVなど、別のフォーマットに対応するとしたら、
どのあたりを触らないといけないでしょうか。
先輩：「出力フォーマット（たとえばCSV）が増える場合、この出力コマンド全体を見直さないといけなさそうですね。」
後輩：「そうっすね。出力フォーマットが増えることは、１ミリも頭にうかんでなかったっす💦」
先輩：「要件にも書かれていませんしね。この出力フォーマットもバリエーションといえますが、今のコードではまったく考慮されていないので、未知のバリエーションということですね。そこへの追加が発生したケースです。」
まとめ
別の出力フォーマットが増えた時の場合は、
未知のバリエーションが増えることになりますが、現状のコードでは扱っていないため、
全体的な修正が必要ということでした。
ここまでのまとめ
・プログラムをよく見ると、様々なバリエーションを扱っている。
・バリエーションには、明示的に取り扱っているものもあれば、不完全なもの、コードとしては扱っていないものもある。
・バリエーションの変化（追加）によって、そのバリエーション要素を使っているコードに、様々な形で影響が出る。
↓ で結局どんなことをしていたかと言うと、、、
変更の可能性をさぐって、バリエーションと、コードへの影響を見極めよう！
そうやってコードを分析的に見た結果というのは、
実際に設計判断を行わないといけない場面で有力な判断材料になります。
先輩：「バリエーションって、奥が深いですね〜☺️」
後輩：「こんなふうに考えたことなかったっす💦」
改めて今回の状況は？
右側にTODOのバリエーションがありました。
左側に不完全ですが、行のバリエーションがありました。
先輩：「TODOの側にバリエーションがあるにもかかわらず、出力コマンドの方では、行のバリエーションを軸にしてコードが書かれているんですよね。」
後輩：「そうなってるっす。でも、これって良くないことなんすか？🤔」
先輩：「絶対にダメというものではないんですよ。それぞれのバリエーションが、コードに引き起こす作用がどの程度なのか、ということを都度考えるんですよ。」
後輩：「コードに引き起こす作用・・・。ついさっき見た、変更要求に対してどこを触らないといけないか、みたいなことっすか？？」
先輩：「ですね。そこと密接に関係しています。」
先輩：「そもそも（行）という捉え方が適切なのかどうかって、どう判断したら良いか分かりますか？」
後輩：「えっ、あ・・・。ユーザーがそう考えてるはずだから、というのは理由にならないんすか？」
先輩：「でも実際formatTitleLine()というメソッドのコードにすると、（タイトル行）の書かれ方は、最初に考えていた以上にそれぞれのTODOで違いがあるのだと思いませんか？」
後輩：「あぁ、、、そういうことかもっす。間違ってる感を感じていたのは、そういうことかもっす。」
先輩：「結局、今回の場合は、出力フォーマットにおいても、行ごとの違いよりも、TODOの種類による違いの方が強いってことなんですよ。」
後輩：「なるほど✨バリエーションとコードへの影響、少し分かってきたっす💪」
先輩：「そして、今考えてきたようなバリエーションの影響を、もう少し機械的に判定する方法があるんですよ。」
後輩：「えっ、せんぱ〜い、それをもっと早く教えてくださいよ〜😊」
先輩：「実は今回のコードのように、TODOのバリエーションが増えたら、フォーマット用の既存コードを修正しないくてはいけない、という状況を、オープン・クローズドの原則に違反している、と言うんです。」
後輩：「これが、OCP違反！ って言ってみたけど、まだ良くわかんないっす💦」
先輩：「(笑) 先程のバリエーションの話と合わせて大事なことなので、説明しましょう。」
オープン・クローズドの原則
オープン
機能を拡張できる。
クローズド
修正を行わない。
これらを同時に満たすこと！！！
モジュールに新たな振る舞いを追加する際に、
既存のコードを修正せず、
単に新しいコードを追加するだけで、
目的を達成できる状態になっていること。
オープン・クローズドの原則の適用範囲
ところで、
オープン・クローズドの原則を説明した先程の文章ですが、
先頭に、
「モジュールに新たな振る舞いを追加する際」とありますが
あらゆる状況にこの原則を当てはめるべきなのでしょうか？
実は、
適用すべき状況っていうのは
振る舞いの追加の要因として、バリエーションがある場合です。
なので、
バリエーションを起因として、
モジュールに新たな振る舞いを追加する際に、
既存のコードを修正せず、単に新しいコードを追加するだけで、
目的を達成できる状態になっていること。
簡単に要約すると、、、
バリエーションからコードを保護せよ！
ということになります。
オープン・クローズドの原則の着眼点
オープン・クローズドの原則というのが、
コード上のどのような部分を対象としているのか。
①バリエーションによって変化する部分はどこか
②それらが、バリエーションの軸に沿ってまとめられているかどうか
後輩：「めっちゃ分かってきたっす！今回のコードだと、どんなふうに見たら良いんすか？」
先輩：「OK、実際にコードで見てみましょう。」
今回のコードをオープン・クローズドの原則で言うと
今回コードで言うと、
TODOの追加によって、
コマンドクラス側にTODOの追加に応じた機能の拡張が必要になりました。
で、コマンドクラスを対応させるには、formatTitleLine()メソッドという部分に
分岐を増やすといった修正が必要でした。
そして、既存のコードに対して修正が必要ということは修正に対して閉じてないっていうことです。
これは、
機能の拡張と、修正しない。っていうことが同時には満たせていないので、OCP違反になります。
後輩：「OCP違反、めっちゃ分かりました！ で、これをどうしたら・・・」
先輩：「では、一緒に修正してみましょうか。」
オープン・クローズドの原則に準拠するには？
バリエーションの軸に沿ってクラス化
オープン・クローズドの原則に準拠するには、
バリエーションの軸をきちんと捉えて、それに沿ってクラス化するということが基本になります。
今回の場合では、
通常のTODOとカレンダーTODOというのがあり、
それがTODOの種類、バリエーションの軸になります。
なので、
出力の処理もこの軸に沿うようにして、
通常のTODO用のフォーマッタークラスと、
カレンダーTODO用のフォーマッタークラスをそれぞれ用意します。
フォーマッタークラス
通常TODO用フォーマッター
// 通常TODO用フォーマッター
class TodoFormatter
{
public function format($todo): string
{
return sprintf('[%s] %s' . PHP_EOL . '%s'),
$this->formatStatus($todo),
$todo->getTitle(),
$todo->getBody()
};
private function formatStatus(Todo $todo): string
{
$statusLabel = ['pending' => '未着手',
'working' => '進行中', 'complete' => '完了'];
return $statusLabel[$todo->getStatus()];
}
}
フォーマットっていうメソッドを持っていて、
TODOエンティティを受け取ってフォーマット処理しています。
ステータス処理だけ、分かりやすいように別メソッドにしてありますが、
特殊な分岐とかはなくて、シンプルなコードになっています。
カレンダーTODO用フォーマッター
// カレンダーTODO用フォーマッター
class CalendarTodoFormatter
{
public function format($todo): string
{
$now = new \DateTime();
if ($todo->getEndDateTime() < $now){
$status = '完了';
}else{
$status = $todo->getStartDateTime()->format('n/j H時');
}
return sprintf('[%s] %s' . PHP_EOL . '%s',
$status,
$todo->getTitle(),
$todo->getPlace(),
$todo->getBody()
);
}
}
カレンダー用はこんな感じです。
こちらもフォーマットメソッドで、カレンダーTODOエンティティを受け取って
フォーマット処理しています。
こちらの場合は、
ステータス部分で完了日時による分岐がありますが、
それでも全体としては、そんなに複雑じゃないです。
で、これってやっぱり
１つの先ほどの
通常TODO、カレンダーTODOどちらもそうなんですが、
１つのクラスやメソッドの中で、複数の種類エンティティを扱っているのと比べたら、
このコードってなんか気分的に楽な気はします。
出力コマンドクラス
class ExportAllTodoCommand extends Command {
// コンストラクターや初期化コードは省略しています
// ...
// 実行コマンド
protected function execute(InputInterface $input, OutputInterface $output)
{
// コマンド引数で指定した出力先のパスを取得
$path = $input->getArgument('output-path');
// リポジトリからTODOの一覧を取得
$todos = $this->todoRepository->findAll();
// そのTODOの一覧の、、に対してarray_mapで何か処理をした後、ファイルに出力する文字列を作って、それを出力先のパスに書き出している。
$contents = array_map(function($todo)
{
switch (true) {
case $todo instanceof Todo:
return $this->todoFormatter->format($todo);
case $todo instanceof CalendarTodo:
return $this->calendarTodoFormatter->format($todo);
}
}, $todos);
$content = implode(PHP_EOL . '----' . PHP_EOL, $contents);
file_put_contents($path, $content);
$output->writeln($content);
}
}
２つのフォーマッタークラス（通常、カレンダー）は、
コンストラクターなどで受け取るなどして、
コマンドクラス内で使えるようにしてあるとして、
executeメソッドの中の、array_map内の処理はこんな感じになります。
array_mapの中身のところにswitchが出てきました。
このswitchでは、TODOエンティティのクラスを判定して、
クラスに応じた、フォーマッターのフォーマットメソッドを呼び出しています。
通常のTODOだった場合は、
TODOフォーマッターのフォーマット。
カレンダーTODOだった場合は、
カレンダーTODOフォーマッターのフォーマットメソッドを呼ぶようになっています。
元々あった、
formatTitleLine()っていうメソッドなんかは無くなってとてもスッキリしました。
元のコードと比べれば断然いい感じになっています。
これでオープン・クローズドの原則に準拠したのか？
ところで、オープン・クローズドの原則に準拠するっていうことを目標にしていたのですが、
今のコードは準拠しているのか確認しておかないといけませんね。
簡略に図に表すとこんな感じです。
コマンドクラスの中の、
array_mapの中のループ処理でswitchを使うコードになっていて、
TODOのクラスに対応するフォーマッタークラスを使うようになっています。
こういう構造で先程と同じような検証をしますと。。。
TODOのバリエーションの追加によって、
コマンドクラス側にTODOの追加に応じた機能の拡張が必要になったとします。
新しいTODOクラス用のフォーマット処理は、
今のこのコードの方針でいけば、新たに別のフォーマッタークラスを作って
そこに、記述することになりますよね。
つまり、
既存のフォーマットクラスの変更は不要です。
新規クラスの追加でOK。
修正に対して閉じてるって言えます。
ですが、
ここはどうでしょうか。
先輩：「このコードだと、switchの中身に修正が必要なので、完全に修正に対して閉じているとは言えないんですよね。」
後輩：「そうなんすね。OCPって厳しいっすね💦 このコードでもすごく良いと思っちゃいますけど・・・」
先輩：「その感覚は間違ってませんよ👍 バリエーションの影響が大きく出るフォーマッターの部分だけに着目すれば、修正に対して閉じています。 実務では、これで十分と判断する場合も多くあるんですよ。」
後輩：「なるほど〜、分かってきました☺️ でもOCP違反を完全に無くしたコードも見てみたいっす！」
先輩：「ですよね！switchを除去して、コマンドクラスがOCPに完全に準拠するようにしてみましょう！」
と先輩は言っていますが、今回はここまでとさせていただきます。
気になる方は、動画で全てお話してくださっているのでぜひご覧ください🙇♂️
まとめ３つ
オープン・クローズドの原則が、コードに要請すること：
拡張できることと、修正しないこととを、同時に満たせる状態
オープン・クローズドの原則の着眼点：
バリエーションの軸に沿ったコードのまとまり
オープン・クローズドの原則に準拠するよう修正するには：
バリエーションの軸に沿ったコードのまとめ直し
今回のOneMoreまとめ
今回オープン・クローズドの原則を柱にして、
後輩くんの書いたコードを分析したり、原則に準拠するように修正したりしました。
オープン・クローズドの原則がどんなものなのかは、理解できたと思います。
しかし、
ソフトウェアのコードを設計するということは
このような原則に単に従うってことじゃないですよね。
本当のコード設計っていうのは、
今目の前にあるコードがどのような特徴をもっているのか、
どのような意図で書かれたのか、そういったことを見極めた上で、
次にどのような方向へ進めていくべきなのかを判断するというところにあると思います。
オープン・クローズドの原則などは、
設計判断を行うための１つの道具にすぎないです。
ですので、
オープン・クローズドの原則を足がかりにして、
要件やコードにあらわれるバリエーションを見極めよう！
感想
そもそもこういった原則は名前だけしか知らなかった。
なので、こういった機会に少し深く勉強して完璧ではないが理解が深まった気がした。
日常的にコードを設計することはないですが、
要望や改修でコードを書く際には、将来的にどのような変更の可能性があるのか？
など今回のような手法を用いて考えられると、もっともっと力がついていく気がしました！
バリエーションと、コードへの影響を見極めれるように頑張りたいと思います！
参考