いつリファクタリングする？臭ったら替えようねって話

2021/08/04(水)
リファクタリング第２版を読んでのログや感想を書いていく。
いつリファクタリングをすればいいか？

その「いつ」には明確な答えはない

ただ不吉な兆候はあるし、その「不吉な臭い」を嗅ぎ分ける能力を養っていく必要がある。

普段コードを読んでいて、「ここの処理やばそうだなぁ。。」って感じるコードがあれば、それらの一つに当たるのかもしれない。
いっぱいあるけど、「不吉な臭い」を嗅ぎ分け、リファクタリングするか迷ったときのヒントにしていくのが良さそう。

各対処法に関しては、リファクタリング第２版を読んでみると良さそうです。
 不可思議な名前明快なコードにするために最も重要なのは適切な名前付け
名前付けはプログラミングで最も難しいことの 2 つのうちの 1 つ
適切に名前付けできていれば、将来思い悩まずに済む
いい名前が思いつかない -> 設計がまだ固まっていない兆候なのでは？
 不可思議な名前の対処法「関数宣言の変更」 (名前の変更のための)
「変数名の変更」 (名前の変更のための)
「フィールド名の変更」 (名前の変更のための)
 重複したコード同じ構造のコードが 2 箇所以上ある場合、1 箇所にまとめることができると良いプログラムになる
なぜ？
コードに重複があると、似ているけど違う部分がないか深く読む必要が出てくるから
重複したコードを修正するとき、重複箇所を見つけて全てに同じ修正をする必要が出てくる
 重複したコードの対処法同一クラス内の複数メソッドに同じ式がある場合（最も単純なパターン）
「関数の抽出」
抽出したメソッドを元のメソッドの本体から呼ぶようにする

似ているけど完全には同じでない場合
「ステートメントのスライド」
コードの順序を調整し、似た箇所を近づけて抽出しやすくする

共通のベースクラス配下のサブクラス間で重複したコードがある場合
「メソッドの引き上げ」
重複したメソッドを親クラスに移動させて、二重定義を避ける

 長い関数関数が長ければ長くなるほど理解しにくくなる
とはいえ、細かく分けすぎると、コードを読む人は次から次へと抽出先を追わなければいけなくなる
こうした場合は、適切な関数名をつけてあげることがおすすめ
適切な名前であれば内部の実装を見なくても先に読み進められるから
そうすると関数の分割にも積極的になれる

命名を適切にすると
関数の分割に積極的になれる
コメントが必要だと感じたときには、代わりに わかりやすい名前をつけた関数に分割する
内部でどんな処理をしているかではなく、そのコードが何をするのかという「意図」を示した名前をつけるようにする
置き換えによって、以前よりもコードが長くなったとしても、意図が明確になればヨシ！
長さを短くすることが目的ではなく、名前と実装の距離を埋めることが重要
 長い関数の対処法「関数の抽出」
関数を短くするために行う作業の 99%はこれ
関数内で一つにまとめられそうな箇所があったら迷わず抽出
パラメータや一時変数が多すぎる関数は抽出が難しくなるので以下で対処
「問い合わせによる一時変数の置き換え」
一時変数を減らす

長いパラメータへの対処でスリム化する
「パラメータオブジェクトの導入」
「オブジェクトそのものの受け渡し」

それでも一時変数やパラメータが残ってしまう場合
「コマンドによる関数の置き換え」 を試す
 抽出部分を見つけ出すいい方法コメントを探す。

コメントは、コードの意図が伝わりにくいところを補って書かれていることが多い。

分割後がたとえ 1 行になったとしても、説明のために分割する価値はある。
 条件分岐やループ「条件記述の分解」
巨大な switch がある場合
「関数の抽出」
同じ条件で分岐している switch が複数ある場合
「ポリモーフィズムによる条件記述の置き換え」
ループ
ループ部分とループ内部のコードを抽出して独立した関数にできる
抽出したループに適切な名前を付けるのが難しいときは、そのループが複数のことをしているのかもしれない
そのときは「ループの分離」で独立したタスクに分離する

 長いパラメータリスト長いパラメータリストもよく問題になる
「問い合わせによるパラメータの置き換え」
パラメータに渡されるオブジェクトに問い合わせてパラメータリスト中の他のデータを取得できる場合、その分パラメータを減らすとができる
「オブジェクトそのものの受け渡し」
既存のデータ構造から多くのデータを取り出す代わりに、元々のデータ構造そのものを渡してしまう方法
「フラグパラメータの削除」
パラメータが振る舞いを変えるためのフラグとして使われている場合に適用する
「関数群のクラスへの集約」
クラスはパラメータを減らすための優れた手法
複数の関数群がパラメータとして同じ値を共有しているとき、その共通の値をフィールドとして持つクラスにまとめる
 グローバルなデータバグにつながるのと、コードのどこが悪さをしているのか特定が難しい。

例でわかりやすいのはグローバル変数。
あらゆる箇所からの汚染にさらされているデータを見つけたときの対処
「変数のカプセル化」
少なくとも関数経由でアクセスすることで、いつ値を変更しているのか突き止めやすくなり、制御できるようになる
さらにクラスやモジュール内部に移動させ、直接参照できる箇所を限定してスコープを狭めるのが良い

変更可能なグローバルなデータは特に有害
グローバルなデータは少量なら良いけど、量が増えるとどんどん危険になる
 変更可能なデータあちこちで値が書き換えられるデータは、プログラムの振る舞いを予測しにくくし、バグの原因になりやすい
特に共有されるデータが頻繁に変更されると、どのタイミングで何が起こるか追いづらくなる
データの変更自体を減らす工夫をする
「変数のカプセル化」
値の読み書きを関数（アクセサ）経由に限定し、いつどこで変更されるかを把握しやすくする

「セッターの削除」
フィールドを書き換えるメソッド（セッター）をなくし、オブジェクトの状態をイミュータブル（不変）にして予期せぬ変更を防

 変更の偏り1 つのクラスが複数の独立した理由で頻繁に変更されるなら、そのクラスに責務が集中しすぎているサイン
例えば、新機能の追加でもバグ修正でも、何かにつけ同じクラスばかりを修正しているような場合
クラスを分割し、それぞれの変更理由に対応する
「クラスの抽出」
1 つのクラスが担っていた異なる役割を、新しいクラスに切り出して責務を分散させる

「関数の移動」や「フィールドの移動」
関連する処理やデータを適切なクラスに移し、1 つのクラスだけが変更の影響を受けない設計にする

 変更の分散1 つの変更要求のために、コードのあちこちに手を入れなければならない状況
修正のたびに複数のクラスが少しずつ変更されるのは、責務が分散しすぎている兆候
関連する変更はできるだけ単一のモジュールに集約できるよう再構成する
「関数の移動」や「フィールドの移動」
分散していた処理やデータを適切なクラスに集め、変更点を局所化する

「クラスのインライン化」
役割が重複しているクラスを統合し、複数箇所への修正を減らす

 特性の横恋慕メソッドが自分のクラスよりも他のクラスのデータや機能に関心を寄せている場合、そのメソッドは「隣の芝生が青く見えている」態
自クラスのフィールドより他クラスのフィールドを参照し、その値を使った計算を行っているようなら、メソッドの所属が間違ってる可能性が高い
関心の対象となっているクラス側に処理を移す
「関数の移動」
他クラスのデータを主要に利用しているメソッドは、そのデータを持つクラスへ移動するのが有効
「関数の抽出」
メソッド内の一部で他クラスのデータを使っている場合、その部分を切り出してから適切なクラスに移す
 データの群れいくつかのデータが常にセットで登場するようなら、それらは「データの群れ」を形成している
メソッドのパラメータリストやクラスのフィールドとして、同じ組み合わせのデータが何度も出てくる場合も要注意
群れているデータは 1 つのオブジェクトにまとめる
「クラスの抽出」
そのデータ群に対応する新しいクラス（ドメインオブジェクト）を作り、関連する値を一箇所で管理する

「パラメータオブジェクトの導入」
常に一緒に渡される複数の引数を 1 つのオブジェクトにまとめ、パラメータリストを簡潔にする

「オブジェクトそのものの受け渡し」
元々ひとまとまりの構造から取っていた複数データは、構造自体を引数として渡して重複を避ける

 基本データ型への執着本来ひとつの概念である値の集合を、言語の基本データ型（プリミティブ）で表現して済ませてしまっている状態
例えば、住所情報を県・市・町・番地の 4 つの文字列フィールドで持っていたり、通貨や単位付きの数値をただの number で扱ったりるケース
基本型のままではそれらに関するロジック（例: 書式や検証）が各所に散らばり、クラスが非常に扱いづらくなってしまう
意味を表現する小さなクラスを導入し、プリミティブな値に役割を与える
「データ値をオブジェクトに置き換える」
単なる数値や文字列を専用のクラスに変え、その中に関連する処理（検証・計算・フォーマットなど）をまとめることで、コード意図と一貫性を高める
 重複したスイッチ文似たような switch 文（または if-else の条件分岐）がコード中に複数存在するのは危険な臭い
新しい条件を追加するとき、散在するすべての分岐文に手を入れねばならず、修正漏れによるバグを生みやすい
分岐処理はポリモーフィズムを用いて表現し、重複する条件記述を無くす
「ポリモーフィズムによる条件記述の置き換え」
条件ごとの振る舞いをサブクラスや Strategy パターンで実装し、コード上の分岐の乱立を避ける

 ループコレクション操作などで手続き的なループを多用していると、コードの意図が見えにくくなることがある
特に長いループやネストしたループは、何を実現したい処理なのか把握しづらい
ループ処理は分割や高レベルの操作に置き換えて、意図を明確に示す
「ループの分離」
1 つのループで複数の目的の処理をしている場合、ループをタスクごとに分割してそれぞれ独立させる

「パイプラインによるループの置き換え」
map や filter、reduce といったコレクションのパイプライン処理で表現し、処理内容を宣言的に記述する

 怠け者の要素ほとんど仕事をしていないクラスやメソッドは、保守のコストに見合わない「怠け者」
作ってみたものの内容がスカスカなクラスや、リファクタリングの結果存在意義が薄くなってしまった要素などが該当する
十分な役割を果たしていない要素は整理して削除する
「クラスのインライン化」
機能がほとんどないクラスは別のクラスに統合し、クラスの数を減らす

「階層の折りたたみ」
大差ない親子クラスがある場合は継承関係を廃止して 1 つにまとめる

「関数のインライン化」
処理がほとんどないメソッドは呼び出し元に処理を移して削除する

 疑わしき一般化「いつか必要になるかも」と入れられた抽象的な仕掛けや未使用のパラメータは、往々にしてコードを複雑にするだけの存在
使われていない抽象クラスやインタフェース、必要以上に柔軟性を持たせようとして残された設定項目などがこれに当たる
実際に利用されていない汎用的な部分は削ぎ落としてしまう
「デッドコードの削除」
将来のために残されているコードでも、不要であれば思い切って削除してコードをシンプルにする

「階層の折りたたみ」
実際には機能していない抽象クラスや継承階層は、継承関係を解消して単純化する

「パラメータの削除」
呼び出されていない引数や常に同じ値しか取らないフラグ引数は取り除いて関数をすっきりさせる

 一時的属性クラスのフィールドが特定の状況でしか値を持たない（大半の時間は未設定）という場合、そのクラスの状態を理解しづらくなる
大部分の場面で使われない変数があると、「いつ値が設定されるのか」「何のために存在するのか」を追うのが大変
そのフィールドが必要になる場面を独立させる
「クラスの抽出」
その一時的なデータと関連する処理を新しいクラスに切り出し、必要な場合にだけそのクラスを使うようにする

「Null オブジェクトの導入」
フィールドが未設定であるケースを表現する専用のクラス（またはサブクラス）を用意し、条件分岐をわかりやすくする

 メッセージの連鎖objectA.getB().getC().getD().doSomething() のように、何段にもわたってオブジェクトのメソッドをたらい回しに呼んいく構造
このような「連鎖」があると、オブジェクト間の関係構造が呼び出し側に露呈しており、内部構造の変更に弱いコードになってしま
オブジェクトの連鎖を隠蔽し、直接必要な処理やデータにアクセスできるようにする
「委譲の隠蔽」
中継役のオブジェクトにメソッドを追加して、呼び出し元が直接目的のメソッドを呼べるようにする（途中経路を意識させない）
「関数の移動」
メッセージ連鎖の末端で行っている処理を手前のオブジェクトに移し、長いメソッドチェーン自体をなくす
 仲介人クラスの主な仕事が他のクラスへのメソッド呼び出しを仲介するだけになっている場合、そのクラス自体が「仲介人」になっている
カプセル化しすぎて何でも中継するクラスがあると、かえってコードが冗長になり、追跡もしにくくなる
過剰な仲介は取り除き、呼び出し側が直接本来のオブジェクトを扱えるようにする
「仲介人の削除」
委譲先のオブジェクトをクライアントから直接参照できるようにし、中継メソッドを撤去する

「関数のインライン化」
何もしないで委譲するだけのメソッドは省略し、直接呼ぶようにしてしまう

 インサイダー取引クラス同士が必要以上に仲良くなりすぎて、お互いの内部に深く干渉し合っている状態（「不適切な親密さ」）
例えば、親クラスとサブクラスが互いの実装に強く依存していたり、2 つのクラスが相互に private データまで覗き見しているよう関係
クラス間の結びつきを緩め、適切な境界を設ける
「関数の移動」や「フィールドの移動」
他クラスの内部情報を頻繁に使っているメソッドやデータは、その情報を持つクラス側へ移して、依存を一方向にする
「継承から委譲への置き換え」
継承関係が親子クラスを過度に密接にしている場合、継承をやめて委譲（コンポジション）で機能を共有し、結合度を下げる
双方向の関連を片方向にする
2 つのクラスが互いに参照し合っているなら、一方から他方への参照だけに留め、依存関係をシンプルにする
 巨大なクラスフィールドやメソッドが多すぎる巨大なクラスは、1 つのクラスが過剰な責任を負っている兆候
インスタンス変数がやたらと多かったり、クラスの行数が非常に長い場合、そのクラスは機能を詰め込みすぎているかもしれない
クラスを分割し、責務ごとに小さくまとめる
「クラスの抽出」
大きすぎるクラスから関連するフィールドとメソッドのグループを新しいクラスとして切り出し、役割を分ける

「サブクラスの作成」
クラスの中の機能の一部を派生クラスに移し、必要な場面でそのサブクラスを使うことで本体を軽量化する

「インタフェースの抽出」
1 つのクラスが複数の異なる機能を提供している場合、機能単位でインタフェースを用意し、利用側が必要な機能だけを扱えるよにする

 クラスのインタフェース不一致似たような機能を提供しているのに、メソッド名やパラメータの違いで相互に再利用できないクラスが存在する状態
例えば、どちらも同じ情報を扱うのに、一方は getName()メソッド、もう一方は name()メソッドで名前を取得するようなケース
クラスのインタフェースを揃えて、重複する機能を統合・再利用しやすくする
「関数宣言の変更」
メソッド名やシグニチャを変更し、同じ役割のメソッドは同じ名前・引数で利用できるようにする
「インタフェースの抽出」
複数のクラスが実質同じ責務を持っているなら、共通のインタフェースを定義してクライアント側がそれらを同一視できるようにる
「Adapter パターンの適用」
内部的にインタフェースを統一できない場合は、一方のインタフェースに他方を合わせる Adapter クラスを用意して差異を吸収す
 データクラスフィールドとそれに対する getter/setter 以外に機能を持たないクラス（ただのデータホルダー）は「データクラス」と呼ばれる
こうしたクラスは自らは何もせずデータを保持するだけなので、他のクラスから過剰にアクセスされがちで、アンチパターンになりすい
データクラスのデータを安易に他から弄られないようにし、必要な振る舞いはクラス内に持たせる
「関数の移動」
そのデータクラスが持つデータを使って他のクラスで行っている処理があれば、その処理をデータクラス側に移してカプセル化す
「変数（フィールド）のカプセル化」
データクラスのフィールドが public で公開されているなら、アクセスメソッドを用意して直接アクセスできないようにする（外からの不用意な変更を防ぐ）
 相続拒否サブクラスが親クラスから継承したメソッドやフィールドの多くを使わず、ほんの一部しか利用していない場合、「親からの遺産を棄している」状態
継承関係なのに子クラスが親クラスの契約を十分に履行していなかったり、親の機能の大半を無視しているようであれば要検討
継承関係を見直し、不要な継承は解消する
「継承から委譲への置き換え」
サブクラスが親クラスのごく一部の機能しか必要としていないなら、継承をやめて必要な機能だけを持つクラスにする（必要ならの親クラスを内部で利用する）
「クラスの抽出」や「インタフェースの抽出」
親クラスの機能の一部分のみ各サブクラスが使うのであれば、その機能部分だけを別の親クラスやインタフェースとして切り出、無駄な継承を避ける
 コメントコメントを書くこと自体は良いことだが、やたらと丁寧なコメントに頼っているコードは、裏を返せばコメントなしでは理解しづらコードになっているかもしれない
「この関数では ◯◯ をしています」など、コードの内容そのものを逐一説明するコメントが多い場合は要注意
コメントがなくてもコードの意図が伝わるように改善する
「関数の抽出」
コメントで説明が必要なコード片は、その説明がそのまま伝わるような関数名でメソッドに切り出す
「関数名・変数名の変更」
コードの目的がわかりにくいなら、コメントを書く代わりに意味が明確になる名前にリネームする