はじめに
こんにちは。
イベントプラットフォーム「BOXIL EVENT CLOUD(以下、BECといいます)」開発エンジニアの石井です。
今回はRuby on Rails(以下、Railsといいます)アプリケーションにおけるコードリファクタリングについてお話します。
BECはオフショア開発でずっと進めていたのですが、1年半ほど前からオンショア（社内）開発に切り替えました。
そこからシステム統合、Railsのバージョンアップ対応を経て、少し落ち着いてきた頃から開発メンバーから「可読性が悪く開発し辛い」と声があがり、リファクタリングする流れになりました。
本稿では、チームでのリファクタリングの取り組み方を中心に話したいと思います。
対象読者
- チーム開発におけるリファクタリングへの取り組み方に悩んでいる方
- 中規模のRailsアプリケーションディレクトリ構成のオススメを知りたい方
- デザインパターンを積極的に取り入れた結果について知りたい方
理想のディレクトリ構成
はじめに、チームで決めたリファクタリング方針(ディレクトリ構成)をお見せします。
appディレクトリ構成
app/ アプリケーション用のディレクトリ app/assets/ アプリケーション用のリソースを置くディレクトリ app/cache_storages/ キャッシュデータ操作用のディレクトリ app/channels/ Action Cableファイル用のディレクトリ app/controllers/ コントローラ用のディレクトリ app/decorators/ ModelとViewの中間に位置しており、データの装飾用のディレクトリ app/helpers/ ヘルパー用のディレクトリ app/javascript/ JavaScript関連のスクリプト用のディレクトリ app/jobs/ Active Job用のディレクトリ app/mailers/ Action Mailerファイル用のディレクトリ app/models/ モデル用のディレクトリ app/models/modules/ データ加工処理用のディレクトリ app/reflexes/ stimulusReflex用のディレクトリ app/views/ ビュー用のディレクトリ app/workers/ sidekiq用のディレクトリ
↑を目指すうえで削除すると決めたもの
# システム統合時の負債 app/models/concerns/event_management_concerns app/models/event_management_models lib/event_management # デザインパターン app/queries/ app/components/ app/interactors/ app/facades/ app/services/
ディレクトリ構成はRailsの標準フォルダ構造を参考にしました。
削除対象や理由については、後ほど説明します。
取り組んだこと
リファクタリングに至った背景
「はじめに」でも触れましたが、システム開発をもともと100%オフショアで行っており、その体制を2022年7月から本格的にオンショア開発に切り替えました。
当初は、引き継いだインフラ構成やコードを元に軽微な不具合改修および追加機能実装を進めていく予定でした。しかし「クリティカルな不具合が頻発する」「応答速度が遅くユーザー離脱の可能性が高い」など、追加機能実装より優先して取り組まないといけない課題がたくさん出てきました。
それらを解決するために、不具合修正を進めつつ、不必要に分かれていたシステムのシステム統合、RubyおよびRailsのバージョンアップ対応など大きな改修を中心に取り組んできました。
大きな改修を経てコードと向き合う時間が増えてきた頃、「コードの可読性が悪い」「影響範囲が追いづらい」など、チーム内でコードに対する不満の声が大きくなっていきました。
引き継いだコードは、デザインパターンを積極的に取り入れた構造になっており汎用的に使える一方、抽象度が高いのでコードリーディングのコストが高い課題がありました。
そういった背景から、開発メンバーで集まって「開発しやすい状態・構成」について議論するミーティングを行ったのがきっかけで、リファクタリングが本格的にスタートしました。
チームで決めたこと、行ったこと
現状把握
課題に対して開発チームで認識のすり合わせを行い、2つの理由で開発体験が損なわれていることが分かりました。
■システム統合時の負債が残っている
BECは過去の開発事情から2つのシステムに分かれていましたが、これが負債の一つになっており、2つのシステムを片方に寄せる形でシステム統合しました。
システム統合で不具合があった際に、システム統合起因による原因か切り分けるため、可能な限りコードをそのまま移行しました。
その結果、動作的には問題ないものの「データ連携処理」「連携データ加工クラス」「連携データを扱うモデルおよびライブラリ群」など、本来1つのシステムを動かすには不要な構成が残っている状況となりました。
システム統合前のイメージ図
イベント視聴とアーカイブ視聴でシステムが分かれており、APIを利用してデータ連携をしていました。
■デザインパターンを使うことで複雑になっている
「Query」「View Component」「Interactor」「Facade」4つのデザインパターンが使用されており、データ取得処理・加工処理が複数ファイルに分散している状況でした。
また、データの受け渡しは基本的にインスタンス変数を用いているので、インスタンス作成しているコードとインスタンスメソッドを実行しているコードが離れていることが多く、ファイルを行き来するのに時間がかかっていました。
理想の構成
「システム統合の負債によるコードの冗長化」「デザインパターンによる複雑化」「インスタンス変数によるコードの読みにくさ」を解決するため、できる限りシンプルな構成を目指してチームで議論しました。
その結果、現状のBEC規模のWebアプリケーションであればRailsの標準フォルダ構造に「デコレーター(app/decorators)」「モジュール(app/models/modules)」を追加した形で問題なさそうだという結論になりました。
再掲：理想のディレクトリ構成
app/ アプリケーション用のディレクトリ app/assets/ アプリケーション用のリソースを置くディレクトリ app/cache_storages/ キャッシュデータ操作用のディレクトリ app/channels/ Action Cableファイル用のディレクトリ app/controllers/ コントローラ用のディレクトリ app/decorators/ ModelとViewの中間に位置しており、データの装飾用のディレクトリ app/helpers/ ヘルパー用のディレクトリ app/javascript/ JavaScript関連のスクリプト用のディレクトリ app/jobs/ Active Job用のディレクトリ app/mailers/ Action Mailerファイル用のディレクトリ app/models/ モデル用のディレクトリ app/models/modules/ データ加工処理用のディレクトリ app/reflexes/ stimulusReflex用のディレクトリ app/views/ ビュー用のディレクトリ app/workers/ sidekiq用のディレクトリ
■デコレーター(app/decorators)
引き継いだコードから組み込まれていたもの。
ModelとViewの中間に位置しており、Modelの値をViewで使用したい形に装飾処理が入っています。
必須で必要ではないものの、削除する必要もなかったので残しました。
デコレーターを使用しない場合は、Modelクラスに装飾処理を追加するといいと思います。
■モジュール(app/models/modules)
ファットモデルを回避するためのコード置き場で、新しく追加したディレクトリです。
簡単なデータ操作はモデル(app/models) 、 複数テーブルを跨いでデータ操作を行なうなどの複雑な処理はモジュールに切り出しました。
モジュールの導入ですが、Rails経験が10年以上あるベテランエンジニア(以下、Mさんといいます)から提案頂きました。
ネット検索しても特にヒットしないので一般的な考えではないかもしれませんが、BECのコードにおいて存在感は大きく、リファクタリングで一番恩恵を感じている概念になります。
トライ
2023年6月に方針を固めて、7月からリファクタリング着手予定でした。
方針を固めたタイミングで、リファクタリングへのモチベーションが高まっており「不具合タスクの中で取り組みはじめよう！」ということで前倒しでスタートしました。
不具合タスクのコードレビューコストは大きくなりましたが、コードレビューを通じて、具体的な対応方法についてのメンバー間の認識ズレが次第に無くなっていくのを感じました。
「鉄は熱いうちに打て」というように、方針決めてから取り組みまでの期間は短いほうが良いと思いました。リファクタリングをスケジュールする際は「方針決め ~ トライ」のセットが短いスパンで実行できるようにスケジュール調整することをオススメします。
結局シンプルがいい
デザインパターンを積極的に取り入れた結果
うまく活用できず、メリットよりデメリット(可読性の悪さ)が目立ちました。
活用できなかった原因として、引き継いだコードだったことも関係してますが、アプリケーション規模が大きくない内はデザインパターンは使わなくていいと感じました。
「Query」「Facade」の処理はモジュール(app/models/modules)に置き換え、「View Component」は部分テンプレート(render partial)に置き換えることで開発体験が向上しました。
取り除いたもの
Interactor
- ユーザー登録動線のワークフローを中心に使用。
- ワークフローの各ステップのデータ連携にcontextを使用していた。
- context起因でエラーが発生した場合、contextの状態がパッと見で判断できない、ユーザー登録なのでエラー発生時はロールバックしてほしいなどの理由で廃止。
Facade
- データ取得・データ整形など複数モデルのデータ操作をしており複雑で読み辛かった。
- インスタンスをビューに渡し、ビューからインスタンスメソッドを実行するのでクエリ制御があまく、無駄なクエリ実行によりパフォーマンスを悪くしていた。
- モジュールに切り出すことで上記の問題を解決。
Query
- 本来、Active Record::Relationに対して操作し、Active Record::Relationを返す必要がある。
- しかし、実装はそうなっておらずActive Record::Relationやデータ整形後のhashを返却する状態になっていた。
- hash利用がパフォーマンス悪化・可読性悪化の原因になっていたので修正しつつ、一度に置き換えることができないのでデータ操作はモジュールに切り出すことにした。
View Component
- 再利用性が高いView Componentが、実装上そこまで再利用されていなかった。
- 機能部分と描写部分を分けられることがメリットですが、機能部分をほとんど活用しておらず部分テンプレートを使用するのと変わりがなかった。
- コードを読む際に、機能部分を冗長に経由するので可読性が悪く部分テンプレートに置き換えた。
Service
- インスタンスメソッドになると処理が追いづらく、インスタンスのライフサイクルも考えないといけない。
- ベテランエンジニアのMさんの経験上、Serviceクラスを上手く活用できているプロジェクトを見たことがなく、Serviceクラスが存在することで困ったケースが多かった。
- 以上の理由から、処理をモジュールに切り出した。
リファクタリングしてどうなった？
直感的にコードが追えるようになり、コードレビューや調査タスクにかかる時間が短縮されました。
データ取得処理が分散していることで複雑になり「N+1問題」を抱えているコードが多い印象でしたが、モジュールにまとめることで見通しがよくなり発生頻度を減らすことができたと感じています。
小 ~ 中規模であれば
小 ~ 中規模のWebアプリケーション開発であれば、標準的なRailsの構成(MVC)で問題ないと思いました。
ファットモデルになりそうであればモジュール(app/models/modules/)を追加して、簡単なデータ操作はモデル、複雑なデータ操作はモジュールで行なうことで解決できます。
デザインパターンの導入は、規模が大きくなりモジュールの運用で課題が出てきた際に検討し始め、モジュールをデザインパターンにどう落とし込めばいいのかチームで話し合って決めることをオススメします。
おわりに
引き継いだコードのリファクタリングだったので、少し特殊なケースのご紹介となりましたが、「Railsの標準構成 + モジュール」にすることで可読性がグッと上がり生産性が向上しました。
今回のリファクタリングは、ベテランエンジニアのMさんがチーム内にいることで、方針決めや実装がスムーズに対応できました。
経験豊富なエンジニアはデザインパターン・アンチパターンに精通していると思うので、積極的に意見を伺うことをオススメします。
また、Railsのアーキテクチャは調べてみると考案された様々なものが出てきます。 プロダクトの規模や特性にあったものを取り入れることも大事だと思います。特にServiceクラスの利用は意見が分かれるので、チームの意見・プロダクトとあっているか調べてみると良さそうです。
リファクタリングは現在も進めており、削除対象をすべて置き換えれていない状況です。
今後、モジュールの肥大化など新しい課題が出てきた際は随時アップデートしつつ、このような形でアウトプットできたらなと思います。
