Railsでどのコードをどこに書くかについてのガイドライン
自己紹介
メディアエンジンの岩元(github)と申します。7月入社の３ヶ月目の社員です。
新卒から１０年程度はメーカーの社内SEでCOBOLや色々な言語で書きつつ社内のいろいろなシステムに関わりました。SEなのでコードを書くのより読むほうが多く、システムの設計に関心があり色々と調べました。
実のところRailsは入社することが決まってから本格的に勉強したくらいなので、Rails的ではないことを言っている可能性があります。参考程度で考えていただけたら幸いです。
経緯
入社してしばらくは、簡単目なチケットをもらっていました。地道にチケット消化してどのような開発文化を持っているかを身に着けました。ソースコードもそれほど難しい書き方はなく、その後だんだんと難易度の高いチケットをふってもらえるようになりました。
そこで、おこったことが既存のメンバーの考えたチケットの重みと実際にかかった時間に差が出て来ました。原因の一つは調査に時間がかかった点でした。どこに何が書いてあるのかわからない私はユーザーレベルの部分から、DBまで紐を手繰るようにして変更を加える点を探す必要がありました。もう一つが、変更点がそもそも最初の想定より多かった点です。多くの場合それは既存のシステムの整合性が取れれいない部分であり、技術的負債があることを物語っていました。
そこで、気がついた点、こうだったら良いなという考えをまとめました。
環境
対象はフロントをVue.js(Nuxt.js)、サーバサイドをRuby on Railsで構築しています。詳しくは弊社田中が書いた「インフラをHerokuからAWSへ移行した話」を参照してください。
弊社での問題点
動作自体には問題はありません。コードも一部をのぞいてRailsを扱える人なら難なく扱える程度の複雑さです。とはいえ下記のような問題を抱えています。
Controllerから取得できるデータが想像できない:
erb(View)に対して渡すデータを、そのままjsonに渡しているとイメージください(実際はそれよりだいぶ簡素にはなっている)。なので、Nuxt.jsのパスとAPIのパスはほぼ一致しています。
例: 投稿者を管理する CreatorController に投稿者の投稿を取得する Articles があるなど。(ArticleController と機能がかぶっている。)
画面とControllerは1対1ではない:
上で書かれたことと逆の話ですが、フロントエンドは必ずしも画面に対応したパスでアクセスしているわけではありません。追加の場合など、すでにある他のコントローラーにアクセスします。これによって、フロントエンドがどのコントローラーを使っているのか調べるのにはコードを追う必要があります。
API上のインターフェイスが多い:
リソース単位にREST APIを作成すると最小のインターフェイスになるので、何らか不必要なインターフェイスを持っていると思われます。実際のところは、レスポンスの問題などで適切な場合もあるので程度はわかりません。
例: 別の項目でも書いたが投稿(Articles)を取得する機能が、ArticleController だけではなく、 CreatoroController にも同様の機能があるなど。
MVCの役割分担が不明確:
多分具体的な例を挙げはじめるときりがないほどルールが無い状態です。すでにコードの重複はおこっていると思われます。
方針
- 重複回避のため、画面単位のAPIをやめRESTfulなAPIに移行しよう。
- 各モジュールで実装されるコードの種類を明確にしよう。
- Keep it simple stupid!(馬鹿らしく単純にしろ！KISSの原則)
- オブジェクト指向プログラミングにはこだわらない
RestAPIに関して
- リソースは外部から見てのリソースでありDBとは切り離す
- Railsの
route.rbでresourcesを可能な限り使う - 階層を持たないリソース毎のパスを用意する
- その上で階層を持つパスを定義しても良い
- クエリパラメータを使う
- アクションはPOSTとする(PATCHは使わない)
- その他
-
users/meなどidでないGETもOK - 数値のみ返す集計(件数・合計)
-
インターフェイスについて
-
GET/POST/PUTは同じ形式のリソースを返す
フロントエンドで型を使い回せるようにする。
-
リソースを入れ子にしない
入れ子になったリソースも調査対象になる。レスポンスに問題ない範囲であれば読み込みを分割する。
-
関連する他のリソースの項目はIDと名前程度にする
- IndexやGetなどは、一覧画面の表示項目まで程度で考える。
- 問い合わせ回数が多くても、実際の速度が遅く感じるまで放置する。
-
例外は認める。ただし、理由を付けて
- 影響範囲がわかりやすいように、フロントエンドの1箇所(1画面)のみでの利用
- 再利用すると利用していない項目に気付けなくなる
- 影響範囲がわかりやすいように、フロントエンドの1箇所(1画面)のみでの利用
MVCの分割に関して
大事なこと
- Controllerは受け渡し
- Modelはデータ操作
- その他のリソースを扱うModelもある
- Serviceは複数のデータ操作の調整
- Viewは出力項目を決定する
controllerに関して
-
リソース毎に作成する
逆を言うとリソース以外データは扱わないようにします。リソースとして何を設定すべきか明確でない場合は都度メンバーで話し合うと良いです。
-
権限で項目が変わる場合はその項目を分割する
例: ユーザーの住所など他のユーザーが見る必要がない項目ははユーザー明細で管理する。
-
複数のリソースの一括更新はできるだけ避ける
例: 一括削除などはOK
例: ユーザー登録時にユーザー明細を作るのはOK
-
new・put以外は直接値を入れない(議論ありそう)
permitでやれる範囲、変換以上のロジックがあるものはModelに書く。 -
パラメータをそのままModelなどに渡さない
Modelの該当メソッドがControllerやパラメータに依存するのを避ける。
-
ソート・集計
ActiveRecordへの操作はModelの役割、Controllerは集計とソートだけにすること。複雑な集計はModelで行う事。
-
副問合せは最低限に
影響範囲が小さいほどシステムは単純になる。
-
View指定は必須
例外として、数値や文字列など単一の項目のものを除く。
-
複雑なロジックが必要になったらServiceクラスを定義する
- 入力チェックと存在チェック以上の
ifは他に任せる - トランザクションが必要な、複数のModelへの処理
- 入力チェックと存在チェック以上の
Viewについて
-
副問合せをしない
リソースとController１対１になるように、別の目的でデータを取得しないように関係の無いデータはできるだけ取得しない。
-
加工・編集は極力しない
フロントエンドのみで済ませたほうが変更負荷が小さい。
-
細かい権限の問題で項目にNullを入れることがある
できるだけ避けたほうが良いがかえってわかりづらくなる場合にはOK.
Modelに関して
-
APIのリソースと別構成(当然)
インピーダンスミスマッチが合ってもしっかり正規化すること。経験的に正規化のしすぎが原因のレスポンス問題は起こったことがなく、ほとんどが正規化をしていないために問題が起きている。DBのリファクタリングはコストが高すぎる。
-
フィルタは可能な限りscopeで書く
フィルタの意図がはっきりして再利用性が高めるつもりです。単純なものでも、拡張性を担保する意味で必要です。
-
更新はメソッドを介して行う(むしろControllerへの制約)
ModelとControllerは1対多の関係になる場合、Controllerにロジックを書くと同じものを書いても気が付かない。
-
他のModelへの更新を呼ばない(議論ありそう)
これもロジックの分散を避けるため。Serviceクラスを導入すること。
-
ActiveRecordでは無いModelもあり
ファルサーバー・他APIの呼び出しなど
その他
-
get_/request_/judge_などの参照系でDBを変更しない！ - レビューアー2名欲しいなぁ。(技術的負債の自転車操業中)
移行に関して
技術的負債は簡単には返済できないから負債なのです。工数に関しては継続が可能な量を継続的に確保したいなと考えています。
具体的にどのようにすすめるかはまた別の機会があれば書きます。
終わりに
とりあえず思いついた範囲で書きましたので、メンバーとブラッシュアップした結果全く違うやり方を選ぶかも知れません。
最後に、弊社ではエンジニアやデザイナーなどの職種で積極的に採用中です！
Wantedlyでも情報発信していますので、もし興味がありましたらぜひご覧いただければと思います！
参考
REST について調べたまとめ:
https://lukesilvia.hatenablog.com/entry/20091025/p1
RESTに対する７つの誤解:
https://www.sakimura.org/2011/11/1289/
Rails のルーティング(Railsガイド):
Discussion