リファクタリングは事前準備が９割

会計チームで債権周りの開発をしている hachi (@hachiblog)です。会計チームが開発している freee 会計は freee の中で一番歴史が長いプロダクトです。加えて会計というドメインは複雑かつバグを生むと顧客の業務を大きく阻害するという点で一度作ったものを変更しづらいという特徴があります。

そのような環境で今回、債権のチームでは freee会計の初期からある「自動で経理」という機能の一部リファクタリングを行いました。リファクタリングのしづらい環境下でうまくリファクタリングをすすめるための tips は多くの人に役立つのではと思い、このエントリを書くに至りました。

今回「自動で経理」でリファクタリングしたときに事前に以下のことを行いました。

課題の発見
課題の具体化
設計とスケジュール見積もり
テストコード実装

それぞれについて今回意識したことを書いていきます。

課題の発見

今回リファクタリングした箇所は、チームとしてもともと認識していた課題でした。自分が入社する前に「自動で経理」の実装把握をチームで行っていて、その際にざっくりこのコードは辛いということが認識として共有されていました。つまり今回自分は課題の発見に対して特に苦労せずチームの知見を利用させていただたので工夫したことは一つもないのですが、自分が課題を発見するために普段から意識しているのは作業ログを残すということです。

作業ログはなにに書いてもいいですが、検索できるものが望ましいと思っています。作業ログに書く内容は基本的にやったことや考えたことです。自分は毎日の業務終わりに「良かったこと、嫌だったこと、辛かったこと、面倒だったこと」をそれぞれ書くようにしています。また、後から参照したいものについては文末にハッシュタグを付けて検索しやすくしています。これをしておくと、日々の業務から課題を見つけやすくなります。*1

この作業ログを書くときに自分が一番意識しているのは自分の気持ちに正直になることです。「自分は面倒だと思ったけど他の人は違うかも」とか「自分は理解できなかったけど他の人はぱっと理解できるんだろうな」とか思わないことです。嫌なものは嫌、面倒なものは面倒だと書いてしまうことをおすすめします。そして大体の場合他の人も自分と同じように考えています。

課題の具体化

課題の発見はとても重要ですが、発見だけでは対応方針を決められません。発見した課題について具体的になにが辛いのか、それがリファクタリングで解決できる問題なのかを詳細に見ていく必要があります。今回もともとチームで認識していた課題は「 ActiveRecord の concern クラスとして定義されている hogehoge.rb というファイルを読むのが辛い」という課題でした。今回は自分が発見した課題ではなかったので、より丁寧に言語化を行いました。今回は以下の２つのことを意識しました。

現状をフラットに整理すること
チームで行うこと

現状をフラットに整理すること

辛いと言いつつ、このコードは本番環境で動作している仕様的には正しいコードです。この正しいコードをまずは正しく理解することを意識しました。具体的には以下を意識して理解をすすめました。

対象のコードは外部仕様のなにを満たすコードなのか 当たり前ですが対象のコードはなにかをするために書かれているものです。それがなにをしているのかを正確に把握する必要があります。自分はまず対象のコードがやっていることを一言で言ったあと、分解できるところを分解していくというのをやっています。「要はなにをするコードなのか」ということです。今回は、対象のコードを一言で表すと「自動で経理で明細に対して登録処理の推測情報を紐付ける」ということをしていました。自分はこのとき、「推測情報」を分解するとなんなのかに着目し、推測情報が大きく分けると２つに分けられることに気づきました。これによってこのコードが辛いのは責務が多すぎるからではないかと当たりをつけられました。

対象のコードの図示 今回のリファクタリング対象自体の理解をまず深めるために、関係する周辺クラスの依存関係を図示しました。また、今回のリファクタリング対象は private メソッド同士の依存関係が複雑になっていて読み解くのが難しかったので、メソッド同士の依存関係を図示するというのもやりました。（下図）

これをやっておくことで自分自身の現状把握が進みますし、チームメンバーに現状をわかりやすく伝えることができます。加えてリファクタリングしたあとの状態も設計時に図示することで変更をわかりやすくチーム内外に伝えられるので図にできるときはしておくと良いです。

チームで行うこと

課題の具体化は一人でやるよりチームでグループワークを行うといいです。前述の図示などの整理は事前にやっておくとより詳細に議論できるはずです。チームで具体化したのは純粋に自分一人だと課題を見逃しそうだと思ったためなのですが、期待していた効果以外にも大きく２つ効果が得られました。

一つは課題が一人で進めるより正確になったことです。自分一人では見逃していた課題の曖昧な部分を指摘してもらったり、他のメンバーの言ったことに質問したりすると課題の精度が上がり、課題のコアの部分を浮き彫りにできました。

２つ目はメンバー全員のリファクタリングへのモチベーションが増して協力を得やすくなったことです。もちろんチームでリファクタリング箇所の課題自体は認識していましたが、なにが辛いのか具体的に言語化は行っていないというステータスでした。全員で課題を浮き彫りにするという場があることで、どこが辛いのかを考えて議論するのでリファクタリングをすると解決される問題を自ら明確にし、解決に導くというプロセスに参画できます。他人から伝えられる課題と自ら言語化した課題ではモチベーションは段違いです。

チームで課題の具体化しなかった未来と比較はできないので確実に効果があったとは言えないですが、少なくとも自分がチームメンバーの場合を想像するとこのプロセスは大事だったと感じています。

設計とスケジュール見積もり

解決したい課題に対して設計を考えていくわけですが、リファクタリングで難しいのはやろうと思えば永遠にできてしまうことです。よって設計とスケジュールの見積もりはセットで考えるべきだと言えます。

課題を解決する最低限の設計をするのはもちろんですが、それ以上をどこまでやるかはスケジュールと相談すべきです。今回はチーム内で走っていた同じ箇所の機能実装のプロジェクトと同時にリリースするという計画を立てていたため、そこに合わせる必要がありました。

細かいところで書き方をもう少し練りたい箇所はあったものの、課題を最低限解決する設計と実装方針を立て、スケジュールに少し余裕をもたせるようにしました。結果的にギリギリになったのでやりたいことを詰め込みすぎずに良かったと思います。ここで課題を最低限解決する設計が出来るのも、一つ前のプロセスで課題を明確に具体化していたからです。

テストコード実装は設計に応じて行う

リファクタリングは仕様を変えないのでテストコードが重要であることは皆さんも重々承知かと思います。自分はリファクタリングするコードの呼び出し部分で網羅的にテストを書くことが多いです。そのテストも重要なので多少書くのですが、今回は少し設計を意識したテストコードを書きました。

今回のリファクタリングは推測情報を返す部分で細かく分けるとその推測情報は２つあるという話は前述しました。（推測結果Aロジック, 推測結果Bロジックとします。）加えて、その２つの推測を計算するために呼び出している共通のロジック（共通Cロジックとします）がありました。つまり今回のリファクタリング箇所は大きく３つのクラスに分けられることがわかっていましたし、そのような設計にしていました。

そこで、詳細なテスト自体も３つに分けて書くようにしました。つまり推測結果A、推測結果B、共通C のそれぞれについて網羅的にテストを書くようにしました。こうするとリファクタリング実装をする際にそれぞれの機能についてよりフォーカスできますし、何よりそれぞれのロジックを実装したクラスのテストを実装し直す必要がなく、ほぼコピペでテストが実装できます。テストがコピペ出来るとそれぞれのクラスの実装の確からしさがより担保しやすくなるのでいいやり方だったと思っています。

もっとできたこと

今回のリファクタリングで至らなかったこともいくつかあります。実装した後に同様の推測処理を他の場所でも流用できる事に気づきました。気づいたときに実装を揃えていったのでよかったのですが、ドメイン理解を予めもっと進めておけば考慮に入れて進められたかもしれませんでした。また、今回は機能実装と同時並行でリファクタリングをすすめるという方法をとったのですが、この判断をするのが少し遅れました。自分としては機能実装とリファクタリングを同時にするべきではないと思っていたのですが、今回平行にすすめてみて、同じPRに入れるべきではないが平行にすすめることは不可能ではないということを学びました。