この記事は Akerunのカレンダー | Advent Calendar 2023 - Qiita 6日目の記事です。
ishturk - Qiita です。 この記事ではエンジニアが大好きなリファクタリングについて綴ります。
リファクタリングの理由
ソフトウェアには必ず不具合が潜んでいます。ハードウェアを制御しているソフトウェアだと、タイミングや外的要因（温度や経年劣化など）で発生率が大きく変わったりします。 これまでも、お客様に迷惑をかけないために、検知した不具合をスピーディに修正するサイクルを回してきました。
その一方で、修正を繰り返した結果、当初の設計が歪み、あらたな不具合を埋め込んでしまうという問題が発生してしまいました。具体的には、単一責任の原則・開放閉鎖の原則に則っておらず、機能に影響する調査対象が多いこと、影響範囲が閉じないことをどうにかしないと打破できないと判断しました。
リファクタリングの戦略
1. ターゲットとなる品質問題を定義する
絶対に直すもの・直さないもの を明確にしプロジェクトKPIとします。
エンジニアは目についたものは全部直したくなります。リファクタリングしていいよ？ というと、水を得た魚のようにたくさんのアイディアを出してくれます。
そこは品質問題になってないから、やりません。をチームで明確に決め、直さないものを決めることも非常に重要です。
2. メトリクス：モジュールごとに潜在不具合リスクを定量化
モジュールごとに以下の指標で不具合リスクを定量化し、上位をピックアップしました
- 行数
- 複雑性
- 変更回数
行数・複雑性の可視化
プロダクトコードがJavaScriptで書かれていたため、 plato - npm を利用して静的解析をかけてみました。
行数と複雑性は明らかに正の相関がありました。また、platoが Estimated errors in implementation なる値を出してくれました。傾向がほぼ Lines of code と同じなので、何らかの計算式で正規化しているのでしょう。
変更回数の可視化
過去に不具合修正・デグレを繰り返していることから、変更回数の多いモジュールが品質リスクが高いと考えられます。
変更回数の算出は Gitで更新頻度の高いファイルを見つける方法 - アジャイルSEの憂鬱 が参考になりました。
以下のコマンドを実行して git で取得しました。
git log --name-only --oneline --diff-filter=M --since='5 year ago' | grep -v ' ' | sort | uniq -c | sort -nr | head -n 20 | sed 's/^[ ]*//' | tr ' ' '\t'
実行結果は以下のようになりました。最上位は92回の変更がされていました。
3. 総合的に判断する
1・2 の内容を鑑み、ここは機械的な判断ではなく、シニアクラスのエンジニアが、どのモジュールをどうリファクタリングするか決定します。
- 行数・複雑性の高いモジュールの分割・構造変更
- ターゲットとなる品質問題の影響範囲を洗い出し、関連モジュールを単一責任の原則・開放閉鎖の原則に則った設計へ変更
今回は行数・複雑性の高いモジュールと品質問題を起こしていたコアモジュールが同一だったため、そのモジュールをどう分解するかが設計の肝となりました。
振り返り
正直なところ、分析するまでもなく、直したほうがよいモジュールはわかっていたし、変更の方針も概ね見当がついていました。 なので、メトリクスを可視化しなくても同じ結果には帰着できたかもしれません。
ただし、事前に問題を分析・ゴールを明確にすることで
- 設計者が惑うことなく設計できる。悩んだ際に目的に立ち戻ることができる。
- before/after で効果を計測できるため達成感が得られ、えらい人にも効果を説明しやすい。
- 継続的な改善の礎になる
という効果がありました。絶対やったほうがいいです。ハッピーリファクタリング。
株式会社フォトシンスでは、一緒にプロダクトを成長させる様々なレイヤのエンジニアを募集しています。 photosynth.co.jp
Akerunにご興味のある方はこちらから akerun.com
