アフターコロナ需要拡大の準備として、1年間リファクタリングプロジェクトをやってみた 〜メトリクスの導入から効果の検証まで
株式会社ロコタビは海外在住日本人と日本在住日本人をつなぐスキルシェアリングサービスを運営している会社です。
海外旅行がほぼ壊滅的なコロナの状況で、他の旅行会社と同じく、かなりコロナの影響を受けています。ただ、他の旅行会社と違いシェアリングサービスなので、コロナが明けてからいち早く立ち直るポジションにいるのではないかと思っています。
アフターコロナの需要拡大の準備として、2021年度はリファクタリングプロジェクトに取り組みました。
なお、リファクタリングを実施したフレームワークは、Ruby on Railsになります。
リファクタリング前の準備
目標と方針を共有する
ビジネスサイドの理解を得られなくてリファクタリングできないという話を時々聞いたりします。
ロコタビの場合は、前々からコードの状態がよくないのでリライトかリファクタリングを検討すべきみたいな話が何度か出ていて、アフターコロナの施策として何ができるかと考えた時に、コードの負債を減らしておくことが重要だとの意見で合意できたのはかなり幸運でした。
最初になぜリファクタリングをするのか？目標を共有しました。
「アフターコロナのロコタビの需要拡大に対応するため、ロコタビのシステムを開発・運用しやすい状態にする」
次にリファクタリング対象を決めました。
コードに接しているエンジニア感覚では、もっとも負債が溜まっていると思われるのは「オーダーのコード」と「ユーザーのコード」。その２つをリファクタリング対象としました。
今後の方針を決めました。
最初に経験に決めたリファクタリング対象が正しいか、コードメトリクスによって確認する
リファクタリングによる成果をメトリクスによって計測して、成果を確認する
Rubocopに任せられるようにする
リファクタリングプロジェクトと言いながらも、１人チーム。（レビューしてくれるメンバーを入れれば2人）１人でできることは限られています。
リファクタリングに手をつけない部分は、現状をキープして更なる負債が増えないようにツールに任せることにしました
具体的には、Rubocopを使っているのですが、
かなり古いバージョンを使っている
無効ににしている項目がかなりある
十分にRubocopが機能していない状態でした。Rubocopのバージョンを最新にして、無効にしている項目を有効にすると警告が３万個くらい出てきました。
３万個は修正箇所が多すぎるので、最初は警告の修正には着手せず、新しいコードに対して最新のRubocopが自動でチェックしてくれる状態にしました。（最終的にはある程度修正しました）
Rubocopのバージョンアップは、rubocop のしつけ方を参考にしました。
進捗が見えるようにする
進捗が見えるようにするため、従来から使っているCI
GitHub＋CircleCI
に加え、
BigQuery＋Re:dash
にて、メトリクスデータを蓄積して可視化できるようにしました。
収集するデータとしては、
Rubocopの警告（通常のチェックでは無効にしている項目も有効にしている）
rubycriticの結果（コードのメトリクス）
rake statsの結果（プロジェクトの大まかな統計情報）
GitHubへのプッシュ時に、CircleCIにてデータ収集してJSON形式に加工し、BigQueryに保存して、Re:dashで表示するようにしました。
Rubocopの警告箇所が３万個と書きましたが、無闇に着手するといつ終わるのか想像もつかなくて、正直、気持ちが折れそうになります。
以下のグラフはRe:dashで表示させたRubocopの違反の項目数の残数の推移です。
結局、修正するのに2ヶ月くらいかかりましたが、日々警告数が減っているのを確認しながら、修正を進めることができました。
Rubocopの警告のうち何から修正するかは、違反している規約と、違反箇所、自動修正可能かどうかのテーブルを作って、自動修正可能で数が多い項目からターゲットを決めていきました。
Rubocopの警告の修正に関しては、特にテクニック的なものはないのですが、基本的に一つのPRに一つの項目として、自動修正の場合でも、レビューで確認しながら地道にコツコツと一つずつ潰していってきました。
rubycriticによるメトリクスの検証
rubycriticは見やすいUIを備えているのですが、継続的にメトリクスのデータを蓄積して可視化するようなことまでは出来ないないので、BigQueryとRe:dashでデータの可視化を行いました。変更頻度と複雑さをグラフにプロットしてみると、
「オーダーのコード」と「ユーザーのコード」の状態の悪さが予想通りぶっちぎりの２トップの状態でした。
プロダクト全体のメトリックスをグラフにしてみるとこんな感じです。
可視化できたからといって、すぐにコードの品質が上がるわけではないですが、プロダクトの成長と負債がどんなふうに積み上がってきたのかが分かります。今、どんな状態なのか？リファクタリングによってどう変わったのか？今後もどんな状態なのかが俯瞰できるようになりました。
リファクタリングの準備として、
今後の負債を最低限にするためのRubocopの正常化
現状の負債状況を見えるようにすための継続なメトリクス測定
感覚的にこのコードが問題だと思っているターゲットをメトリクスで確認する
を行いました。準備に3ヶ月くらいかかかりましたが、ここからがリファクタリング本番です。
リファクタリングどうやってやるか？
大きく整理する
最初にリファクタリング対象にした「オーダーのコード」は、MVCで言うところのモデルのコードになります。
一つのファイルが1000行以上あったので、何がどうなって、どこから手をつけていいのか？という状態だったので、最初のステップとしては、もっとも基本的なメソッドの移動を行いました。
Rubyのコードなので、機能ごとにmodule化して、ファイルを分けて、includeしていきました。１ファイルのコード量は減るので、若干、見通しはよくなりますが、根本的に何も解決してません。
ぐちゃぐちゃのにモノが詰まった引き出しを整理するのと同じで、一旦、中に入っているものを引き出して分類することで、コードの理解も進みます。ファイルをかけることで対象を小さく出来る、移動だけなのでリスクが少ない、など最初のステップとしては、ちょうどよかったです。
ここで意識したのは、テストで保護されていないコードもあったので、
メソッドの移動だけに徹する。メソッド名や内容が気になっても修正しない。
一つのPRには、一つのmoduleへの切り出しだけにして、レビューをしやすくする。
テストで保護する
次のステップとして、定番ですがリファクタリング対象をテストで保護しました。
C++のような言語であれば、対象をテスト入れるだけで、かなり面倒だったりするのですが、RubyはMockを入れるにしても、偽装クラスを作るにしてもかなり楽な言語です。
全てのテストを最初に用意するのは、マンパワー的にもモチベーション的にも無理だったので、「大きく整理する」で範囲を絞って、その範囲のテストを書いて、その範囲をリファクタリングする、のようなステップで進めていきました。
テストの書き方として、「仕様化テスト」がとても役に立ちました。
仕様化テストは、コードの振る舞いを明らかにするためのテストです。「システムはこうするべきだ」とか「こうしていると思う」と言うことを確認するためのテストではありません。仕様化テストは、システムの現在の振る舞いをそのまま文書化します。
具体的には、今回の対象のオブジェクトの場合は、２０種類くらいの状態に分類することができました。
２０種類の状態のオブジェクトを用意する。
それぞれのオブジェクトに対してメソッドを呼び出すテストを書く。最初はテストが落ちるようにする。
テストを実行して、オブジェクトが返してくるメソッドの戻り値を仕様として記録していく。
ここで注意が必要なのは、
バグを見つけても修正しないことに徹する。つまり、バグと気がついても出力をそのまま記録する。バグの修正は別のPRで実施する。
リファクタリングの実施
テストで保護した後に、仕様を壊していないことを確認しながらコードを修正していきます。工事現場でいうと、テストが足場で、建物がリファクタリング対象みたいな感じです。
仕様化テストとリファクタリングは、お互いが型枠みたいな関係で、
仕様化テストを書く時は、コードを固定して型枠として利用する
リファクタイングする時は、テストを固定して型枠として利用する
テストコードと修正対象コードを一緒に修正すると、何が正しいのかわからなくなってしまうので、テストを修正するPRとリファクタリングのPRはめんどくさくても分けたるようにしました。
リファクタリングというと、凝集度・結合度・デザインパターンとか、小難しい設計技術ってイメージが強かったのですが、実際にやることの大部分は、メソッドの抽出、メソッドの移動、メソッド名の修正、制御フラグの削除など、小さな修正の積み重ねです。
ここで意識したことは、
とにかく小さく刻んでいく。時々、修正後の構造が見える気がする時もあります。一気に変えたくなりますが、いくらテストがあるとはいえ原型を留めないほどにリファクタすると、レビューする人も不安になりますし、自分も100％大丈夫と思えなくなったりします。めんどうですが、少しずつ細かくPRを出して、だんだん、理想の構造に寄せていきました。
コードが絡み合っていて、修正がどんどん連鎖していく場合は、試行リファクタリング（レガシーコード改善ガイド）やThe Mikado Methodの考え方が役に立ちました。
レビューしやすいPRを作る
一回の修正範囲をちょうどよいサイズに納める技術が、リファクタリングをする上でかなり大事なんじゃないかと思ったりしました。
人の認知能力には限界があります。
個人的な感覚として、修正ファイル数が10ファイル超えてくると、絶対大丈夫と自信を持っていえないような気がしています。（もちろん、能力の個人差もあり、状況によりけりなんですが、メソッド名の変更で、一つのメソッド名変えたら40ファイルくらい修正になったとかもあると思いますし）
1回の修正範囲を、自分が絶対安全と思える範囲で納めることは、かなり大事だと思います。
変更の範囲が絞れない、そんな時には、試行リファクタリングだと思って最初のPRは捨てることや、The Mikado Methodがおすすめです。
ビューのリファクタリング
先ほどまで、モデルのリファクタリングを前提として書いてきましたが、かなり厄介なのは、ビューやコントローラーに紛れ込んでいるロジックだと思います。
テストを書くにしても、feature テストは、遅いし、壊れやすい。
feature テストで、テスト実行してリファクタリングをやっていると、ひたすらテスト待ちになってしまいます。
今回、すごく役に立ったのが、ViewComponentでした。
ViewComponentに入れることで、ビューの仕様化テストが書けます。そこから、ビュー内のロジックをモデルに移動できます。
ViewComponentは便利なのですが、ここで困った問題があります。ViewComponentの導入のためのテストをどうするか？です。
レガシーコードのジレンマ
コードを変更するには、テストを整備する必要がある。多くの場合、テストを整備するためには、コードを変更する必要がある。
今回は、最低限のfeature テストだけを書きました。ViewComponent呼び出し時にエラーが出ないかどうかくらいのテストです。あまりやりたくないですが初回は手動で確認しました。
そして、ViewComponentを入れるときも、極力最初は、コントローラー内にあるロジックを何も触らずに、丸っとViewComponentに入れることに注力しました。
レガシーコードの依存関係を除去する際には、自分の美的感覚を少し脇におしやらないといけない場合もある。依存関係をきれいに除去できる場合もあれば、理想的な設計にならない場合もある。これは手術の切開部のようなもので、作業後のコードに傷跡が残るかもしれないが、その奥はこれまでよりよくなっているはずである。
依存関係を排除した部分に対して後からテストを作成できるなら、その傷跡を消すこともできる。
DBのリファクタリング
Ruby on Railsの場合、モデル＝テーブルなので、モデルが肥大化している＝テーブルも肥大化していると思います。
コードのリファクタリングが進んでくると、リファクタリング対象のモデルから切り離した方が都合が良いメソッドやデータの塊がぽつぽつと現れてきます。
今後のために分けた方がよいとなったので、クラスを抽出して、DBのテーブルも分離していきました。ただ、テーブルの分離が入ってくると、リリースのタイミングとか、データの整合性を保つために、一時的にデータを２重に持たないといけないとか、リリースの計画・段取りが重要になってきます。
コロナ禍でサイトの利用者が減っていて、DBのリファクタリングをやるなら今しかないというものあり、DBのリファクタリングに踏み込みました。DBのリファクタリングはリスクもコストも高いので、通常は判断に迷うところだろうと思います。
見つけた不具合はどうするか？
リファクタリングしたくなる怪しいコードは、不具合を含んでいることが多いです。
ただ、実害が出た不具合は修正されているので、見つかる不具合は仕様の穴みたいな感じが多かったです。
この場合、本当にこの振る舞いでいいんだっけ？不具合なのかどうなのか？ある特殊な条件で発生する宙ぶらりんのレアケースみたいな感じです。
不具合の場合も基本的にやることは同じで、不具合の仕様化テストを書く。不具合時の振る舞いを確定して、本来のあるべき仕様を確認して、正しいテストに修正してコードを修正する。
1年間やってみての成果と課題
成果
最初に見える化したので、Before、Afterを誰でも分かる形で確認できるようになりました。
ターゲットとしたオーダーのコードの複雑度は下がっています。
ただ、正確にいうと、「大きく整理する」で、他のモジュール切り出したことが一番大きいので、このグラフだけを見て成果があるとは言えないのですが、一番分かりやすいのでよく使っています。
プロダクト全体の状態です。
全体のコード量からすると、コード量はちょっと減少傾向にある程度です。全体の複雑度も徐々にですが改善傾向にあります。
途中、コードが急に増えているのは、ViewComponentを導入したからです。時々スパイクみたいな波形になっていますが、これはデータ取得時のバグです。（ここまで修正する余裕はなかったので、そのままにしてます）
アフターコロナに向けて新しいエンジニアにぼちぼちと入ってもらっているのですが、これまでなら、コードが複雑になっていて、実績のあるエンジニアにでも、修正をお願いすることに躊躇するような箇所がありました。
リファクタリングを実施したことで、一部の熟練エンジニアしか触ることができないようなコードがなくなったことは大きな成果だと思います。
課題
見積もりが、難しいことです。
実質の工数として、ザクっとこんな感じです。
Rubocopの正常化と警告の修正：2ヶ月
メトリクスの測定基盤の作成：2週間程度
オーダーのリファクタリング：6ヶ月
開始時には、「オーダー」と「ユーザー」の２つをターゲットにしていて、ざっくりオーダー3ヶ月、ユーザー3ヶ月としていたのですが、オーダーを終わらせるだけで精一杯でした。
ある程度リファクタリングが進めば工数が見えてくるのですが、初期の段階で見積もるのは不確定要素が多すぎて、なかなか難しいなと感じています。
リファクタリングによる不具合
どんなに慎重にステップを踏んで作業しても間違いは起こします。
大きな不具合はなかったのですが、何個か不具合は出しました。
リファクタリングの不具合で気をつけたことは、
リファクタリングによる不具合は、ゼロにはできないと関係者には本当のことを伝える
リリース時間もバグ対応しやすい時間帯にする
致命的な不具合が出る箇所のリファクタリングを実施した場合には、社内にアナウンスして、少しでも異変に気がついたら、すぐに連絡して欲しいと伝える
まとめ
「アフターコロナのロコタビの需要拡大に対応するため、ロコタビのシステムを開発・運用しやすい状態にする」を目標に、1年間リファクタリングしてきました。
メトリクスを見える化したこともあり、それなりに目に見える形で成果も出すことができました。
リファクタリングに関しては、正直、これが正解というのは、分からないですが、地道にやっていると、そのうち、あれ？これいらんわ、これも重複してるわ、これ違うクラスのメソッドやん、みたいな瞬間があって、
「ぷよぷよ」でいうと、
ファイヤー、アイスストーム、ダイアキュート、ばよえ～ん
みたいな感じで、コードが連鎖的に消える瞬間が何度かありました。めちゃめちゃスッキリする瞬間でした。
とは言っても、技術負債を溜め込まないのが一番なので、普段から小さなリファクタリングをコツコツとするのが大事なんだろうと思ったりします。
現在は違うタスクをしているのですが、このタスクが終わり次第、リファクタリング第2弾に取り組む予定でいます。（コロナが開ければ状況が変わるので、まだ、未確定ではありますが）
次取り組む予定のコードは、今回修正したコードと劣化の具合が違っている気がするので、何か新しい発見があれば、第２弾報告できればと思っています。
参考書籍
今回「仕様化テスト」くらいしか紹介していませんが、テスト書けないと思った時には、色々とヒントが詰まっています。
第17章 私のアプリケーションには構造がありません
第22章 モンスターメソッドを変更する必要がありますが、テストを書くことができません
第24章 もうウンザリです。何も改善できません
英語の本しかないのですが、DeepLがあれば余裕で読めます。
依存関係が絡まっている部分をリファクタリングすると、修正の連鎖が止まらないことがありますが、そんな時には役に立ちます。
大きなリファクタリングでも、小さなマージ可能なPRに分けて、少しずつプロダクトコードにマージしていくようなリファクタリングを可能にしてくれます。
プロジェクトをどう進めるか？を考えるときに、参考になりました。
第3章 リファクタリングの準備
チームのコンセンサスを培う
組織から承認を得る
候補を選ぶ（価値と難度とリスクによる分類）
決断の時（リファクタか、リライトか）
リファクタリングをしなくてはいけないと思うけど、ビジネスサイドに伝わらない。そんな時には参考にしてみてください。
開発者はリファクタリングがどういうものか知っている。だが、それを表現するのに適切な語彙を使う必要がある。つまり、価値とリスクにつながるビジネスの語彙を使う必要があるのだ。
リファクタリングをしたいけど、テストを書く習慣がない。そんな時は、ここから始めるのがいいと思います。
TDDBC
書籍ではないのですが、TDDBCというイベントが時々開催されていることがあります。僕も過去に何度か参加しました。
TDDの最初の一歩を踏み出すためにはおすすめです。
和田さんのTDDの解説動画もお勧めですよ。
［動画で解説］和田卓人の“テスト駆動開発”講座
具体的な手法について紹介されています。
リファクタリングを通して何度もこの本に戻ってくることをお勧めします。
内容的にはリファクタリング（第２版）とほぼ同じです。Rubyの場合、メタブログラミングですんなりできてしまうこともあるので、Rubyを使っている場合はこちらもおすすめです。
Rubyの場合、メタプログラミングで改善できる部分も多いので、メタプログラミングを学ぶのはおすすめです。
今回は、同じような内容のmethodやscopeを、defime_methodを使って規則に従って自動生成するなどは、よく使いました。
ただ、変な使われ方をしてるメタプログラミングを普通の書き方に変えたりもしました。メタプログラム面白いですが、使い所には注意が必要です。
ただ闇雲にリファクタリングしても、メソッドを細切れにして終了となってしまいがちです。
パターンを取り入れる／近づく／離れるリファクタリング
優れた設計者はさまざまな方向にリファクタリングを行うが、その目的は常に、よりよい設計に到達することである。私が行うリファクタリングの多くにはパターンを含まないが（「メソッドの抽出」、「メソッド名の変更」、「メソッドの移動」、「メソッドの引き上げ」といった小さいシンプルな変更なので）、含める場合には、パターンを取り入れるか、パターンに近づけるか、ときにはパターンから離れるように、リファクタリングを行う。
もし、デザインパターンが苦手なら分かりやすいのでおすすめです。
そもそもリファクタリングによって、どんな構造にしたらいいのか？そもそもの設計がよくわからない場合には、厚い本ですが、デザインパターンから設計の原則まで、よくまとまっていると思います。
リファクタリングによって、どういう構造に寄せていくか？を大きな構造で考える時に参考になります。
今回、DBのリファクタリングはそんなにやっていないのですが、DBのリファクタリングに取り組む場合は一読してもいいかもしれないです。