🏛️

レガシーコードの塊に立ち向かうリファクタリング

に公開

クロスカウンターよりボティブロー

よくある話

既存コードの品質がゴミ!どうにかしなきゃ!!君は怒りと焦燥感を持ってリファクタリングを始める。コードを眺める。よくわからない。デグレードの恐怖におびえながらプログラムを少し変更しテストする(もしかしたらテストはしないかも)。そのうち、君は道の果てしなさとデグレードの恐怖で疲れ果てる。そして、デグレードは実際に起こり、プログラムは動かなくなる。原因はわからない。
君は考える、「こんなんじゃ埒が空かない、まるごと作り直しだ!」。まずは外仕様の調査を始める。しかし、異常系のパターンとあるべき振る舞いがわからない。仕様書は存在しないか不完全だし、仮に完全だとしても(完全であればあるほど)人間には理解できない。そのうち君は無気力を学習した犬になる。助かる道はないと知り、ゴミコードを元にゴミコードを再生産し続けるオートマタになる。

じゃあどうするか?

本などで言われる定石は、まず回帰テストを作成して患部を保護して安全に好きなだけ変更する、だ。しかし現実にはコードの品質が悪ければ悪いほどテストをつく来るのは難しく、またリファクタリングしたいコードは品質の悪いコードである。非常に辛い。
そこで私としては、ボトムアップで小さくて安全なカイゼンを繰り返して、ブレイクスルーを狙う、をおすすめする。ものすごく小さいがリスクも極小なリファクタリングを行い続け、徐々にスケールアップするのである。
この方法の利点としては、

  1. 安全性が高い
  2. やる分だけ品質が向上する
    • 小さな時間単位でも実行できる
    • どこで中断しても成果が得られる
  3. 全体を理解する必要がない
    • 極部分的な理解度でも始められる
    • 過程で理解度を上げていける

小さく始めて大きく育てる!はここでも適応できる原則なのだ

よくある質問と回答

それでは抜本的なリファクタリングにならないのではないか?

小さいリファクタリングを地道に続けていくと、急にシステム全体の見通しがよくなる時がくる。そうなってから、アーキテクチャレベルの変更を検討すればいい。
人間の理解能力は線形ではない。学習曲線をイメージするといい。地道な停滞期と急激なブレイクスルーの繰り返しだ。
経験が無ければ、実感しづらいので疑わしく感じる人もいるだろうが、信じて欲しい。黙ってやれ。

総工数の見通しが立たないのではないか?

そもそもリファクタリングについて総工数を出すのは現実的ではない。リファクタリングに終わりは無いし、終わらせる基準も設定しづらいからだ。投入工数をあらかじめ決めて(例えば4H/Wや1H/D)、その中で定期的に実行するのをおすすめする。
また、仮に”まるごと作り直し型”でも、総工数を出すのは現実敵ではない。工数を出すためには作業内容に明確な見通しがついている必要がある。リファクタリングの特性を考えると、それほどの見通しがついている時点で、総工数の7割は消化済みになっているはずだ。その時点からの残工数は元々知りたかった総工数は別のものだろう。

具体的な技法

比較的安全(バグを追加で埋め込むリスクが低い)に出来て、見通しがよくなるリファクタたち。巨大な神クラスの分割とかはまた次の段階かなー。
これらのパターンは脳死しながらでもできるデザインパターンなので超おすすめ!

読みやすさの向上編

  1. クラス、関数名を明示的にする
  2. 使われていない関数、フィールドの削除
    • いつか使うのいつかは2度とこない。部屋の掃除と一緒。使うときにまた作れ。
    • 今どきどのIDE・エディタでも無参照な定義を注意してくれるから素直に従え。自分が機械より賢いと思うな
  3. ガード節の使用
    • 参考
    • ネストの深さは闇の深さ
    • ネストが深いと脳のメモリを爆食いする
    • ガード節はお手軽にネストを減らせるのでおすすめ!
  4. 可能な限りイミュータブル(不変)にする
    • この変数・このインスタンスは値が変わりませんよ、を明示されると脳のメモリに隙間ができる
    • 逆にコロコロ値が変わる変数はバグの苗床
    • C# なら readonlyrecordImmutableList、 python なら data class、TypeScript なら constreadonly、Kotlin なら valdata class etc... 今どきはいろんな言語でイミュータブルな書き方のための機能が存在する。使う言語の機能を調べて活用しよう

デザインパターン導入編

  1. ValueObject の採用
    • 参考
    • Dry な書き方を促進してくれる
    • 完全コンストラクタと合わせて活用すると異常系対策に心強すぎる
    • ValueObject への処理を ValueObject 自身に取り込んで行くと勝手にオブジェクト指向になっていくぞ!
  2. 完全コンストラクタ
    • 参考
    • インスタンスのイミュータブル化と、コンストラクタ引数チェックによる、フィールド安全性の確保
    • 異常系のパターンを超絞れるので安心感に直撃する
  3. リストの操作に集約ルートの考え(ファーストクラスコレクション)を導入
    • 参考
    • オブジェクトの所持するオブジェクトを直接操作出来ないようにしよう
    • カプセル化の第一歩
    • 経験上、品質の低いソフトのソースはリストに対する操作の分散がひどく、暗黙の前提がソース中にバラけている&重複して存在している。それに対応できれば大分わかりやすくなる

その他

リファクタリングした箇所(特に ValueObject とファーストクラスコレクション)は、基本的にユニットテストを作って保護しましょう。安心できる領域が広がっていって気持ちいいですよ。

まとめ

  • やばいソフトに対して抜根的な対策じゃないと行けないと思い込んでるよく見るけど、そうとは限らないよ
  • そびえたつゴミに直面しても小さく確実に地道にやっていこう
  • 小さく安全に前に進む方法はいくつかある、知識は力
  • 『そうだな…。わたしは「結果」だけを求めてはいない。「結果」だけを求めていると人は近道をしたがるものだ……….。近道した時、真実を見失うかもしれない。やる気もしだいに失せていく。大切なのは「真実に向かおうとする意思」だと思っている。向かおうとする意思さえあれば。たとえ今回は犯人が逃げたとしても。いつかはたどり着くだろう?。向かっているわけだからな…………違うかい?』

その他参考

  1. 良いコードの書き方
    いいこと書いてあるけど長い。
  2. きれいなコードを書くコツ
    上のやつを読むのがいやなら。
  3. 新人プログラマに知ってもらいたいメソッドを読みやすく維持するいくつかの原則
    より実践的な解説かな?
GitHubで編集を提案

Discussion