『Refactoring』の有名な一節に「コードの臭い」に関するものがあります。これは良くない設計のコードに見られる特徴を「臭い(bad smell)」という言葉で表現したもので、ネット上でもこれらに言及した記事を見かけます。
https://qiita.com/NagaokaKenichi/items/22972e6ba698c7f2978a
今回、第2版を読み進めていて、第1版とは異なるリストになっていることに気づきました。そこで、第1版と比較して、第2版がどうなっているか紹介します。なお、第2版で追加された臭いには見出しに「☆」を、削除された臭いには「★」をつけています。また、第1版についてはオーム社版を参照しています。
☆奇妙な名前(Mysterious Name)
初っぱなから第1版にはない「臭い」です。「コンピュータサイエンスにおいて難しいことは2つしかない。キャッシュの破棄と名前付けである」というPhil Karltonの警句にもあるように、名前付けは難しく、それゆえに名前の変更を行うリファクタリングは頻繁に行うことになります。
When you can’t think of a good name for something, it’s often a sign of a deeper design malaise.
いい名前が思いつかないなら、設計がまずい兆候である。
Refactoring, 2nd edition
重複したコード(Duplicated Code)
同じコードが2カ所以上に表れるなら、1カ所にまとめることでコードを改善できます。
☆長すぎる関数(Long Function)
第1版では「長いメソッド」でしたが、関数に変わっています。これはサンプルコードで使用する言語（=執筆時点で最も人気のある言語）がJavaからJavaScriptに変わったことが影響していると思われます。
経験の浅いプログラマーは短い関数を嫌い、全てをmain関数に押し込みます。しかし、プログラミングのためのメンタルモデルが育ってくると、全体像を頭に入れつつ必要に応じて細部を把握することができるようになってきます。こうなると、短い関数の連なりによって構成されたプログラムの方が理解しやすくなります。また、コードの再利用性のためにも、短く役割のはっきりした関数を書くことは重要です。
短い関数を使って、保守性の高いコードを書くための鍵は名前付けです。名前が関数の役割を的確に表していれば、その関数の中身を見なくても、コードを理解することができます。
また、一つの教訓として、「何かにコメントを付けたいと思ったときは、その処理を関数化すべきである」というものがあります。
長すぎるパラメータリスト(Long Parameter List)
関数に必要なものを引数として渡すのは、悪いことではありません。グローバル変数に依存するよりは、はるかにマシです。しかし、長すぎるパラメータリストは悪いコードの兆候です。
引数リストが長い場合、そのリストを1つのオブジェクトにする(Introduce Parameter Object)といったリファクタリングが適用できます。
☆グローバルデータ(Global Data)
グローバル変数への依存が「悪いこと」であるのはよく知られていることだと思いますが、意外なことに第1版ではグローバル変数への言及はありません。これは、第1版がJavaで、グローバル変数という概念がなかったからでしょうか？
一方、JavaScriptプログラミングでは、グローバル変数を頻繁に触ります。たとえばライブラリの設定方法がグローバル変数になっていることは珍しくありません。
しかし、グローバル変数は、いったん使用が広まると、それを置き換えるのは大変です。使用は避けられないとしても、抑制的な使い方が重要です。
☆変更可能なデータ(Mutable Data)
データへの変更は、予期しないバグを生み出します。関数型プログラミングの考え方では、データ構造に変更を加える際は、値を書き換えるのではなく、変更後の値を持った新しいデータ構造を返すべきである、とされています（そのようにしかできない言語もあります）。
変更可能なデータのスコープが数行程度であれば問題ありませんが、スコープの広い変数が変更可能である場合、それが問題を引き起こすリスクは高まります。
変更の偏り(Divergent Change)
1つのモジュールが様々な理由によって変更されているのは良くない兆候です。
このような場合、そのモジュールが持つ責務が多すぎることが考えられます。フェーズの分割(Split Phase)等のリファクタリングを適用して、責務を明確にし、変更理由が1つになるようにすべきです。
変更の分散(Shotgun Surgery)
1つの目的を持った変更が様々なモジュールに分散してしまうのも、良くない兆候です。
この場合、分散した責務を集約したモジュールを作成し、変更箇所が分散しないようにすべきです。
特性の横恋慕(Feature Envy)
あるモジュールの関数が、別モジュールのデータや関数とばかりコミュニケーションを取っている場合は、その関数を別モジュールに移動させたほうが良いかもしれません。
データの群れ(Data Clumps)
いつも連れ立って使用されるデータは、1つのクラスにまとめた方がいいかもしれません。たとえば、ユーザの名前とメールアドレスは、別々の変数として管理するのではなく、Userクラスのフィールドにするのが良いでしょう。
基本データ型への執着(Primitive Obsession)
整数や文字列といった基本データ型のみを使用してプログラムを書くことも、経験の浅いプログラマーに見られる兆候です。
たとえば、郵便番号を単なる文字列型に格納することがあります。しかし、郵便番号の扱いには独特のドメインロジックがあり、単純な文字列以上の存在とみなすことができます。このような場合は、基本データ型をオブジェクトで置き換えることを検討すべきです。
☆繰り返されるswitch文(Repeated Switches)
オブジェクト指向言語では、条件分岐をポリモーフィズムで置き換え可能です。第1版の出版当時は、このことを強調するために、switch文を使うことは悪い兆候とみなしていました。
現在では、ポリモーフィズムは広く使われるようになり、一方でswtich文にパターンマッチのような洗練された機能を持たせた言語も現れています。
このような状況を鑑みて、「switch文それ自体は悪ではなく、同じswitch文が何度も出てくるのが悪い兆候である」とトーンダウンしています。
☆ループ(Loops)
第1版の出版当時において、Javaにはループの代替となる機能はありませんでした。しかし、現在では、Javaも含め様々な言語で、ループのより良い代替が存在します。
JavaScriptの場合、多くのケースで、ループを配列のfilter/map/reduceなどに置き換えることが可能です。
☆怠け者(Lazy Element)
第1版では「怠け者クラス（Lazy Class）」でしたが、より汎用的な名前に変わっています。
役に立たないモジュールは削除すべきである、ということです。
リファクタリングの結果不要になるようなケースもありますし、変更を予期して作った間接化のレイヤーが不要だとわかることもあります。
疑わしき一般化(Speculative Generality)
将来の変更に備えるという目的でコードを書いたが、実際には変更が発生せず、無駄に柔軟な（その分、複雑な）コードになっていることがあります。
このような場合は、そのコードをよりシンプルな形に変更した方がよいかもしれません。
一時的属性(Temporary Field)
特定の状況でしか必要とされないフィールドは、別のクラスに切り出した方がよいかもしれません。
メッセージの連鎖(Message Chains)
メソッドチェーンには注意が必要です。呼び出し側がチェーンの途中で呼び出されるオブジェクトに強く依存することになるためです。
第三者のメソッドを呼び出すようなメソッドチェーンは、デメテルの法則にも違反します。
このような場合、委譲の隠蔽(Hide Delegate)を使用して、チェーンを隠蔽すべきです。
仲介人(Middle Man)
隠蔽が行き過ぎて、クラスのほとんどのメソッドが別のオブジェクトに委譲をするだけになっているような場合は、仲介人の削除(Remove Middleman)を適用し、移譲先のオブジェクトに直接メッセージを送るようにすべきです。
☆インサイダー取引(Insider Trading)
モジュール間でのデータの受け渡しはモジュールの結合度を高めます。ある程度のデータの受け渡しは必要不可欠ですが、過度にデータを交換しているオブジェクトには注意が必要です。
このような場合、お互いが共通して使用するデータを持つ別のクラスを用意した方がよいかもしれません。
また、継承にも注意が必要です。サブクラスはスーパークラスのことを知りすぎることが少なくありません。継承を委譲に置き換えることで、過度のデータ交換を抑制できます。
巨大なクラス(Large Class)
巨大すぎるクラスは大量のフィールドを抱えていることが多いです。
クラスの抽出(Extract Class)を適用し、フィールドをひとまとめにしましょう。スーパークラスの抽出(Extract Superclass)や種別コードのサブクラスへの置き換え(Replace Type Code with Subclasses)が使えるかもしれません。
クラスのインタフェース不一致(Alternative Classes with Different Interfaces)
あるモジュールから別のモジュールへの置き換えを可能にするには、インタフェースが同じである必要があります。代替可能なはずのモジュールのインタフェースが異なっている場合は、関数宣言の変更(Change Function Declaration)等を使用して、インタフェースを揃えましょう。
データクラス(Data Class)
フィールドとゲッター・セッター以外に何も持たないようなクラスは、単なるデータの保持者です。
データクラスは、必要な振る舞いが誤った場所に書かれている兆候です。そのデータクラスを使用しているコードを、データクラス自身に持たせるようにできないか検討してみましょう。
この原則には例外があり、関数呼び出しの結果を表すオブジェクトなどは独自の振る舞いを持たないクラスとなることがあります。
相続拒否(Refused Bequest)
サブクラスがスーパークラスの一部しか利用していない場合、その継承関係が本当に必要か検討すべきです。
メソッドやフィールドの階層を変更したり、継承を委譲に置き換えたりといったリファクタリングを適用する余地があります。
コメント(Comments)
コメント自体は悪いものではありませんが、悪いコードを説明するためにコメントを手厚く書いているような場合があります。
このような場合、関数の抽出などを通して、コメントが不要なコードに改善することができます。
★パラレル継承(Parallel Inheritance Hierarchies)
第2版では削除。複雑な継承関係は廃止すべき。
★未熟なクラスライブラリ(Incomplete Library Class)
第2版では削除。基盤ライブラリの設計がまずいと、それに立脚したコードを書くのが大変になる。
感想
「イミュータブルなデータは不吉な臭いである」といった、第1版の出版後に広まった知見も反映されていて、現代風にしっかりアップデートされていると思いました。第1版を読んだことがある方も、第2版を読めば新しい発見があると思います。