技術的要件で共通化と抽象化は9割間違っている
今回もエンジニア中二病紹介シリーズです。ベテランなエンジニアは幾度となくジュニアエンジニアの共通化や過去のエンジニアの誤った共通化によって苦しめられてきたのではないでしょうか?
今回はそちらについて解説していきます。
最悪共通化はリファクタに回していい。
共通化する場合と共通化をしない場合でそれぞれにどのような違いが現れるでしょうか?先に結果をお出ししておきます。
共通化する場合 | 共通化しない場合 | |
---|---|---|
単体の機能の調査コスト | テストがある場合:◯ テストがない場合:✕ | ◯ |
開発速度 | 適切に共通化されている場合:◯ 適切でなく共通化されている場合:✕ | ◯ |
システム全体の調査コスト | △ | X |
共通化しないで縦に書いていく場合
プログラムは上から順に実行されるので、上から順に読まれていきます。
class Controller {
main() {
// 処理A
// 処理B
// 処理C
// 処理D
}
}
なので、処理Aと処理Bと処理Cと処理Dというふうに上から見たら良いだけなので、ロジックが頭に入ってきやすいですよね。もちろんわかりにくいところはコメントがしてあったり、共通化目的ではないメソッドの切りだしはある前提です。
共通化がゴリゴリ進められている場合
極端な例になってしまいっていてすみません。ただよくある例をかけ合わせたものなので、現実とかけ離れているというわけではありません。
※もうちょっとわかり易い例をストックしておけばよかった。
// middleware処理A
// middleware処理B
// middleware処理C
class Controller extends BaseController {
main() {
this.preprocess();
// 処理A
// 処理B
// 処理C
// 処理D
this.postprocess();
}
}
思いつきで共通化して、共通化のスコープが大きいときに完全に頭に入ってこないソースコードが出来上がります。BaseControllerがpreprocessとして、アプリケーションの前処理をして、postprocessで後処理をする。そして、いくつかのmiddlewareが差し込まれている。これが、開発体験を上げるものであればよいのですが経験の少ないエンジニアが作ったものは大体の場合うまくいきません。
BaseControllerのpreprocessに書くべきなのか、middlewareに差し込むべきなのかの実装判断にも迷いますし、もし共通化するメソッドにそのコントローラに特有の処理が入り込んでしまった場合は、地獄の始まりを予感させます。
型とテストがあればなんとかなる場合はありますが、そういうコードが書けてしまう環境で自動テストなどは殆どないのではないでしょうか?
縦からズラッと書いて読みにくくなるとき
縦からズラッと書くときは、本当にif文とfor文だけを上から書いていくのではなくて、下記のような工夫をどんどん記載してください。
- プライベートなメソッドに切り出して何をしているかわかりやすくする
- コメントを付けて、わかりにくい実装に補足する
特にビジネス要件による処理は、コメントがないとその意図がわかりにくくなるので必ず書きましょう。チケット番号をコメントするでもいいですし、コンフルエンスのような社内でドキュメンテーションをするシステムが決まっているのであればそのリンクを記載するのもいいでしょう。
クラスの中のインスタンスメソッドにしてもいいですし、そのファイルの中で関数として扱ってもいいでしょう。
関数にするかメソッドにするかは、そのクラスに関係するものであればインスタンスメソッドのほうが良いと思いますが、ユーティリティ的な色が強ければ関数にするのが良いでしょう。
プログラミング経験で2,3年程度積んだヒトが迷うものであれば、それがどのような方法であれ、プライベートに表現されているのであればどちらでもいいのでしょう。
レビューするヒトもぜひその意図を汲み取ってほしいですね。無用なところでのレビューはノーセンキューです。
共通化のレベルを把握する。
共通化するかどうかの判断は、いつ決まるのでしょうか?
共通化するべきかどうかには慎重になるにしてもすべての共通化を気をつける必要はないと思います。共通化のレベルに応じて、ガンガン共通化していったらいいものとそうでないものはある程度区別が付きます。
難易度 | 共通化の種類 | |
---|---|---|
1 | プリミティブな値や配列の操作 | ビジネスロジックやアプリケーション内で自作したデータ構造によらないもの |
2 | データ構造に依存した処理 | 特定のデータ構造に依存した処理で、副作用を持たない。純粋な関数。 |
3 | データ取得とデータの整形などを一緒にした共通処理 | APIの呼び出しとその戻り値を加工したりする共通処理。データの加工などをおこなうために、呼び出し元によっては使いにくくなる可能性がある場合に実装の足を引っ張る |
4 | 異なるエンドポイントの呼び出しを共通化するような処理 | 非常にリスクが高い。異なるものとして扱わないとエンドポイントやページベースでの最適化をしたい際に相互に影響を及ぼしてしまう。 |
5 | 外部システムの呼び出しを種別によって切り替える処理のデザインパターン化 | 決済の処理を外部サービスに依存していて、それをストラテジーパターンのように無理に共通化、抽象化しようとするようなパターン。コールバックのような処理もあったり、仕様や規格が決まってないもの字面で共通化できる |
1,2の共通化はほとんどリスクや難易度が高くないのでガンガンやってもそこまで問題にならない。3になるとこの辺がエンジニア2,3年めがガンガンやると大体失敗してるなとなっているパターンです。共通化することを前提に考えすぎていて、異なる共通化がガンガン生まれて、共通化したものに更に共通化するみたいな地獄が生まれます。
4,5はだいたい失敗しますね。古い時代のwebアプリであれば、おすすめ機能などもなくシンプルな一覧であればよかったのかもしれません。今日のwebアプリは複雑になっているのでページごとにテンプレートを共通化するというのは、たとへばヘッダーやフッターのレベルに留めておいたほうがいいと思います。むしろそれ以上やると破綻すると思います。
1,2,3まではガンガンやりましょう。そして、1,2のレベルであればユニットテストも書きやすいので、ユニットテストを入れてその部分での結合テストは省略しましょう。3に関しては、mock化して結合テストもありだと思います。
3のパタンーに関しては、データ取得した後にデータ構造に依存させてパターン2の共通化に落とし込むという手法があります。データの呼び出しと加工の処理を一緒にすることに圧倒的なメリットが有るなら3のパターンをとってもいいですが、自信がなければ、そのような手法もありでしょう。
まとめ
共通化は、技術的に同じ処理の繰り返しかもと思って取り組むとだいたい失敗します。縦にずらっと書いたほうが遥かにメンテナンス性が高いことがあるのでそちらを意識して、共通化するべきかしないべきか判断をしましょう。負債はほとんどが誤った共通化によるものです。
Discussion