コードの不吉な臭い

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)を読んでます。

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)

リファクタリングのきっかけとなる「コードの不吉な臭い」の章を備忘録としてまとめる。
コードに対してなんとなく気持ち悪いなって感じるけれど、なぜそう感じるのかという理由が読むと分かる。

重複したコード

同じようなコードが2か所以上あったら1か所にまとめるといいコードになる。

長すぎるメソッド

長いメソッドほど理解がしにくい。
コメントの必要性が感じたら、わかりやすい名前をつけたメソッドに分割していく。 メソッド名が適切なら、内部の実装を見なくても先を読み進めていける。

大事なのは、メソッドの長さを切り詰めるのではなく、メソッド名とその実装との距離を埋めること。

巨大なクラス

1つのクラスがあまりに多くの仕事をしている場合は、たいていインスタンス変数の持ち過ぎになっている。
インスタンス変数が多いと、重複したコードが存在する可能性も高くなる。

長すぎるパラメータリスト

パラメータの数があまりに多いと、ひとつひとつが何を意味しているか理解しづらくなる。
また、新たなデータが必要になった時に、パラメータリストを変更していかなければならない。
オブジェクトを渡すようにしていれば、パラメータとして渡されてきたオブジェクトに問い合わせを少し追加するだけで済むので、影響が少なく済む。

ただし、例外としてパラメータに渡すオブジェクトに対して依存関係を持ちたくない変数は、別のパラメータとして渡すべき。

変更の偏り

1つのクラスが別々の理由で何度も変更される場合は、「変更の偏り」が起きている。
変更しなければならない時は、変更箇所が1つで済むようにするのがいい。
変更要求に対処するためにはクラスと1つだけ変更し、バリエーションが増えた時は新しいクラスを追加するというのが望ましい。

変更の分散

変更を行うたびにあちこちのクラスが少しずつ書きかわる場合、不吉な臭いと受け取った方がいい。 変更すべき箇所が全体に広がると、探すのが難しくなり、重要な変更を実装し忘れる場合が出てくる。
変更部分を1つのクラスにまとめ上げるようにする。

特性の横恋慕

オブジェクト指向には、処理及び処理に必要なデータを1つにまとめるという重要な考え方がある。あるメソッドが、他のクラスに興味を持つような場合は、古典的な誤りを犯している。
たいていはメソッドの位置が悪いので、正しい位置に定義してあげるようにメソッドの移動を行う。

データの群れ

数個のデータがグループとなって、クラスのフィールドやメソッドのシグニチャなど、さまざまな箇所に現れることがある。
こうした群れをなしたデータは、オブジェクトとして1つにまとめるべき。

基本データ型への執着

たった、1、2項目のためにオブジェクトを定義するのは面倒に思えるかもしれないが、意味あるまとまりをオブジェクトにまとめることによって流用しやすくなる。

スイッチ文

スイッチ文は重複したコードを生み出す問題児。 ポリモーフィズムを使って解決する。

パラレル継承

新たなサブクラスを定義するたびに、別の継承ツリーにもサブクラスを定義しなければならない状況のこと。
ある継承ツリー中のクラス名につけられたプレフィックスが、別の継承ツリー中のクラス名のプレフィックスと同じ時に、この臭いを疑ってみたほうがいい。
一方の継承ツリーのインスタンスが、もう一方の継承ツリーのインスタンスを参照するようにすることで、この重複を取り除くことができる。

怠け者クラス

一度作成したクラスは理解や保守のためのコストがかかるため、十分な仕事をせずに、その見返りに合わないようなクラスは排除すべき。

疑わしき一般化

いつか必要になる機能、と言って、現在では必要としない凝ったものを考えている時には警戒が必要。
この仕掛けが全て使われているならば価値はあるが、無用の長物なら削除した方がいい。

一時的属性

通常オブジェクトは、値を属性として常に保持するものである。特定の状況下でのみインスタンス変数が設定されるオブジェクトは、非常に理解がしづらくなる。
一時的属性が定義される原因として、たくさんのパラメータで受け渡しをしたくないから、属性として定義することが考えられる。

メッセージの連鎖

クライアント→オブジェクト→オブジェクト…というように、オブジェクトが別のオブジェクトにメッセージを送るという過剰な連鎖が起こることがある。 このような形の場合、オブジェクト間で強く依存があるため、中間のオブジェクトの関連が変わるたびにクライアントが影響を受けることになってしまう。

getXXX()のようなメソッド呼び出しが長い行になって連なっていたり、一時変数がたくさん使われていると、この現象が起きていると判断できる。

仲介人

メソッドの大半が別のオブジェクトに委譲しているだけのクラスは、本当に仕事をするオブジェクトに直接処理をさせることを考えてもいい。

不適切な関係

クラス同士の関係が密接すぎる。

クラスのインタフェース不一致

処理は同じでシグニチャのみが異なるメソッド。

未熟なクラスライブラリ

クラスライブラリの作成者も全能ではないので、万能のライブラリをあらかじめ作成することは非常に難しいこと。
問題なのは、クラスのライブラリの修正が困難なこと。

データクラス

属性と、getメソッド、setメソッド以外には何も持たないクラス。
こうしたクラスは単なるデータ保持用であり、他のクラスからのアクセスを過剰に受けがちなので、なるべく早い段階からカプセル化をしていくべき。

相続拒否

親の属性や操作を一部しか利用していない場合、使われていない属性や操作を兄弟クラスへ移すことが伝統的なやり方。 ただし、継承したものが使われていないことで混乱や問題を引き起こしている場合に限って、伝統的な解法を使うようにすればいい。うまくいっている場合は、ほうっておいて大丈夫。

コメント

コメントは、不吉な臭いの予兆。リファクタリングによって不吉な臭いの除去を行う。