社内で最大の技術的負債のリファクタリングに取り組んだお話し

Transcript

1. 社内で最大の技術的負債のリファク タリングに取り組んだお話し After Kaigi on Rails LT Night (2024/11/12) kidooonn ～循環的複雑度が1,400を超えたソースコードの解体プロジェクト～
2. 自己紹介 名前 きどーん (@kidooonn)　 (※ 小学生の時からのあだ名です) 所属企業 株式会社TOKIUM。主に「TOKIUM経費精算」にずっと携わってきました。 略歴 1991年、福岡市出身。 新卒ではメーカーに就職し、海外営業職の仕事をしていました。 そこで「自分の技術でモノづくりがしたい！」と思うようになり、2019年にエンジニアに 転身。Ruby on RailsやVue.js, React.js, Firebaseあたりをよく触ってきました。 その他 最近は専らリファクタリングとパフォーマンスチューニングの事ばかりを考えています。 これらのトピックについてお話ししたい方、是非とも懇親会でお話ししましょうw
3. 発表しようと思った経緯 Kaigi on Rails 2024 (2024/10/25 - 26)のプロポーザルに応募✊ →　ウエイトリスト入りした末、落選 😢 →　このままでは終われない… →　ので、今日この場で供養しようと思います 🙌 　　(機会を作ってくださった皆様に感謝！ )
4. 目次 1. 前提知識 ~技術的負債と循環的複雑度~ 2. 現状分析 ~今のコードは「何がいけないのか」~ 3. 戦術 　- エクストラクト戦術とスプラウト戦術。戦術の決定 　- ルーティング戦術とフェーズ設計 　- 「検出用変数」パターンとその応用
5. 1. 前提知識 〜技術的負債と循環的複雑度〜
6. 1. 前提知識 〜技術的負債と循環的複雑度〜 🤔 技術的負債のマズさや規模、客観的な指標で計測できてますか？
7. 1. 前提知識 〜技術的負債と循環的複雑度〜 🙌 RubyCriticを使う事で、客観的に計測できます！グラフも見やすい 横軸(Crurn)が「変更頻度」、縦軸が(Complexity)が「循環的複雑度」 要は、グラフの右上に位置するクラスほど、リファクタリングの期待値が高 い。
8. 1. 前提知識 〜技術的負債と循環的複雑度〜 🤔 循環的複雑度 とは？ ざっくり言うと、「そのクラスの中に幾つの分岐があるか」を数値化したもの 循環的複雑度
 複雑さの状態
 バグ混入確率
 10以下
 非常に良い構造
 25%
 30以上
 構造的なリスクあり
 40%
 50以上
 テスト不可能
 70%
 75以上
 いかなる変更も誤修正を生む
 98%

9. 2. 現状分析 〜今のコードは「何がいけないのか」〜
10. 2. 現状分析 〜今のコードは「何がいけないのか」〜 🤔 循環的複雑度が75以上だと『いかなる変更も誤修正を生む』らしい。では、 今回の解体対象となるクラスは？
11. 2. 現状分析 〜今のコードは「何がいけないのか」〜 ？？？「私の循環的複雑度は1,400です」
12. 2. 現状分析 〜今のコードは「何がいけないのか」〜 🤯 循環的複雑度1,400…🤯 循環的複雑度が75以上で「いかなる変更も誤修正を生む」ってことは… 理論上、一度の改修で、18.6個の不具合が生まれる…ってコト！？？
13. 2. 現状分析 〜今のコードは「何がいけないのか」〜 🤔 もう少し、解体対象のクラスを分析してみた (自分で分析したり、チームに聞 いてみたり)
14. 2. 現状分析 〜今のコードは「何がいけないのか」〜 疑似コード ： ↓下記のような実装が 2000行にも渡っていた わかった事 (認知負荷の高さの理由)： 1. テストコードは信用できない a. テストコードはあるにはあるが、本来は 仕様・設計に対してテストを書くべきも のの、何に対してのテストなのか不明 2. 全てのユースケースが、全ての分岐を 通りうる。コードを1行変更するだけ でも全てのユースケースへ影響する事 となり、変更のリスクが高い 3. そもそも誰も、仕様の全てを把握でき ていない 敵は大きいけど、困難を「見える化」でき た事は大きな進捗 🙌
15. 3. 戦術
16. 3. 戦術 ここから、上記で出た課題それぞれに対してどう向き合ったかを解説します！ 1. テストコードは信用できない a. テストコードはあるにはあるが、本来は仕様・設計に対してテスト を書くべきものの、何に対してのテストなのか不明 　　　→　3-1. 戦術 - エクストラクト戦術とスプラウト戦術について 2. 全てのユースケースが、全ての分岐を通りうる。コードを1行 変更するだけでも全てのユースケースへ影響する事となり、変 更のリスクが高い 3. そもそも誰も、仕様の全てを把握できていない 　　→　3-2. 戦術 - ルーティング戦術とフェーズ設計 　　→　3-3. 戦術 - 「検出用変数」パターンとその応用
17. 3-1. 戦術 - エクストラクト戦術とスプラウト戦術について 1. エクストラクト戦術 a. 既存のコードにテストを書いて保護しながら、解体してゆく 2. スプラウト戦術 a. 既存のコードへのテスト追加は諦めつつも、せめて新しく追加するコードについてはテストで保 護する 🤔 どちらで進めるべきか？ 参考書籍: 「レガシーコード改善ガイド」
18. 3-1. 戦術 - エクストラクト戦術とスプラウト戦術について 1. エクストラクト戦術 a. 既存のコードにテストを書いて保護しながら、解体してゆく 2. スプラウト戦術 a. 既存のコードへのテスト追加は諦めつつも、せめて新しく追加するコードについてはテストで保 護する 結論：スプラウト戦術をベースにして新しいクラスを作成/リプレイスしつつ、そこ で書いたテストコードのみ既存のコードへも転用する形で一部エクストラクトに 理由: 1. そもそも既存のテストコードが信用できるものではなかった a. Railsのshared_contextを多用し、「明らかに現実で起きないケース」もテストしていた 2. 両方で見積もった結果、エクストラクトでいくと物凄い工数に a. 1つのユースケースのみで使われる分岐もあり、それを他のユースケースでもテストする事になるなど
19. 3-2. 戦術 - ルーティング戦術とフェーズ設計 🤔 サービスの中でも特に根幹となるクラスである以上、特に不具合が出た時の 影響は大きい (即インシデント)。安全に進めたい。ビッグバンリプレイスではな く、小さいリリースを繰り返し1つ1つの影響範囲を極力小さくしたい
20. 3-2. 戦術 - ルーティング戦術とフェーズ設計 🤔 サービスの中でも特に根幹となるクラスである以上、特に不具合が出た時の 影響は大きい (即インシデント)。安全に進めたい。ビッグバンリプレイスではな く、小さいリリースを繰り返し1つ1つの影響範囲を極力小さくしたい 結論：「アクター」「ユースケース」「オブジェクト」の組み合わせの単位で フェーズを切る。クラス内部でルーティングを切る事で、それら1つ1つを独立し て進める方針に。
21. 結論：「アクター」「ユースケース」「オブジェクト」の組み合わせの単位でフェーズを切る。クラス内部でルー ティングを切る事で、それら1つ1つを独立して進める方針に。 具体的には、下記のスクショの黄色部分をフェーズ1と命名 (フェーズは全部で21個)。ルーティングを設け、この新 規実装したフェーズのみ新しい分岐(綺麗なコンポーネント)を通るようにする。そのフェーズに該当しない時には今 までの実装に向かうようにする。 更に、Phase⚪⚪クラス(新しいクラス)のテストを書いたら、そのテストを既存の古い実装にも流す事で、振る舞いが 変わらない事を担保する。 3-2. 戦術 - ルーティング戦術とフェーズ設計 疑似コード：
22. 3-2. 戦術 - ルーティング戦術とフェーズ設計 メリット 1. まさに「小さく安全に進める」事ができる a. 実装したフェーズ以外のケースでは古い実装に向かうので、リスクを局所化できる b. フェーズが進む毎に、下記の画像の「if routing == ‘フェーズ⚪⚪’」の分岐が増えるイメージ 2. 新しいクラスを作成するという事で、既存のコードに極力振り回されない a. 新しいクラスでのテストカバレッジは100%。そのテストコードを既存の古い実装にも流すする事で、振る舞 いが変わらずリファクタされている事を担保 3. 仕様化が非常にしやすい a. 今まで仕様が無かったところを、新しい実装では仕様をちゃんと定義するため 4. 1つめのフェーズが完成するまでが山場。以降はどんどん早くなり楽しくなる 🙌 a. フェーズ1で完成したコンポーネントのうち幾らかは、後続のフェーズでも流用できるため 疑似コード(再掲)：
23. 3-3. 戦術 - 「検出用変数」パターンとその応用 🤔 「検出用変数」とは: デッドコードを検出するために使われる手法。下記のコードでいう「found」変 数が例。意図しない振る舞いになった際に出力をする (GPT先生に頼んで出してもらいました) 。 参考書籍: 「レガシーコード改善ガイド」
24. 3-3. 戦術 - 「検出用変数」パターンとその応用 🤔 「検出用変数」を使って、「デッドコード」「変数の振る舞い/発生条件」を 検査しよう。本番環境にデプロイして、「検査結果をDatadogに通知する」よう に。 各変数の使われ方/振る舞いを、自力で完全に 検知できる自信がありませんでした。 ならば「ユーザー(お客様)に検知してもらお う」という逆転の発想です🖊
25. 3-3. 戦術 - 「検出用変数」パターンとその応用 🤔 「検出用変数」もっと応用できそう
26. 3-3. 戦術 - 「検出用変数」パターンとその応用 😺 そうだ！「フェーズ単位で」見るべきコード、 IN/OUT (引数/戻り値) のパターンを洗い出すよう にしよう！ そのために「各変数/パラメーターの組み合わせか らフェーズを特定する」ようにする。これにより、 フェーズ毎に2,000行あるソースコードのどこに着 目すればいいのかも明確化 🙌 (elsif条件を充実させる事で、右図でいうelse句を 通らなくなれば解析が完了) (後付けですが) 「検出用関数」と呼べたりしそう 🤔
27. まとめ： ここまでの方針・計画をもって着手する事で、巨大なクラスを「小さく安全にリ ファクタする」事に成功しました 🙌 (リリース後の不具合も0件です) 社内で最大の技術的負債の解消に一役買うことができて満足 🙌 3-3. 戦術 実際は「実行」の段階に入った後に別の困難にぶつかりもしたのですが、今回は時間の都合でここまで 🖊
28. 以上です！ ご清聴ありがとうございました！ 皆様の携わっているプロダクトの技術的負債解消に際し、何か1つでも参考にな る情報があれば幸いです。 技術的負債とダンスしつつ、楽しいRailsライフを 🙌 この機会を作ってくださった皆様に、本当に感謝です！🙇