こんにちは、estie VPoEの青木です!
2021年2月16日、estie pro開発チームで、📕 『パーフェクト Ruby on Rails』の著者igaigaさんをお招きしてRuby on Railsのアドバイスをいただきました。
twitter.com
今回はその際の様子をご紹介します!
参加したestieのメンバ(写真左上から順番に)
- t-poyo
- shin
- igaiga-san
- eririn
- hige
- tomu
経緯
実は、igaigaさんにestieに来ていただくのは今回が2回目です!
前回は、我々チームの状況を踏まえ「estieのためのRuby on Railsのコード規約」についてアドバイスを頂きました。
(前回来ていただいた際も「サービス層のクラスの"キャラを立たせる"」など面白い内容がありますので、ぜひ読んでみてください!)
inside.estie.co.jp
今回は、以下の2つを目的として設定しました。
- 前回のアドバイスの後、チーム内で議論したコード規約を igaiga さんに確認してもらい、その完成度を高める
- igaigaさん主体の勉強タイムを作り、チーム全体の Ruby on Rails の理解や興味を深める
前回はestieから聞きたいことをガンガン聞く形だったのですが、今回はigaigaさんに講師をお願いしました。
スケジュール
当日は、igaigaさんにリモートで参加していただき、estieメンバと濃密な6時間を過ごしました。
この記事では、読者のみなさんに当日の様子が伝わる内容をいくつかご紹介します。
作成したコード規約のレビュー/議論
前回の議論を元に作成したestie pro開発チームのRuby on Railsコード規約を説明し、以下の点についてigaigaさんを交えて議論しました。
コード規約で定義したレイヤー
コード規約の中で以下のように整理することを検討していました。
- controller
- model
- usecase
- service
- presenter
特にアドバイスを多くいただいたサービス層のコード規約について紹介します。
確認したサービス層の規約案
- ApplicationService,DomainServiceの分類は行わず、命名できるキャラの立った処理群をまとめたClassをServiceと呼び、servicesフォルダに格納する
- 複数のModelにまたがる処理を主に行う
- ServiceがServiceを呼ぶことは許容する
- Service間のBoundaryについてRelationは許容せずPORO(Plain Old Ruby Object), modelに限定する
estieの開発チームでは当初、上記の通り「Service層のBoundaryにRelationを許容しない」方向で規約を考えていました。
これは、ActiveRecordに依存する層を最小限としたいという考えとともに、Service層の処理を読んだだけでは渡されたActiveRecord:::Relationの中身が分からず、読まないといけないコードの量が増加するという考えからです。
この点についてigaigaさんより、「Boundaryを規定するClassに詰め替える」ことが多く起こり、逆に見通しを悪くして将来的に負債とならないかを指摘いただきました。
この議論の中で実際にこのような規約で書かれたコードを確認するかという話題になり、igaigaさんの「特に静的型付けではないRubyのような言語ではコードが読みづらくなるのではなかろうか」というコメントから、静的型付け言語に慣れているエンジニアと動的型付け言語に慣れているエンジニアが思う、「挙動を理解しやすい」コードや「書くのが楽」なコードの捉え方の違いが見えてきて、非常に興味深かったです。
また、コードベースに対して十分な知見がチームに蓄積するまでは、可能な限り判断を引き伸ばしたほうが良いというアドバイスもいただきました。ついつい「最強のアーキテクチャ」を考え出そうとしてしまう我々にとって不足してしまいがちな観点を補足していただきました。
コードレビュー
前回は、estieから相談したい点をあげて議論する形が中心だったのですが、今回はestieのコードベースやPull Requestに対して気になる点を挙げていただきました。
頂いたアドバイスをいくつかご紹介します。
- 現在のRuby 及び Railsのバージョンで発生しうるセキュリティリスクと、今後のポリシーについての提案
- ディレクトリトラバーサルへの対応の必要性とその対応方法(今回の対象はs3で権限制御されている箇所でしたが今後にむけた対応も教えていただきました)
- RailsのSingle Table Inheritance関連の予約語にならないようにtypeではなくkindを利用するべきなどのカラム名の命名に関するアドバイス
- レビュー時に他のユーザーのデータを取得してないことを確認しやすいコード記載方法のアドバイス
このように、igaigaさんの豊富なご経験から、普段の機能実装では見落としがちな点を数多くご指摘いただきました。
いがいがふむふむタイム
「いがいがふむふむタイム」とは、igaigaさんがestieメンバのためにRuby on Railsのさまざまなトピックをお話していただく時間です。口に出して読み上げたくなる素敵な名前ですね。
今回は、ActiveRecord周りのトピックをリクエストして、「has_manyにmethodを生やす」というお話をしていただきました。
has_manyにmethodを生やす
以下のようにActiveRecordの `has_many`にはmethodを生やすことができるという話からスタートしました。
Railsガイドでは以下に記載されている内容です。Active Record の関連付け - Railsガイド
class User < ApplicationRecord has_many :books do def foo self end end end
と定義すると
irb> User.first.books.foo (0.1ms) SELECT sqlite_version(*) User Load (0.2ms) SELECT "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT ? [["LIMIT", 1]] Book Load (0.1ms) SELECT "books".* FROM "books" WHERE "books"."user_id" = ? LIMIT ? [["user_id", 1], ["LIMIT", 11]] => #<ActiveRecord::Associations::CollectionProxy [#<Book id: 1, title: "Rubybook", price: 2480, user_id: 1, created_at: "2020-05-28 02:28:46", updated_at: "2020-05-28 02:28:46">]>
として呼び出せます。
このように、このmethodの中は ActiveRecord::Associations::CollectionProxy というオブジェクトであることを確認しました。途中、
- これらをチェーンする事はできるの?
- has_oneだとどうなるの?
といった質問をはさみながら、文字通り「ふむふむ」と確認しながら進んでいきました。
普段の実務とは異なるテーマに興味が湧いて、Ruby on Railsというフレームワーク自体の経験があまり多くないメンバにとっても、自分で勉強したいと思うきっかけになったと感じます。
全体を振り返って
第1回とは趣を変えて、
- チーム内で議論したコード規約を igaiga さんに確認してもらい、その完成度を高める
- igaigaさん主体の勉強タイムを作り、チーム全体の Ruby on Rails の理解や興味を深める
という形で実施することができました。
非常にお話しやすいigaigaさんとたくさん議論する中で、コードベースのうまく行っている点もたくさん褒めていただきました。
普段の機能開発とは異なる、将来的な目線やRuby on Railsというフレームワーク自体への興味をもつことができ、大変良い時間となりました。igaigaさん、本当にありがとうございました。