ざざっとメモ。当たり前ですが網羅はしてません。いわゆる After Rails な Web MVC を想定しています。紹介するコードは Ruby か PHP だけです。システムの規模などに依存しない内容です。悪しからず。
続くかどうかも分かりません。
まず基本のベストプラクティス
1. 名前重要
それを表す最もよい名前を探すこと。
よい名前が付けられない場合はそのことについて十分よく知っているとは言えない。よくない名前、嘘をついている名前1はそれを後から読んだ時に混乱を生み、理解を妨げ、変更時に問題の温床となる。
2. KISS ( Keep It Simple, Stupid )
いろんな業界、いろんなシーンで言われるのでよく分かると思う。プログラミングにおいては
- 変数を多くしすぎない
- functionを長くしすぎない
- (classに対して)functionを多くしすぎない
とかそんな意味で考えるとよい。
人間がパッと理解できるのはせいぜい3つ。それ以上になるなら何かがおかしい。
3. DRY ( Don't Repeat Yourself )
- 同じことを何回も書いてるならそれはやばい兆候
- その後変更が入った際にそれらすべてを「正しく変更できるか?」常に問い直そう。
ただし、一切のコピペを禁止するのは「早すぎる最適化」でよろしくない。目安は3回。
テスト書け
「伝統的な」と書いている通り Web MVC のフレームに対するテストの自動化は今や十分こなれていると言ってよい。まず書け。話はそれからだ。
テストの書きやすさ書きにくさ、ユニットテストなのかe2eなのかみたいな話はここでは省略する。
想定する構成
伝統的なサーバサイド Web MVC を想定している。伝統的な Web MVC には
- (router)
- Controller ( HTTP request / response を担う )
- View ( response 内の詳細を担う )
- Model ( アプリとして重要なロジックを担う )
がある。
アンチパターン1 - Loudness View
※ 一般的な呼称ではありません。自分で勝手に名前を付けました。
個人的にはサーバサイド Web でプログラマ / エンジニアが関わるものには大別して「Webアプリ」と「Webサイト」があると思っていて、業務アプリとかなんらかのソリューションを提供するものは全般的に「アプリ」と呼んでよいと思うけど、いわゆるブログ、CMSではないかもしれないけど結局「読まれる情報」を提供するためのものは「サイト」と呼んだ方がよいと思っている。
「サイト」の場合、非常に強い要求がない限りはフロントエンドヘビーにする必要はない。非常に強い要求とは尋常じゃなく大きなトラフィックをめちゃくちゃ高速に捌くために PWA 化したいとか細かく分析したいとか、モバイルで無限スクロールを実現しつつ非モバイルと共有したいとかそういう要求。
逆に「アプリ」に関してはユーザーの操作は「サイト」よりもやや複雑であり、URI 設計は改めて考えなければいけないが、API + リッチクライアントな方向に倒してしまうのも一つの手なので、イマドキは View が肥大してしまうことを悩む必要性は減ってきているのではないかと考えている。(もちろんその分フロントエンドの開発の方に難しさがシフトしている。)
そのうえで。
「読まれる」ものを動的に生成するということはそもそもそれなりに変数は多い。変数が多いのはよくない。View で扱う変数が増えるままにすると素朴な実装では Action の変数が増えてしまう。
対策
- 変数を連想配列やオブジェクトでまとめられないか考える
- オブジェクトでまとめられるなら「役割」がまとめられるはずなので Cells などを利用できないか考える
- Cells は HTTP と直接結びつかないアプリ内 micro MVC を実現するもの
trailblazer/cells: View components for Ruby and Rails.
上の Cells は Ruby 用で PHP の世界にはやや馴染みが薄いものらしいが、CakePHP 3 にはあるようだ。
CakePHP3新機能 View Cellの使いどころ - Qiita
残念ながら Laravel 用のものでまともに動作する Cells は見つけることができなかった。というか Laravel には標準で View Helper を簡単に定義できる機能がないので、どうも構造的に V と C が汚れやすい問題があるように見える。代わりに
- arrilot/laravel-widgets: Widgets for Laravel
- rinvex/laravel-widgets: Rinvex Widgets is a powerful and easy to use widget system, that combines the both power of code logic and the flexibility of template views. You can create asynchronous widgets, reloadable widgets, and use the console generator to auto generate your widgets, all out of the box.
この辺が使えそう。
Ruby の Cells は以前は Rails 依存だったが今は独立しているので、うまく整理すると汎用のものを PHP でも作れるかもしれない。
Cells にまとまればテストは HTTP request に依存せず行える。(逆に HTTP request をそのまま渡す、params をそのまま渡すのはよくない兆候と言える。)
とにかく View から見える変数が多すぎると人間の手には負えない。コンポーネント分割の話はデザインの文脈やフロントエンドの文脈でも語られている。最終的にはこの辺とすり合わせながら役割を整理する必要がある。まったく整理されておらず、変数が増えすぎることだけが分かっている場合はちゃんとアラートを上げること。
アンチパターン2 - Fat Controller
Controller の Action は routing に紐づいている。1 Action : 1 function がだいたい基本である。 Action で担う責務が多い場合、KISS の原則を守れなくなる。
具体的には以下のように破ってしまう。
- 変数が多すぎる
- function が長すぎる
アンチパターン2-1 Actionの変数が多すぎる
これには複数の理由がある。だいたい以下のどちらかあるいは両方。
- View で表示する変数が多い
- Model が多い、あるいは Model に渡す変数、Model から受け取る変数が多い
変数の代入が増えてきた class ベースの Action の場合、super class に持たせたり、Action の lifecycle を使って Action に処理が渡ってくる前に「初期化」したりすることが多いけど、これは誤り。Action と View が気にしなくてはいけない変数が減っていないのなら意味がない。
View の変数の数の話はアンチパターン1 の話と同じなので省略。2
対策 - Viewの値の取得、決定ロジックはViewModelあるいはPresenterへ
View で表示する変数がそれなりに多く、かつその準備のためのロジックが Action の中に散らばっているのであれば ViewModel あるいは Presenter が使える。ViewModel という言葉は文脈によっていろんな意味を持ち得るし、View と Presenter の分離と呼ばれたりもする。Rails の世界ではどちらかいうと Presenter と呼ばれ、Laravel の世界では Presenter がほぼ Decorator と同義なので ViewModel と呼ばれたりするようだ。
- Decorator と Presenter を使い分けて、 Rails を ViewModel ですっきりさせよう - KitchHike Tech Blog
- Views: Basic Usage | Hanami Guides
- spatie/laravel-view-models: View models in Laravel
変数が多すぎるアンチパターンは、Rails の場合、インスタンス変数が View から丸見えなのでインスタンス変数が増えすぎるという兆候で現れる。Laravel の場合は変数の binding を書く部分が膨らむ形で現れる。いずれにせよそれらの変数が増える部分をすべて Action の中に書くと Action の責務が分かりにくくなる。そこで ViewModel あるいは Presenter を入れる。
分かりやすいのは Hanami の View で、Template から参照できる変数をメソッドで定義する というものだ。Cells は View そのものを含めての分割になるが、ViewModel あるいは Presenter は View 向けの値の取得、生成の際のロジックを分離、隠蔽するためのものになる。これは HTTP からも HTML からも独立しているのでテストしやすい。
多くの場合はこの方法でロジックも変数も減らせるはず。
Model も Presenter から呼べばその前処理ごと隠蔽できるはずだ。
アンチパターン2-2 Actionが長すぎる
まず大前提なのだが、変数を扱う部分が減れば Action の長さはもうだいぶ縮んでいるはずである。
にも関わらずまだ Action が長い場合は View に直接結びつかないロジックが Action を膨らませていることになる。恐らくは Model とのやりとり、それも複数の Model とのやりとりでなんらかのロジックを組み立てているはずである。
対策 - ロジックを担うのはModel
複数の Model にまたがるロジックがどうしても Action を長くしてしまう場合は新たにそれを担う Model を作ればよい。まずこれ。Model は別にデータベーステーブルに紐づいていなくてよい。迷わなくてよい。ロジックを担うのは Model の責務。そして複数の Model 間をまたぐロジックは恐らく HTTP request と直接紐づけることなく書けるはずだ。
まず Controller, HTTP request と分離すること。そしてそのロジックのテストが十分に書きやすくなっていること。そこをスタート地点とするのでよい。
どうしても Model を作るのが気持ち悪い場合、扱いやすいのは例のサービスクラス、サービスオブジェクトだ。ただしサービスは難しい。サービスは雑に言うと「呼べる処理を一つにする」程度の意味でしか機能しないことが多い。
- 俺が悪かった。素直に間違いを認めるから、もうサービスクラスとか作るのは止めてくれ - Qiita
- Rails のアーキテクチャ設計を考える - Qiita
- Railsコードを改善する7つの素敵なGem(翻訳)
「サービスを作ればよい」という認識が生まれてしまう可能性があるとしたらそれはまた別なアンチパターンと呼んでよい。
アンチパターン2-3 Actionの中で条件分岐が多い
ここまでで View に渡す変数は減っていて、Model に押し付けるべきロジックも減っている前提で、それでも条件分岐が多いのはたぶん View を使い分けたいとかパラメータによって表示される内容を増減させたいと思っている。
対策
これも ViewModel あるいは Presenter でよいはず。
アンチパターン2-4 Actionが多すぎる
これは最悪のパターン。Controller が分かれていない。何に関する request を受け取っているのかまったく整理できていない。
これは頑張って分けるしかないが、順番としてまず Action を短くしていくのを先にやった方がよい。Controller class がでかいということは恐らくコンストラクタでかなり頑張って何かを準備していたり、大量のフィルタがあって「なんとかして共通化できるものを共通化して少しでも事態をよくしようと」したコードが入っているはずだ。
これは単純に OOP の失敗パターンである。Web MVC 独自の問題ではないので、そういう知見を普通に活かしていけばよい。
アンチパターン3 - Logicful View
よくあるのは
- if で分岐しまくっている
- Helper 関数などを使っていてもその引数が多い
- 値をそのまま見るのではなく変換の処理が挟まっている
など。
単純な boolean 分岐、単純な変換でも数が多くなれば十分にあぶないコードである。View ではその数が多くなりやすい。
対策
- partial に分ける
- cells に分ける
要は分岐、変換ごと分離しちゃう。
多くの変数も View 側で渡さなきゃいけないのかを吟味して不要なら Cells など別なレイヤーで処理してしまう。
アンチパターン4 - Controller Filter
Controller の Action の記述を短く抑えるために、Action ではなくその前後に動作する Filter(Rails だと before_action とか)に書きたくなる場合がある。
しかし例えば Action のバグを直すために Action に注目しているような場合に Filter の動作は目に入って来にくいので、複雑な動作が Filter に含まれるとか、Filter の数が多すぎるのはメンテナンス性が低いと言ってよい。
あと独立した function になってなくて HTTP request と切り離してテストできないとしたらそれは完全に誤り。
対策
特に Action の中で参照する値を扱っている、下手すると書き換えているような場合は Action から明示的に呼び出す形にしておくべき。params の validation なんかもそれが繊細であればあるほど Action の中から明示的に呼んだ方がよい。
対策
アンチパターン5 - JS in View
JavaScript リテラルを View の中に書いちゃうやつ。これをやると
- サーバサイドの状態を簡単に JavaScript に渡せる
代わりに
- ESLint でチェックできない
- テストコードも書けない
- cache も効かない
- サーバサイドの状態がリテラルで渡されるので、nil とか JavaScript で扱えない状態が発生する
- 最悪 SyntaxError とかリカバリ不能な状態になり、すべての JavaScript の実行が止まる
対策
絶対にやるな。
絶対だ。絶対に許すな。
お前も 2016年に設計なんてない、そこそこの量のJavaScriptのエラーを監視して対策し始め たいか?
いいか。絶対に許すな。
サーバから何も渡さず、何のロジックもない十分に短いコードだけは例外的に許可しろ。それ以外は全部ダメ。