LoginSignup
243
240

More than 5 years have passed since last update.

新装版 リファクタリング 既存のコードを安全に改善するを改めて読んでみて「第3章 コードの不吉な臭い」が面白かったので、読書ログとして残しておく。

91Zo4BdfTmL.jpg

コードの不吉な臭いとは

コードに深刻な問題、リファクタリングを必要とするコードが存在することを示す何らかの兆候、雰囲気を、臭いで比喩して表したもの。

1. 重複したコード

同じようなコードが2箇所以上で見られたら、1箇所にまとめることを考えると良いプログラムになる。重複したコードが存在すると、その箇所を改修する時に同じ改修を重複コード分行う必要があるため、保守性が低くなりがちになる。

2. 長すぎるメソッド

長いメソッドは理解するのが大変。長すぎるメソッドは小さいメソッドに分けていくと良い。ただ、小さいメソッドに分けることでコードを追っていくのが大変になるが、 適切なメソッド名を命名することで内部の実装を見なくても読み進めることが可能になる。メソッド名には、内部でどのような処理をしているかではなく、そのコードが何をするのかという意図を示す。こうした分割を行うことで以前よりもコードが長くなったとしても、意図が明確になっていればOK。メソッドの長さを切り詰めるのではなく、メソッド名とその実装との距離を埋めることが重要。

3. 巨大なクラス

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

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

パラメータがあまりに多いと、1つ1つが何を意味しているのか理解しづらくなる。それぞれのパラメータ間の一貫性がなくなり、使いにくくなる。何より、新たなデータが必要になった時にパラメータリストを変更しなければならない。こうしたケースはパラメータオブジェクトを用意しそれを渡すようにしてやると変更の影響を受けにくくなる。

5. 変更の偏り

1つのクラスが別々の理由で何度も変更される状況では変更の偏りが起こっている。例えば、「データベースが新しくなるたびに、いつもこの3つのメソッドを変更しなければならないし、金融商品を追加するたびに毎回この4つのメソッドを修正している」等が同一クラスで見られるケースがこれにあたる。こうした場合はクラスを2つに分けたほうがすっきりする。

6. 変更の分散

変更を行うたびにあちこちのクラスが少しずつ書き換わる(1つの変更要求に対して複数のクラスが書き換わる)ケース。変更すべき箇所が全体に広がると、探すのが難しくなり重要な変更を実装し忘れる場合も出てくる。こうした場合は変更部分を1つのクラスにまとめあげる等することで、変更箇所を局所化できる。

7. 特性の横恋慕

オブジェクト指向には処理および処理に必要なデータを1つにまとめてしまおうという重要な考え方があるにもかかわらず、あるメソッドが自分のクラスよりも他のクラスに興味を持つようなケース。たいていの場合、メソッドの位置が悪いのでメソッドを移動することで解決できる。

8. データの群れ

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

9. 基本データ型への執着

オブジェクトにちょっとした仕事をさせる場合には、小さなクラスを作ることが面倒に感じることがある。例えば、住所を表すために、県、市、町、番地などをそれぞれStringとして人クラスのフィールドに作成するようなケース。こうしたクラスが大きくなってくると、保守性が下がり将来の変更にも弱くなりやすい。

10. Switch文

Switch文は重複したコードを生み出す問題児。コードのあちこちに同じようなSwitch文があると、新たな分岐を追加した時に全てのSwitch文を探して似たような変更をしていかなければならない。オブジェクト指向ではポリモーフィズムを使いこの問題を解決する。Switch文を見たらポリモーフィズムが使えないかを検討すると良い。

11. パラレル継承

新たなサブクラスを定義するたびに、 別の継承ツリーにもサブクラスを定義しなければならない状況。ある継承ツリー中のクラスに付けられたクラス名のプレフィックスが、別の継承ツリ中のクラス名のプレフィックスと同じ時はこの臭いを疑ったほうがよい。

12. 怠け者クラス

十分な仕事をせず、その見返りに合わないようなクラスは排除するべき。リファクタリングの結果クラスのダウンサイジングにより不要になる場合や、変更を予期して作成したクラスが実際には役に立っていない場合があるので、そういったクラスは排除する。

13. 疑わしき一般化

「いつか必要になりそうな機能だから」という理由で、現在は必要としていないのに凝った仕掛けや特殊な状況を考えているケース。いわゆるYAGNI。無用の長物なら削除したほうが良い。

14. 一時的属性

インスタンス変数の値が特定の状況でしか設定されないオブジェクトがある。通常オブジェクトは、値を属性として常に保持するものなので、そうしたコードは非常に理解しづらい。原因として、複雑なアルゴリズムがいくつもの変数を必要としている場合がある。

15. メッセージの連鎖

クライアントがあるオブジェクトにメッセージを送り、受け取ったオブジェクトがさらに他のオブジェクトに送り、それがまた別のオブジェクトに送る、、、というケース。オブジェクトをナビゲートする過程の構造にクライアントが強く依存することになる(中間のオブジェクトの関連が変わるたびに、クライアントが影響を受けてしまう)。

16. 仲介人

カプセル化はしばしば権限の委譲をもたらすがこれが過剰となる場合もある。例えば、メソッドの大半が別のオブジェクト(仲介人)に委譲しているだけのクラス等がそれにあたる。こうした場合は、仲介人を除去して本当に仕事をするオブジェクトに直接処理させる、仲介人メソッドがわずかであればメソッドをインライン化して呼び出し側にその部分を埋め込む、等すると良い。

17. 不適切な関係

クラス同士が必要もないのに密接に結びついているケース。

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

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

19. 未熟なクラスライブラリ

クラスライブラリの作成者も全能ではなく、それを非難することは意味のないこと。ソフトウエアの設計が正しいと確信できるのは、かなりの構築が進んでからがほとんどなので、万能なライブラリをはじめから作成しておくのは非常に困難。問題なのは、第三者によるクラスライブラリの修正が不可能であるか、可能であっても混沌とした形で行われるということ。

20. データクラス

属性とsetter/getterしか持たないクラスは単なるデータ保持用であるため、他のクラスからのアクセスを過剰に受けがちになる。開発の初期段階では、こういったクラスの属性はpublicで定義されることもあるが、なるべく早い段階からカプセル化を行っていくと良い。

21. 相続拒否

サブクラスは親の属性と操作を継承するのが普通だが、ほんの一部しか利用していない場合がある。こうしたケースは継承階層が間違っている、というのが伝統的な見方。

22. コメント

コメントは悪い臭いではなくむしろいい香り。問題としているのは、コメントが消臭剤として使われているケースがあるということ。コメントが非常に丁寧に書かれているということは、実はコードの分かりにくさを補うため、ということがよくある。コメントの必要性を感じた時は、リファクタリングを行ってコメントを書かなくても内容が伝わるコードを目指すと良い。

各臭いに対する対応

新装版 リファクタリング 既存のコードを安全に改善するの別章に対応方針が載っているので、気になる人は是非読んでみてほしい。

243
240
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
243
240