(WIP)いつリファクタリングする？臭ったら替えようねって話
updated: 2021-08-18
リファクタリング第２版を読んでのログや感想を書いていく。
いつリファクタリングをすればいいか？
その「いつ」には明確な答えはない
ただ不吉な兆候はあるし、その「不吉な臭い」を嗅ぎ分ける能力を養っていく必要がある。
普段コードを読んでいて、「ここの処理やばそうだなぁ。。」って感じるコードががあれば、それらの一つに当たるのかもしれない。
いっぱいあるけど、「不吉な臭い」を嗅ぎ分け、リファクタリングするか迷ったときのヒントしていくのが良さそう。
各対処法に関しては、リファクタリング第２版を読んでみると良さそうです。
不可思議な名前
- 明快なコードにするために最も重要なのは適切な名前付け
- 名前付けはプログラミング言語で最も難しいこと２つのうちの１つ
- 適切に名前付けできていれば、将来思い悩まずに済む
- いい名前が思いつかない -> 設計がまだ固まっていない兆候なのでは？
不可思議な名前の対処法
- 「関数宣言の変更」(名前の変更のための)
- 「変数名の変更」(名前の変更のための)
- 「フィールド名の変更」(名前の変更のための)
重複したコード
- 同じ構造のコードが２箇所以上ある場合１箇所にまとめることができると良いプログラムになる。
- なぜ？
- コードに重複があると、似ているけど違う部分がないか深く読む必要が出てくるから
- 重複したコード修正するとき、重複箇所を見つけ全てに同じ修正をする必要が出てくる
重複したコードの対処法
- 同一クラス内の複数メソッドに同じ式がある場合（一番単純パターン）
- 「関数の抽出」
- 抽出したメソッドをもとのメソッドの本体から呼ぶようにする
- 「関数の抽出」
- 似ていいるけど完全には似ていない場合
- 「ステートメントのスライド」
- でコードを整え、似た箇所を寄せておくと抽出しやすくなる
- 「ステートメントのスライド」
- 共通のベースクラス配下のサブクラス間で重複したコードががある場合
- 「メソッドの引き上げ」
- 重複したメソッドの呼び出しを避けることができる
- 「メソッドの引き上げ」
長い関数
-
関数がながければ長くなるほど理解しにくくなる
- とはいえ、細かくすると、コードを読む人にとって次から次へと追っていかないといけなくなる
- この場合へのおすすすめは、適切な関数名をつけてあげること
- 適切だと内部の実装見なくても先に読み進められるから
- 関数の分割に積極的になれる
-
命名を適切にすると
- 関数の分割に積極的になれる
- コメントが必要だと感じたときには、代わりに わかりやすい名前をつけた関数に分割する
- 内部のように処理をしているではなく、そのコードが何をするのかという「意図」を示した名前をつけるようにする
-
置き換えによって、以前よりもコードが長くなったとしても、意図が明確になればヨシ！
- 長さを短くすることが目的ではなく、名前と実装の距離を埋めることが重要。
長い関数の対処法
- 「関数の抽出」
- 関数を短くするために行う作業の99%はこれ
- 関数内で一つにまとめられそうな箇所があた迷わず抽出
- パラメータや一時変数が多すぎる関数は抽出が難しくなるので下記で対処
- 「問い合わせによる一時変数の置き換え」
- 一時変数を減らす
- 長いパラメータへの対処で、スリム化する
- 「パラメータオブジェクトの導入」
- 「オブジェクトそのものの受け渡し」
- 「問い合わせによる一時変数の置き換え」
- それでも一時変数やパラメータが残ってしまう場合
- 「コマンドによる関数の置き換え」を試す
抽出部分を見つけ出すいい方法
コメントを探す。
コメントは、コードの意図が伝わりにくいところを補っている書いていることが多い
分割後がたとえ１行になったとしても、説明のために分割する価値はある。
条件分岐やループ
- 「条件記述の分解」
- 巨大なSwitchがある場合
- 「関数抽出」
- 同じ条件で分岐しているSwitchが複数ある場合
- 「ポリモーフィズムによる条件記述の置き換え」
- ループ
- ループ部分とループ内部のコードを抽出して独立した関数にできる
- 抽出したループの名付けが難しいときは、ループだけでない別のことをしているかもしれない
- そのときは「ループの分離」で独立したタスクに分離する
長いパラメータリスト
- 長いパラメータリストもよく問題になる
- 「問い合わせによるパラメータの置き換え」
- パラメータに渡されるオブジェクトに問い合わせることで、パラメータリスト中の他のデータを取得できる場合、減らした分パラメータを減らすことができる
- 「オブジェクトそのものの受け渡し」
- 既存のデータ構造から多くのデータを取り出す代わりに、元々のデータ構造を渡してしまう方法
- 「フラグパラメータの削除」
- パラメータが振る舞いを変えるためのフラグとして使われている場合、こちらを適用する
- 「関数群のクラスへの集約」
- クラスはパラメータを減らすための優れた手法
- 複数の関数群がパラメータで渡される値を共有しているとき、こちらを使って共通の値をフィールドとして定義する
グローバルなデータ
バグにつながるのと、コードのどこが悪さをしれいるのか特定が難しい。
例でわかりやすいのだとグローバル変数。
- あらゆる箇所からの汚染にさらされたデータを見つけたときの対処
- 「変数のカプセル化」
- 少なくとも関数経由でのアクセスにすることで、いつ値を変更しているのか突き止めやすくなり、制御できるようになる
- その後にクラスやモジュール内部に移動させることで、直接参照できる箇所を限定し、スコープをできるだけ狭めてあげるのが良い
- 「変数のカプセル化」
- 変更可能なグローバルなデータは特に有害
- グローバルなデータって、少量なら良いけど量が増えていくと有害になっていく。