“社内つらみトップランカー”を210プルリクで倒しきった話　技術的負債は“継続して返却”すればいつかは終わる

登壇者の自己紹介

川原万季氏：「歴史あるプロジェクトのとある技術的負債を隙間プロジェクトの210PullRequestsで倒しきった話」というタイトルでお話をさせていただきます。よろしくお願いします。

先ほど「Twitter」に資料を上げたので、手元でご覧になりたい方は、そちらをご覧ください。

自己紹介をします。「makicamel」という名前で活動をしています。Rubyとビールとお酒が大好きで、好きなVRゲームは「Beat Saber」です。株式会社アンドパッドというところで働いていて、今日はこの話をします。

最後までやりきるのが難しい“大きな技術的負債”

さて、みなさんのプロジェクトに技術的負債はありますか？　ある方が多いんじゃないかなと思うんですけれども、返していくのはなかなか難しいですよね。

というのも、小さな負債はカッとなってシュッと返せばいいのですが、大きな負債になるとなかなかそうもいかず、1日、2日ではなかなか終わらないので、計画を立てて戦略的に倒していくのがセオリーかなと思います。

とはいえ、なかなか優先度が上がらず後回しにされて、放置されて、利息が膨らんでしまいがちだなと感じています。

始めるの自体はそんなに難しくないんですよね。ですが、それを継続して最後までやりきる難易度が高いなと感じています。でも、やりたい。だって、楽しくコードを書きたいから。

アンドパッドのつらみトップランカー「CrudController」

というわけで、今日のお話です。まず、背景の説明からしていきたいと思います。アンドパッドの中で一番古い歴史あるプロジェクトですね。7年間で大きく育ったRailsアプリで、毎日数十人が触るようなプロジェクトになっています。

7年も育ってきたら、技術的負債もたくさんあるわけですね。複雑な設計・実装、神テーブル、神モデルとか、とにかく一つひとつもなかなかつらい負債なんですが、これらが組み合わさることで、より複雑なつらい負債となっています。

今日の対象は、「CrudController」というモジュールです。これは開発初期を支えたオレオレフレームワークです。最初の、コミットの2つ目にCrudControllerが出てくるくらい、開発初期から支えてくれたものなんですが、アプリの成長に合わせたメンテナンスが行われず、最近では負債化してしまっていました。

「ANDPAD」のつらみを話す時には、必ず名前が出てくるような存在になっていて、何年も前から課題として紹介されていたり、撤廃の手順書が書かれるんだけれども、解消がなかなか進まないという状態になってしまっていました。

暗黙的なメソッド定義・暗黙的なコールバック…「CrudController」のつらみ例

そんなCrudControllerがどんなものかというと、こんなふうにコントローラで2行書くだけで、（スライドを示して）こんな感じでめちゃめちゃいっぱいのメソッドが生えたようにいい感じに振る舞ってくれるというものです。

つまり、CrudControllerというモジュールをincludeすると、クラスメソッド「crud_controller」が生えるんですね。それでアクションメソッドを生やしてくれたり、サブのメソッドをいろいろ生やしてくれて、いい感じに振る舞ってくれます。

さらに、そのいい感じのものをカスタマイズしたい場合は、メソッドをオーバーライドして、レスポンスをカスタマイズできるというものになっていました。「すごく簡単で便利で、タイプ数も激減するし、めっちゃ良さそう」という感じはするのですが、まぁ、やはりそれだけでもないわけですね。

例えば、暗黙的にメソッドを定義してくれるんですが、暗黙的なので、動的なメタプロ（メタプログラミング）がされるんですね。表側には一切出てこないので、「メソッド定義、newもcreateもないけど、なんで動くの？」みたいな感じになったり、コールバックも暗黙的に追加してくれるので、便利なんですが、「ここの@userはいったいどこから来たの？」みたいな感じになってしまう。

ほかにも、ANDPADのアンチパターンとして、超ファットコントローラなんですね。それをさらにモジュールに分割してあるので、これらを全部読まないとどんな挙動になるとかまったくわからない。

また、先ほどもお伝えしたように、メタプロが使われているので、メタプロが苦手な人にはちょっと読みにくいなど、複雑な状態になっていました。

バグを生みやすいし、学習コストも高い。オンボーディングの方もそうですよね。あと、認知コストが高く大変な状態になっていました。また、利用しているコントローラが300ぐらいあって、1日2日でどうにかなる量じゃないという感じです。

技術的負債を倒しきるためにやった4つのこと

そこで、倒しきるために（スライドを示して）以下の4つのことをやったりとか、心掛けたりしていました。

まず1つ目、スクリプト化ですね。手作業でやることは考えていなかったんですね。時間と根気と集中力が必要だし、あくまでも本業が別にあって、その隙間時間の取り組みとしてやっていたので、これは選択肢には入っていませんでした。

逆に、部分的にではありますが、撤廃の手順書が書かれていたので、書かれているんだったらスクリプト化できるんじゃないかなと思って、書いてみました。

ちょっと長いんですけど、書けたんですね。長めなんですが、やっていることはそんなに難しくないので、順を追って見ていきたいと思います。

やったことその1　スクリプト化

まずは1つ目ですね。「crud_controller .*アクション名」というのを含むファイルを抽出します。Rubyは、スクリプトの中でバッククォートで囲んだものをコマンド実行できるので、まず「git grep -l 'crud_controller .*new' -- 'app/controllers'」しますね。

そうすると、対象となるcrud_controllerを含んでいるファイルが抽出されるので、そのコントローラのクラス名を取得して、specがあるものから最初はやろうと思っていたので、そのspecがあるコントローラを抽出します。

その対象がわかれば、コントローラファイルを文字列として読み込んで、File.readすることで、文字列として扱えます。文字列として扱ってしまえば、もうこっちのもんですよね。

正規表現を使ったりsubを使ったりして、ふだんと同じように文字列を切り貼りします。最大でここが20個ぐらい生えるんですよ。24個ぐらいメソッドが生えるので、ここがちょっと長いんですが、これで変更した文字列をコントロールファイルに書き戻します。File.writeで書き戻して、「RuboCop」をかけて、コミットを積む、までをやっていました。

今回は、文字列としてファイルを整形したり、ファイルをいじったりしたのですが、カスタムCopを作ってauto-correctするのも良さそうだなと思っています。

スクリプトを書くと何がいいかというと、継続的に負債を返却できるようになるんですね。というのも、コマンドを打つだけで、diffどころかコミットの作成まで完了するので、隙間時間で行えるようになります。例えばミーティング前の10分とか、空かないですか？

そういう時に10分どころか1分もあればdiffが作れます。書いたとおりに動くし集中力不要でできるので、継続的にやりやすくなります。

また、この時は手元でコマンドを打っていたのですが、「GitHub Actions」とかでプルリクの作成まで自動化するのも良さそうだなと思います。

スクリプトにするともう1個いいことがあって、作業手順を忘れられるんですね。ということは、いつでもやめられるし、いつでも再開できる。実際、本業が忙しくなった時に、途中で「ちょっと今は無理だな」と思って1年弱放置したことがあります。その間に、このCrudControllerの振る舞いを知らないことで障害が起きてしまったんですが、「あぁ、あれはやはりやったほうがいいよな」と思って再開した時もハードルが低く、再開ができました。

やったことその2　変更リスクの削減

2つ目ですね。変更リスクを減らすのを心掛けていました。ボーイスカウトルールというものがあります。『コードに汚い部分を残したままにすることは、誰から見ても「してはならないこと」なのです』というのがあるんですが、これをあえて守らないようにしていました。

どういうことかというと、変更の関心事を少なく保つようにしていたんですね。例えば、「crud_controller User, [:update]」と呼ぶと、search_oneというメソッドが生えます。さっきのメタプロで生やされるやつですね。

ただこれ、@userというインスタント変数にセットするのは、before_actionの中ではなくて、update内でやったほうが、良さそうだなと思って、そういうふうにしたんですが、そうすると、別のコールバックで呼ばれていた@userがnilになってエラーになるということがありました。

コード品質に関心がある人、ボーイスカウトルールを守る人ほど、ついでに直してしまいがちだと思うんですね。でもそうすると、関心事が増えてバグの混入に気がつきにくくなるので、それはそれで別として、CrudControllerの撤廃は撤廃として、さらなるリファクタリングは、また次の変更で直そうというようにしていました。

また、（スライドを示して）こんなプルリクがあります。勘のいい方はお気づきかもしれませんが、Commitsが43、Files changedが33、diffが「+584-207」で、けっこう大きな変更になっています。これが障害を起こしました。

確かに変更量は多かったんだけど、テストがあったのになんで気がつかなかったんだろうと思って、よくよく見てみたら、テストの中にexpectがなくて検知できなかったんですね。なにもテストしていない、いわゆる偽陰性のテストになっていました。

つまり、テストはあるけどテストをしていない状態だったんですね。これはさすがに二度と起こしたくないので、expectationのないitを検知するカスタムCopを作って導入しました。

ちなみにこれは、@r7kamuraさんが「rubocop-rspec」の本体にパッチを送ってくれて、今ではrubocop-rspecで利用できるようになっています。

また、ちょっと話が戻りますが、変更量を少なく保つというのもやっています。基本的にやっている方も多いんじゃないかなと思うんですが、機械的な変更もプルリクを分割すべきかというと、そこは議論の余地があるなと思っていて。

というのも、RuboCopのauto-correctとかは、何千、何万行という感じになったとしても、そんなに分けないと思うんですよ。そこは、安全性とコストのバランスだなと思うんですが、今回は安全性を取って、小さくプルリクを分割するようになりました。

というのも、さっきお伝えしたとおり、最初はテストがあるところからやっていたのですが、テストがないところもあって、追加しながらやっていたりとか、手作業でちょっと仕上げをしたりというところもあったので、今回は安全性を取って、小さくプルリクを分割しました。

これは1年と200プルリクエストぐらいかかったんですけど、これがなかったら、正直もうちょっと少ない日数とか、もうちょっと少ない、100行かないくらいのプルリクエストで完了できたんじゃないかなと思います。でももし障害が起こったら、めちゃめちゃモチベーションが下がるんですよね。そうすると継続できないので、今回は安全性を取って、こういうふうにしていました。

変更リスクを減らすというのは、以下のようなことですね。プロジェクトを進めていると、安全な変更を阻害する要因がいくつか出てくるんですね。それをただ、そのまま場当たり的にするというよりも、具体化して、それを地道に排除するということをやっていました。

やったことその3　チーム戦で進める

3つ目はチーム戦にする、ですね。私たちはチーム開発をしているので、レビューを依頼するわけですが、最初の頃は、コード品質に関心が高い人にレビューを依頼していました。でもそうすると負債返却がコード品質に関心が高い人の関心事になってしまって、みんなの関心事じゃなくなるんですよね。

そこに分断が発生してしまうのはすごく良くないなと思って、途中からコードのオーナーっぽいチームにレビュー依頼をするようにしました。これで負債をみんなの関心事にするようにしました。

また、最初スクリプトを手元に置いてやっていたのですが、それをrakeタスクにして、masterマージして、誰でも取り組めるようにしました。

それからもう少しして、メンバーが手を挙げてくれたので、2人で取り組むようになりました。これ、すごくよかったです。レビューが爆速で回るんですよ。お互いにプルリクを作って、レビューし合えるようになるので、レビューがすごくスムーズに回るようになってありがたかったです。ほかにも、進め方の改善を提案してくれて、倍速で撤廃が進むようになりました。ありがたかったですね。

これがすごくよかったんですけど、なんといっても1人でやっていると、飽きるんですよ。さすがに1年間も同じことをやっていると、飽きる。

ですが、人と一緒にやるとコミュニケーションが発生するので、すごく楽しいんですね。なので、チーム戦、隙間プロジェクトであっても2人以上でやるというのは、すごくおすすめです。

やったことその4　モチベーションを保つ

最後ですね、モチベーションを保つということなんですが、先ほどもちょっとお話が出たんですが、飽きるんですね。社内でCrudControllerに一番詳しい人というのをやっていたので、「もうみんな完全に理解してしまって、困っている人はいないんじゃないかな」と思い始めるんですが、やはりそんなことはないわけなんですよ。

けれども、自信がなくなってくるので、モチベーションを保つために、まず進捗を見える化するというのが、1つおすすめです。

数字ですね、何パーセント終わったかとか、あと何ファイル残っているんだろうというのをやることで、進んでいることが目に見えてわかるので、すごくうれしくなります。

あとは、私はけっこう長い期間1人でやっていたので、手元でコマンドを打って確認していたのですが、チームでやる場合、複数人でやる場合は、例えば「Slack」に通知とかしていると、みんなの目に見えてよいかなと思います。

また、褒めてもらうというのもすごく大事だなと思っています。timesで「なになにが終わった」とつぶやくと、みんな優しいので、リアクションをすごいつけてくれるわけですよ。

そうすると、「あっ、これはちゃんとやっていて意義があることなんだな」と自分でも認識できるので、褒めてもらうというのもすごくよかったなと思っています。

また、「開発本部自慢大会」というのを会社で月1でやっているんですが、この自慢大会で、CrudController撤廃完了を自慢した時も、みんなが「すごい、すごい！」とすごく言ってくれて、「あぁ、興味関心を持ってくれてたんだな」というのがわかって、これでまた次の負債の返却もがんばろうと思うことができました。

ということで、CrudControllerの撤廃が完了しました。