Upgrade to Pro — share decks privately, control downloads, hide ads and more …

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

kidooonn
November 12, 2024

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

kidooonn

November 12, 2024
Tweet

Other Decks in Technology

Transcript

  1. 自己紹介 名前 きどーん (@kidooonn)  (※ 小学生の時からのあだ名です) 所属企業 株式会社TOKIUM。主に「TOKIUM経費精算」にずっと携わってきました。 略歴 1991年、福岡市出身。

    新卒ではメーカーに就職し、海外営業職の仕事をしていました。 そこで「自分の技術でモノづくりがしたい!」と思うようになり、2019年にエンジニアに 転身。Ruby on RailsやVue.js, React.js, Firebaseあたりをよく触ってきました。 その他 最近は専らリファクタリングとパフォーマンスチューニングの事ばかりを考えています。 これらのトピックについてお話ししたい方、是非とも懇親会でお話ししましょうw
  2. 発表しようと思った経緯 Kaigi on Rails 2024 (2024/10/25 - 26)のプロポーザルに応募✊ → ウエイトリスト入りした末、落選 😢

    → このままでは終われない… → ので、今日この場で供養しようと思います 🙌   (機会を作ってくださった皆様に感謝! )
  3. 目次 1. 前提知識 ~技術的負債と循環的複雑度~ 2. 現状分析 ~今のコードは「何がいけないのか」~ 3. 戦術  -

    エクストラクト戦術とスプラウト戦術。戦術の決定  - ルーティング戦術とフェーズ設計  - 「検出用変数」パターンとその応用
  4. 1. 前提知識 〜技術的負債と循環的複雑度〜 🤔 循環的複雑度 とは? ざっくり言うと、「そのクラスの中に幾つの分岐があるか」を数値化したもの 循環的複雑度
 複雑さの状態
 バグ混入確率


    10以下
 非常に良い構造
 25%
 30以上
 構造的なリスクあり
 40%
 50以上
 テスト不可能
 70%
 75以上
 いかなる変更も誤修正を生む
 98%

  5. 2. 現状分析 〜今のコードは「何がいけないのか」〜 疑似コード : ↓下記のような実装が 2000行にも渡っていた わかった事 (認知負荷の高さの理由): 1.

    テストコードは信用できない a. テストコードはあるにはあるが、本来は 仕様・設計に対してテストを書くべきも のの、何に対してのテストなのか不明 2. 全てのユースケースが、全ての分岐を 通りうる。コードを1行変更するだけ でも全てのユースケースへ影響する事 となり、変更のリスクが高い 3. そもそも誰も、仕様の全てを把握でき ていない 敵は大きいけど、困難を「見える化」でき た事は大きな進捗 🙌
  6. 3. 戦術 ここから、上記で出た課題それぞれに対してどう向き合ったかを解説します! 1. テストコードは信用できない a. テストコードはあるにはあるが、本来は仕様・設計に対してテスト を書くべきものの、何に対してのテストなのか不明    → 3-1. 戦術

    - エクストラクト戦術とスプラウト戦術について 2. 全てのユースケースが、全ての分岐を通りうる。コードを1行 変更するだけでも全てのユースケースへ影響する事となり、変 更のリスクが高い 3. そもそも誰も、仕様の全てを把握できていない   → 3-2. 戦術 - ルーティング戦術とフェーズ設計   → 3-3. 戦術 - 「検出用変数」パターンとその応用
  7. 3-1. 戦術 - エクストラクト戦術とスプラウト戦術について 1. エクストラクト戦術 a. 既存のコードにテストを書いて保護しながら、解体してゆく 2. スプラウト戦術

    a. 既存のコードへのテスト追加は諦めつつも、せめて新しく追加するコードについてはテストで保 護する 🤔 どちらで進めるべきか? 参考書籍: 「レガシーコード改善ガイド」
  8. 3-1. 戦術 - エクストラクト戦術とスプラウト戦術について 1. エクストラクト戦術 a. 既存のコードにテストを書いて保護しながら、解体してゆく 2. スプラウト戦術

    a. 既存のコードへのテスト追加は諦めつつも、せめて新しく追加するコードについてはテストで保 護する 結論:スプラウト戦術をベースにして新しいクラスを作成/リプレイスしつつ、そこ で書いたテストコードのみ既存のコードへも転用する形で一部エクストラクトに 理由: 1. そもそも既存のテストコードが信用できるものではなかった a. Railsのshared_contextを多用し、「明らかに現実で起きないケース」もテストしていた 2. 両方で見積もった結果、エクストラクトでいくと物凄い工数に a. 1つのユースケースのみで使われる分岐もあり、それを他のユースケースでもテストする事になるなど
  9. 3-2. 戦術 - ルーティング戦術とフェーズ設計 メリット 1. まさに「小さく安全に進める」事ができる a. 実装したフェーズ以外のケースでは古い実装に向かうので、リスクを局所化できる b.

    フェーズが進む毎に、下記の画像の「if routing == ‘フェーズ⚪⚪’」の分岐が増えるイメージ 2. 新しいクラスを作成するという事で、既存のコードに極力振り回されない a. 新しいクラスでのテストカバレッジは100%。そのテストコードを既存の古い実装にも流すする事で、振る舞 いが変わらずリファクタされている事を担保 3. 仕様化が非常にしやすい a. 今まで仕様が無かったところを、新しい実装では仕様をちゃんと定義するため 4. 1つめのフェーズが完成するまでが山場。以降はどんどん早くなり楽しくなる 🙌 a. フェーズ1で完成したコンポーネントのうち幾らかは、後続のフェーズでも流用できるため 疑似コード(再掲):
  10. 3-3. 戦術 - 「検出用変数」パターンとその応用 😺 そうだ!「フェーズ単位で」見るべきコード、 IN/OUT (引数/戻り値) のパターンを洗い出すよう にしよう!

    そのために「各変数/パラメーターの組み合わせか らフェーズを特定する」ようにする。これにより、 フェーズ毎に2,000行あるソースコードのどこに着 目すればいいのかも明確化 🙌 (elsif条件を充実させる事で、右図でいうelse句を 通らなくなれば解析が完了) (後付けですが) 「検出用関数」と呼べたりしそう 🤔