👋

脱Rails初心者のためのリファクタリングガイド

2022/04/18に公開

概要

Railsのコードはそこそこ書けるけど、
いざコードの山を目の前にするとどこからリファクタリングしていいかわからない、という人(Rails歴 3年未満くらいの人)のためのガイド。

この記事ではフリーランスSEである私が過去に関わってきたいくつかのプロジェクトから、大体どのようなシステムにおいても使えそうなリファクタリングの手法と全体的な流れをまとめた。
なお、jsについてはフロントエンドに何のツールを使用するかで大きく変わるのでこの記事ではあまり深く書かない。

本題の前に

サービスクラスについては、ネット上には多くのリファクタリング手法が掲載されているがそれを鵜呑みにしてそのまま実装すべきではない。
なぜなら、その手法の多くは熟練したエンジニアによるもので且つ、チームの中でもリーダーをしているような裁量・自由度が高い人が書いたものが多いからだ。そういったものが本当に自分のチーム・システムに合ったものであるかを判断することは難しい。
脱Rails初心者向けレベルのリファクタリングはできるだけRailsのレールからはみ出さずシンプルに進めるべきだろう。

手順

すべてを同時進行でリファクタリングするのは骨が折れる。
特に優先してリファクタリングしたい箇所が決まっていなければ

  1. Fat Controllerの整理
  2. modelの整理 (+ サービスクラス)
  3. viewなど

の順ですると良い。
controllerがfatになっている場合、modelへ処理を移す可能性が高いのでよりスムーズに進められる。

Fat Controllerの整理

最初はあえてFat Modelを目指す

modelに処理を集めることでそのmodelの規模が把握でき、
サービスクラス用のツールを導入するかなど、今後の方針検討もしやすくなる。

全体の行数を減らす

  • publicメソッドはアクションのみに
  • 各メソッドの行数を減らす
  • ネストを減らす(if文のブロックならreturn if ~ などで)
  • メソッド分割をする

処理の移動先は基本model。
modelへの移動とリファクタリングを同時にするのが厳しそうな場合は、「本来controllerにあるべき処理は…」などと考えず、アクションの中身を一旦全てmodelに映してから必要に応じてcontrollerに戻す、という手順を取るのも良い。

この、 「必要最低限の処理だけがcontrollerにあり、残りほぼすべての処理がmodelにある状態」 がまず第一に目指すべきところだと言えるだろう。

Modelの整理

  • controllerと同様に、メソッド分割をする・ネストを減らす
  • scope, association, 条件付きassociationを定義してデバッグしやすくする。
  • 値の検証はvalidate(s)に移動する
  • 機能ごとに処理を切り出す

機能ごとに処理を切り出す方法としては

  1. 同ファイル内でmodule化する
  2. 外部ファイルでmodule化する
  3. serviceクラスに切り出す

同ファイル内でmodule化する

機能のまとまりごとに、Module#concerning(参考)でブロックにすることでファイル全体の見通しが良くなる。
private, private_constant を使って影響範囲を限定的にするとなお良い。
java出身の人などは特に、1ファイル1クラスでないことが非常に汚く見えるかもしれないが、実際にこのように記述されていることがよくあったしRailsライクであるようだ。
リファクタリング方針が決まらない内や、近い将来にさらなる改修が予定されている場合に一時的にこうしておくのも良い。
ただし、concerningprivate_constantなどは使わない人にとっては異物感があるので初めて使う場合は注意。PRレビューで事前にコメントを入れておくなどすると良い。

同ファイル内でmodule化するだけならmodelファイルの行数を減らすことにはならないのではないか?

この疑問に対して、実際にそれを私が調べたり人に聞いて回ったりして出した結論としては以下のようなものである。

  • ファイル自体が大きくともその中身さえ整理されていればあまり問題にはならない。
    modelファイルが大きくなるのはそれだけ機能があるということなのである程度はしょうがない。
  • 可能であればテーブル自体をさらに細かく分割し1テーブルが持つ役割(1modelが持つ機能)を減らすようにするのが良い

外部ファイルでmodule化する

例としては、user.rbの処理を切り出したいならusers/***_module.rbとして切り出す形を基本とする。
外部moduleだからといってconcerns/に置くのはおすすめしない。
concerns/に置くのはあくまで複数のmodel間で共通する処理のみとしておく。

他のやり方に比べて最もシンプルであるが、ちょっとしたコードまで分割しているとファイルが多くなるのでバランスが大事。

サービスクラスに切り出す

サービスクラスの理想は明確な指針があること。
こういった感じのルールをチーム内で策定するか、公開されているossのgitリポジトリを共有し、このような形でやっていきたい、という方針を立てる。(例えばこれとか gitlabhq/gitlabhq)
逆に、メンバーそれぞれが勝手に俺流のサービスクラスを作り始めると地獄。
つまり、ルールを決めて継続的に運用していくことの難しさ・手間を考えると、チームの状況によっては下手にサービスクラスを開拓するよりはmodelの中に含めてしまった方が良いことも多いだろう。
サービスクラスを作るかどうかはあくまで選択肢の1つである。

Viewの整理

  • ロジックをhelperへ分離
    • viewがごちゃごちゃしている割に忘れ去られているかのようにhelperが使われていない事がある。
      helperは使いにくいと感じている人もいるかもしれないが、Decoratorなどを検討する前にhelperで事足りないか試してみると良い。
  • 共通するコードのpartial化
  • formの送信はform_withで統一
  • jsの整理
    • viewに直接コードを記述せず、ページごとのjsファイルに分離する
    • そもそもサーバー側に書ける処理はサーバー側に寄せる
  • cssの整理
    • viewに直接styleを記述しない(slimだとファイル内でsassクラス定義できるので、ちょっとした定義ならファイル分けするより良いかも)
    • js専用のclassはjs-プレフィックスをつけて区別する

補足、その他

リファクタリング時にspecも一緒に作る

リファクタリングは仕様がきっちり定まっていることが前提。
仕様がはっきりしていないのにそれに手を加えようとするのはおかしい。
specも同タイミングで作りリファクタリング前後で結果に変わりがないことを確認できると良い

specの書き方

これはチームの方針によるところが大きいが基本的には、
specはapp下のコード以上にレールがなくコーダーによって書き方がまちまちなため、コードレビューではあまり細かく(厳しく)指摘しないほうがベター。

テストのレベルや記法についての要望がある場合は、rubocopのrspec向けのルールを細かめに設定するなどCIで検出できるようにしたり、実装方針をwikiなどにまとめておくと良い。

let記法やマッチャ、共通化のためのshared_examples_for、system specでよく使われるcapybaraなどはじめは書き方に癖が強くあると感じるものがたくさんあるので少しずつ調べていこう。

細かな記法より構造が整理されていることが大事

たった5行のメソッドでspecまでついていれば、細かい記法についてはそこまで気にならない。
逆に50行のごちゃっているメソッドなら、細かい部分についてもコードレビューで指摘したくなる。

リファクタリングしすぎない

使用頻度の低いところ、将来的に改修する予定のないところには時間をかけない。
また、きれいなコードを書くこと自体が目的とならないようにする。YAGNI.

また逆に、近い将来に改修予定がある場合には少し入念にリファクタリングしておいたほうが良いこともある。

コードレビューにおいてもそういったことを念頭に、過剰なリファクタリング要求をしないようにしたり、適切なリファクタリング提案ができたりすると良い。

プルリクエスト単位を小さくする

リファクタリングでありがちなのが巨大な変更のPRになってしまい、自分が何を変えたのかが把握しづらくなること。コードレビュー者の負担も大きくなる。
1PRに詰め込まず、分割して作業していくことで自分が根が詰まる事も少なくなる。

まとめて作業したい場合は、commitの時にファイルを分けておけば後からgit cherry-pickを使って簡単にPR分割することもできる。 git diff [ブランチ名] --name-onlygit stash系コマンドも使えるようになっておくと便利

admin(検索機能)なら

ransackのgemを利用する。
ガチの爆速でコーディングできるのでおすすめ。

コピペを減らしたいなら

新しいファイルを作るときは、コマンドからscaffoldやジェネレータを叩く形で適切なファイルが生成されるようにする。
コーディングにスニペットツールを利用する/利用するように勧める。

Skinny Controllerの維持(rails console開発のすすめ)

これは本題から少しずれるが、
rails consoleを利用してコーディングしていくことで、自然とカプセル化されたmodelメソッドから作ることができ、実行結果が確認しやすくspecも書きやすくなる。
新機能を開発する時にcontrollerからコードを書き始めているのであればそれをやめてみましょう。

コードチェックツールの導入

まずは、ローカルにrubocopを導入しよう。
rubocop -aでの自動修正、またはrubocop -A(unsafeな自動修正)を使うと自ずときれいなコードが書けるようになっていく。

その次の段階として、
rubocop導入の手順をチームに共有したりGithub ActionsなどにCI環境を作れるとgood。
.rubocop.ymlを調整したりjs用のlinterのコードチェックツールの導入などもできるとさらに良い。

サードパーティツールを安易に導入しない

素晴らしいgemやjsライブラリは多いがやたらめったら入れればいいというわけではない。
Railsの標準から離れれば離れるほど運用ルール、メンテナンスコスト(アップデートリスク)、新規参入者の学習コストなどが増える。
導入はメリット・デメリットを把握して問題ないという判断の上で。

宣伝

こういったRailsの技法・tipsをまとめた本をkindle unlimitedで公開中です。
良かったら読んでみてください。
https://www.amazon.co.jp/dp/B0BNKTNV6M

Discussion