Closed49

読書記録「レガシーコード改善ガイド」

kitagrykitagry

レガシーコード改善ガイドを読む

基本的には章ごとに読んだ内容をまとめていく。
この本を読んで知りたい内容は以下の通り

  • ソフトウェアを変更したくなる時とはどんなときか?
  • どのように改善を行うとうまくいくのか?
  • 改善手法について方法をいくつか知りたい

ちょっと雑いけどこんな感じで読んでいきますー

kitagrykitagry

第1部 変更のメカニズム

第1章 ソフトウェア変更の4つの理由

この章ではソフトウェアの変更にどのような種類のものがあるかを記した。
基本的には機能変更・バグ修正・リファクタリング・最適化
これらを見ることで何を変えて何を変えないかを分類する.

例えば、リファクタリングならふるまいは変更しないはず。
これらを理解して自分が今何をしようとしているのかに集中しなければならない。

危険な変更 = どうやって変更出来たとこ(他を変更していないこと)を確認するのかを考える必要がある

kitagrykitagry

第2章 フィードバックを得ながらの作業

交渉ではどのようにレガシーコードを変更するかについて書いている.
コードの変更の仕方は2つ

  • 変更して祈る
  • コードをテストで保護して、変更する

この本ではテストを書いて、満たすべきふるまいを変えないようにすると言っている。
基本的には以下のステップで行う

  1. 変更箇所を見極める
  2. テストを書く場所を見つける
  3. 依存関係を消す
  4. テストを書く
  5. 変更とリファクタリングを行う
kitagrykitagry

第3章 検出と分離

リファクタリング箇所には依存関係を取り除くことが必要である。
これらには偽装オブジェクトやモックオブジェクト等が使える。

この辺は結構知っていたので流し読みした。

kitagrykitagry

第4章 接合モデル

接合モデルとは対象部分を変更しなくてもふるまいを変更できる部分である。

  • プリプロセッサ
  • リンク
  • オブジェクト

の3種類あるけど、上2つは使うことはなさそう。
オブジェクト接合部は要はinterfaceにして、呼び出されるものを変えるっていう感じ。
C, C++とかを書かないならオブジェクト接合部だけで良さそうな雰囲気。

kitagrykitagry

第5章 ツール

この章では使えるツールについて説明している。

自動でリファクタリングするツール。これはlinterとあformatterとかも入るんかな?
これを使うにしろ、テストは書けってなってる

次に、テストのフレームワークの説明。
やっぱりxUnitが出てくるよねえ。この辺はコードとして出すために説明しているって感じだった。

kitagrykitagry

第2部 ソフトウェアの変更

第6章 時間がないのに変更しなければなりません

この章では既存のコードのテストを書かずに新機能を追加したい場面についての手法が書かれていた。基本的には既存コードにはテストは書かないけど、新しいコードにはテストを書くという感じ。手法は以下の通り。

  • スプラウトメソッド(クラス)
  • ラップメソッド(クラス)

スプラウトメソッド = 既存のコードに1行手を加えるけど、それ自体の処理は他に切り出す。他に切り出した処理はTDDで行う
ラップメソッド=既存メソッド(クラス)をラップするものを作成することで新機能を追加する。切り出した処理はTDDで行う

という感じ。追加機能が大きいときはラップメソッドを使う。元のアルゴリズムが理解できるときはスプラウトメソッドを使うという感じらしい。
確かにスプラウトメソッドは元のコードを変更するので、理解できないと実装出来なさそう。

kitagrykitagry

第7章 いつまで経っても変更作業が終わりません

この章ではレガシーコードを変更するためにどのように速度を上げていくべきなのかということが主眼として置かれていた。
レガシーコードはテストが無く依存が絡みあっているので問題箇所を見つけるのが難しく、また見つけたとしても修正に時間がかかる。それに対して、きれいなコードはテストによって依存が解決されているので直すのも勇気をもって修正出来る。

また、テストの依存を消しておくことでコンパイル時間が減るというメリットもある。
DBをinterfaceで抽象化しておけば、DBの変化時にいちいちビルドしなくても良い。
→ RustとかGoみたいな、シングルバイナリだとあんまり関係ないんだろうか?
→ Rustの場合はライブラリごとにビルドしている?感じがするから、この場合は適切にライブラリに分けるみたいな印象だろうか。。?

kitagrykitagry

第8章 どうやって機能を追加すればいいでしょうか?

この本ではずっと述べているが、コードは変更前にテストで保護する必要がある。

TDD

TDDについては他の本で読んだことがあるから省略。
TDDの問題は既存コードにテストが無ければできない?
レガシーコードを変更する場合は

  1. 既存のコードをテストで保護する
  2. 落ちるテストを書く
  3. コンパイルを通す
  4. テストを通過させる(ただし、なるべく既存コードはいじらない)
  5. 重複を取り除く
  6. 繰り返す

という感じらしい

差分プログラミング

これは継承のあるオブジェクト指向ならではの方法っていう感じ。
ある機能を追加したい時に、元のオブジェクトを継承したオブジェクトを作成するというもの。
この時ももちろんTDDで進める。これだと、既存のコードのテストを書かなくてもよさげ?

差分プログラミングの問題点として、構造が複雑化してくるというものがある。
ただ、テストのおかげでリファクタリングはしやすくなるので、容量用法はお守りください。

Liskovの置換原則というものがあって、長方形オブジェクトを正方形オブジェクトが継承すると変なことになるみたいなのがある。
継承する場合は、抽象オブジェクトを使って明白にしろって書いていた。(抽象オブジェクトはあんまり知らないから後で調べよう。)
ただ、僕が最近書いている言語継承が無いからあんまりこの手法使えなさそうだなあと思った。

kitagrykitagry

第9章 このクラスをテストハーネスに入れることが出来ません

基本的にテストハーネスに入れることが出来ないときは以下のような問題が考えられる.

  • クラスのコンストラクタが依存関係がある
  • クラスのコンストラクタの引数のオブジェクトを作成することが難しい
  • クラスのコンストラクタ内でグローバル変数を使っている

基本的にはオブジェクトをインタフェースに抽出して、それをインタフェースを使用して引数に入れることを推奨しているっぽい.
まあ、確かにその方法が一番思いつきやすいかなあと思った.

他にはサブクラスを作成して、実行したくないメソッドをオーバーライドするという方法も取り上げられていた。ただ、Goとかではこれは使えなさそうだなあ。

グローバル変数を使っている場合の方法は良く分からなかった。
多分Singletonのコンストラクタをテストからは使えるようにするみたいな感じだと思う。?

kitagrykitagry

第10章 このメソッドをテストハーネスに入れることが出来ません

全章のクラスをテストハーネスに入れるまでは簡単で、メソッドをテストハーネスに入れる方が難しいらしい。

privateメソッドをテストしたい場合はどうするか?

これは著者の意見としてはprivateテストをテストしたくなった場合は、そのクラスに追わせている責任が重すぎるということらしい。
これはt-wadaさんのブログでも似たようなものを見たことがあるな。
基本的にはリファクタリングを行って別のクラスにするのがいいらしい。

今回の例では時間がない場合はとりあえずprotectedにしろよっていう感じだった。goとかでは使えなさそう。。

インタフェースに切り出せないプリミティブなものを引数にする場合はどうするか?

これはとりあえずプリミティブなオブジェクトをラップするオブジェクトを作成して、interfaceに切り出すのがいいらしい。
確かに、既存のコードがプリミティブなオブジェクトに依存しているということだもんな。

コマンドメソッドの変更が検出できない場合

コマンドメソッドは何かしらの命令を実行するメソッドで、返り値がない。
この時は中の一連のメソッドを別のメソッドに切り出して、サブオブジェクトでオーバーライドするのが良いと書いてあった。
けど、これGoではできんなあ。
Goでメソッドのオーバーライドする方法ないかしら。

kitagrykitagry

第11章 変更する必要がありますが、どのメソッドをテストすればよいでしょうか?

変更したい関数があったときにその変更がどのようなメソッドに伝搬していくかをグラフに書いて、それをテストで守るようにしていた。
今回では引数にListがあった場合、そのメソッドの呼び出し元からそのListを変更出来てしまうという例があった。基本的には呼び出し元を読んでいる関数を順に調べていく感じになりそう。

kitagrykitagry

第12章 1 カ所にたくさんの変更が必要ですが、 関係するすべてのクラスの依存関係を排除すべきでしょうか?

割り込み点と絞り込み点

割り込み点=変更する場所の影響マップのなかですぐ近くのpublicな関数
絞り込み点=複数の変更する箇所の影響マップが収束するpublicな関数

絞り込み点→アプリケーション内の境界線となってその関数のテストを書いておき、中身はリファクタリングしやすくなる
ただし、テスト大きくなりがちなので、十分な単体テストが無くなればテストをなくす。

kitagrykitagry

第13章 移動する必要がありますがどのようなテストが必要でしょうか?

必要なテスト=仕様化テスト=現在のコードのふるまいを記述したもの。

とりあえずテストで呼び出す -> テストが失敗 → 実際に得られた値をテストの予測値にする

これによって変更後も変わらないことが担保される。

注意点

  • 変更するコードの境界値を見つけてテストに加える
  • 変更するコードが実際にテストで使われているかを調べる(coverage)
kitagrykitagry

第14章 ライブラリへの依存で身動きが取れません

この辺はラッパーを作成することを薦めていた。確かにその通りではあるけど、めんどくさくなっちゃうなあ。
後はSingletonのオーバーライドも言っていたけど、Goではできないんじゃ。。

kitagrykitagry

第15章 私のアプリケーションはAPI呼び出しだらけです

最初はAPIを呼び出していると思って設計が微妙なテストの無いコードになるが、時間とともに複雑なレガシーコードになる。
こういう場合のテストの書き方は以下の通り

  • APIをラップする
  • 責務をもとに抽出する

APIをラップは小さいAPIにしか使えない。(作業量多い)
責務をもとに抽出は楽だけど、抽出した部分はテストできない。

多くの場合は両方とも同時に行う

kitagrykitagry

第16章 変更できるほど十分にコードを理解していません

巨大なコードはしばしば理解できないので、理解するための道具が必要

  • 紙にコードの呼び出しなどの図を書きだす
    • 正確さはいらず、その場で理解するために書く
  • コードを印刷して、しるしをつける
    • 色分けすることによって、どれを変更するかのしるしをつける
  • 試行リファクタリング
    • Gitでコミットして置き、自由にリファクタリングしてみる
    • テストは書かなくてもいい
  • 不要なコードを消す
    • いらないコードはバージョン管理で復活出来る
kitagrykitagry

第17章 私のアプリケーションには構造がありません

構造についてどのように把握するかということが書いている

  1. アプリケーションの構造について簡潔に説明する
  2. CRCを使って説明する

アーキテクチャはプログラマ全員が把握する必要がある。そのためプログラマに上の2つの説明をさせると理解が広がるかもしれない。
CRCは白紙の紙をつかってロジックを説明するみたいな感じかな。
言葉よりもものを使って説明した方がやりやすそう。

kitagrykitagry

第18章 自分のテストコードが邪魔になってます

この章ではテストの命名規則とディレクトリ構成について話している。
特に目新しいことはなかった。基本的にはその言語でのスタンダードな方法に従うので良さそう。

kitagrykitagry

第19章 私のプロジェクトはオブジェクト指向ではありませんが、どうすれば安全に変更できるでしょうか?

リンク接合部などで、テストする感じらしい。
基本オブジェクト指向がいいぜーっていう主張みたい。
オブジェクト接合部でテストをしようみたいな感じ。

基本的には同意。で、継承がちょっとなあという感じだった。

kitagrykitagry

第20章 このクラスは大きすぎて、もうこれ以上大きくしたくありません

クラスを分割する方法

  1. クラスの目的を短い文章で書く

クラスの目的にそぐわないものを別のクラスに分けていく

  1. メソッドを近いもので分類する

これによって、大体どんなメソッドがあるかを把握できる
privateメソッドをテストしたいと思った場合は基本的には設計が間違っているので、リファクタリングして別クラスのpublicメソッドにするようにする.

  1. メソッドの影響マップを書く

インスタンスを書きだす->インスタンスを使用しているメソッドを書く
これによって、影響マップが見えてどう分けたらいいかが分かる。

テストで保護できない場合⇒名前をMOVINGとか付けて、分かるように分けていく。
この時にコンパイラ任せにせずに、自分でインスタンスを追って処理を行わないと思わぬ副作用を見逃してしまうので注意が必要。

kitagrykitagry

第21章 同じコードをいたるところで変更しています

まとめた方が良い。
まず小さなステップとしては連なっているメソッド内の処理を合わせる.

a -> a -> b -> a -> b -> b => aa -> b -> a -> bb or a -> ab -> ab -> b
みたいにメソッドを作成する。
そして他のクラスにまたがっている場合は抽象クラスを作成して、統一出来るところを統一する.
Goだったらどうするかなあ。継承ない言語でどうするかはちょっと要調査。

良い設計は拡張には開いていて、修正には閉じている。

kitagrykitagry

第22章 モンスターメソッドを直す必要がありますが、テストを書くことが出来ません

モンスターメソッドとはすごく長いメソッドのこと。
以下の2つの種類がある

  • 箇条書きメソッド: 命令が羅列で並んでいる
  • 錯乱メソッド:ifとかswitchとかの大きいネストがある

錯乱メソッドが結構気合を入れないといけない。

取り組み方

  • 簡単なロジックだけ抽出
    • maxとか
  • 確認用の変数を作成
    • テストを書いて確認用の変数が変わってないか確認
    • テスト後に変数ごと消す
  • 依存の落穂拾い
    • 大事な部分だけテストを書いて、抽出前と後で変わらないかだけ確認する

抽出方法

  • ワイヤーフレーム? 骨組みメソッド
  • 条件ごと処理シーケンスの発見

if (hoge) fuga; というのがあった場合、hogeをもとの状態に残すのがワイヤーフレームでhogeごと他の関数に分けるのが処理シーケンス
ワイヤーフレーム? 骨組みメソッド -> ロジックが元の関数に残るので、一連のロジックを追いやすい
条件ごと抽出 処理シーケンスの発見-> 条件ごと取り出せるので、同じ処理をしているやつに名前を付けれる?

箇条書きメソッド -> 条件ごと抽出 処理シーケンスの発見
錯乱メソッド -> ワイヤーフレーム? 骨組みメソッド

kitagrykitagry

第23章 どうすれば何も壊していないことを確認できるでしょうか?

一つのことをやるようにペアプログラミングをする
シグネチャの変更を行わないようにする
コンパイラに任せる

kitagrykitagry

第25章 依存関係を排除する手法 その1

長いので1個ずつに分ける

1 パラメータの適合

when メソッドの引数が生成が難しいオブジェクト
オブジェクトが小さい -> interfaceの抽出
オブジェクトが大きい -> 使用する関数だけを抽出したinterfaceの作成 & それをimplementしたオブジェクトの作成

これGoだと簡単そう。

kitagrykitagry

2. メソッドオブジェクトの取り出し

when 超巨大メソッドのテストを追加したいが、オブジェクトのインスタンス化が難しいとき
超巨大メソッドのみを持つオブジェクトを作成する.
オブジェクトの作成方法は以下の通り

  1. メソッドを実行するためのクラスを作成する.
  2. constractorはメソッドの引数 & 元のオブジェクトのポインタにする
  3. コンパイラに任せながら、必要なクラスメソッドなどはpublicにして、元のオブジェクトのポインタから呼び出すようにする
  4. 元のオブジェクトで必要なものだけをinterfaceに抽出する
  5. 元のオブジェクトでの呼び出しを新しく作成したオブジェクトの呼び出しに置き換える。

これでメソッドオブジェクトのconstructorは簡単になるはずなのでテストがしやすくなる!
これはなるほどだわ。ただし、これは一時的なものでこのメソッドオブジェクトをテスト & リファクタした後は、元のメソッドに戻すなり設計をみなおさないといけない。

kitagrykitagry

3. 定義の補完

C,C++のような定義ファイルと実装ファイルが分かれている場合に使える。
テスト用に実装ファイルを書くという方法。
デバッグの際に混乱を招きやすいので、あまりお勧めはできない。

kitagrykitagry

4. グローバル参照のカプセル化

グローバルな関数はテストがしづらくなるという欠点がある.なので、出来るだけ依存をするなくしたい.
手法としては、まとめられそうなグローバル関数をまとめてオブジェクト化する.
それによって落ちるテストの部分を全部変更する.
後は、コンストラクタやパラメータにそのオブジェクトを含める方法を使うことでテスト容易性を上げていく

グローバルメソッドの場合は抽象クラスを作って、本番コードではグローバル関数を使う.テストコードではテスト用のコードを使うという風にする.
これはGoとかでは無理そう.

kitagrykitagry

5. 静的メソッドの公開

インスタンス化しづらいクラスをテストに加えるときに、もし可能ならstatic関数にしてしまうというもの。
これはあんまり起こらなそう。

kitagrykitagry

6. 呼び出しの抽象とオーバーライド

テストを行えないのが局所的なコードが原因であれば、その部分をメソッドに抽出して、テスト時にはオーバーライドして使うというもの。
というかgoでこういうのどうすればいいんだろう。buildタグ使うとか?

kitagrykitagry

7. ファクトリーメソッドの抽出とオーバーライド

コンストラクタに複雑な処理を書くと、インスタンス生成が難しくなる。
→ 別メソッドに書きだす
→ テスト時にはそのメソッドをオーバライド

kitagrykitagry

8. getメソッドの抽出とオーバライド

C++みたいなコンストラクタ内で別メソッドを呼び出せないもの用の処理
Getメソッドを使って、遅延でコンストラクタ内の処理を実行する.
呼び出し元は全部Getメソッド内で書き換える
なのでコンストラクタではただnullが呼び出されるようになる

kitagrykitagry

9. 実装の抽出

オブジェクトをインターフェースにする方法。

  1. オブジェクトの名前を変更する.
  2. 定義ファイルのメソッドをインターフェース用のメソッドにする
  3. 元のオブジェクトでインターフェースを継承する

こんな感じ。やっぱJavaとはC++とかはinterfaceを明示的に継承しないといけないからな。
Goはこの辺何もしなくていいのは最高。
Rustはやらないといけないな。

kitagrykitagry

10. インターフェースの抽出

オブジェクトをインターフェースに直してテストできるようにする。
これはまあ、簡単だから特にいうことはないかな。
ちょっと意外だったのはテストするために必要な分だけinterfaceに抽出すれば良くて、全メソッドを抽出する必要はないということ。これはまあ、そうか。

kitagrykitagry

11. インスタンス委譲の導入

Singleton的なstatic関数があった場合
→ これのせいでテスト出来なくなるケースがある

クラス関数にして、static関数に委譲する

class A {
  static void get() {
  }

  void get() {
    return A.get()
  }
}

これを呼び出し元で呼ぶように以上すると、これを回避するようにサブクラスでオーバライドしてテストが書ける

kitagrykitagry

12. 静的setメソッドの導入

Singletonのせいでテストが出来ない -> コンストラクタの制限レベルを下げてテストを書く
setメソッドを追加して、テストでSingletonのインスタンスを変更する
これは賛否両論
最初は静的setメソッドを使う-> クラスとして引数で渡せるようにする
多分Singleton嫌いなんだろうな

kitagrykitagry

13. リンクによる置き換え

メソッドのテストをするときにリンカで対象を変える
これは使わない。

kitagrykitagry

14. コンストラクタのパラメータ化

コンストラクタ内でクラスのインスタンスを作成しているとテストできない
-> そのクラスを引数とした新しいコンストラクタを用いる(パラメータ化)
そのほかにもデフォルト引数でも出来る。(pythonとか便利)

kitagrykitagry

15. メソッドのパラメータ化

メソッド内でインスタンスを作成されるとテストできない
-> そのクラスを引数としたメソッドを用意する
C++では同じ名前のメソッドを作れるが、名前は変えた方がいい run -> runWith〇〇
これではシグネチャが変わってしまうので、それが気になる場合はFactoryメソッドの抽出とオーバライドを行う

kitagrykitagry

16. パラメータのプリミティブ化

インスタンスを作成するのが壊滅的に無理な場合の応急処置
必要な処理を切り出す。 → テストが書ける
その処理に必要なパラメータを作成するためのメソッドを追加する。 → テストが書けない

これはリファクタリングの前段階にやる。
あまり使いたくない手法。

kitagrykitagry

メソッドと変数の引き上げ

メソッドをテストしたいが、他のメソッドのせいでテストするのが困難な場合。
テストしたいメソッドや変数をスーパークラスのものにして、既存のものはそのスーパークラスを継承したものにする。
メソッドオブジェクトよりも軽いメソッドに適用できる。

ただし、設計の点からはあまりよくはなさそう

kitagrykitagry

18. 依存関係の押しだし

あるメソッドが依存する関数がテストが不可能な時。
例えば、UIに依存するとか

この場合、元のクラスを抽象化する→具象クラスを作成→具象クラスに依存している関数を移す
テスト用にサブクラスを作成する

これも、リファクタリングするための一歩目と考えられる

kitagrykitagry

19. 関数ポインタによる関数の書き換え

Cとかで使える関数を置き換える手法らしい。
とりあえず飛ばす

kitagrykitagry

20. getメソッドによるglobal参照の置き換え

Singletonメソッドを使う場合にそれを呼び出す部分を全てgetに置き換える
→ getメソッドをオーバーライドしてテスト用に変更
→ テストが書ける

kitagrykitagry

21. サブクラス化とメソッドのオーバーライド

テストが難しいメソッドがあるときに、サブクラスを作成してオーバーライドでテストできるようにする。
確かにこれを見るとオブジェクト指向はテスト書きやすそうだなあと思う。
他の手法もこの方法の変形だとみなせる

kitagrykitagry

22. インスタンス変数の入れ替え

これはC++特有の問題みたいなので一旦飛ばした。
問題としては、テスト可能にするときはオーバーライドする
しかし、C++ではコンストラクタ内ではオーバーライドできない。

kitagrykitagry

23. テンプレートによる再定義

テンプレートを使って依存を解決できるらしい。
あんまり使うことはなさそう

kitagrykitagry

24. テキストの再定義

インタプリタ系の言語では、簡単に継承が出来る場合があるみたい。
本番コードを変更することなく、テスト内で継承できる。

kitagrykitagry

1か月くらいかかったけど、やっと読了出来た。
一旦最初に考えていた何を知りたいかをまとめる。

ソフトウェアを変更したくなる時とはどんなときか?

機能変更・バグ修正・リファクタリング・最適化の4つに分けられる。
これらはふるまいを変えるのかどうかが結構肝になる。
振る舞いを変える変更なのかどうかを考えながら、コードに変更を加えると良い

どのように改善を行うとうまくいくのか?

既存の変更方法は、変更して祈るというものだった。
しかし、プロとしてこれはいけないよね。。

この本で紹介されていたのは、まずテストで保護して確認しながら変更するというもの。
まずはテストで保護することを考えないといけない。
この本では基本的にどのようにテストで保護するかが中心になっていた。

改善手法について方法をいくつか知りたい

まずはテストを可能にするというのが改善のための第一歩だった。
この本では基本的には以下の2つの方法があった。

  • interfaceにする
  • サブクラスを作成して、オーバーライドする

基本的にはinterfaceにするというのが良さそう。
サブクラスを作成するのはGoとかだと厳しそうだった。ので、interfaceにどうやって抽出するかがカギになってきそうだなあと思った。

このスクラップは2021/02/07にクローズされました