コードスメルについて調べてみた

コードスメルとは様々なプログラムのあるべきではない状態のことを指します．コードからすぐ見て取れる重複部分の存在や，実際に変更を加える際に発生する手間の多さなど様々なパターンがこれまでに分類され，それらに対する改善方法が提案されています．

今回，仕事でコードを書く上でどのよう書かれているべきかを体系的に知っておきたいという思いでコードスメルについて調査しました，

本記事は Code Smells (SOURCEMAKING) の素晴らしいまとめを日本語訳したものです．５つのコードスメル大分類と，それぞれの小分類についての解説と改善方法がまとめられています(しかも具体的なリファクタリング方法については言語別に！)．ところどころ記述を省いていたり，自身の解釈が混じっている可能性があるので，気になる箇所は直接読んでみてください.理解を助ける画像等もあり，英語に抵抗感がなければ直接読まれたほうが理解が深まると思います．

尚，何がコードのスメルで、何がそうでないかは主観的な判断であり、プログラミング言語、開発者や開発手法などによって異なります．SOURCEMAKING でのまとめはコードスメルを体系的にまとめた一つに過ぎないことは注意してください．

Code Smells (SOURCEMAKING)

Bloaters 膨れ上がったコード

• 様々な目的が一つにまとめられたメソッド，クラス．コードの量が多く複雑
• プログラムが発達していく過程で様々な役割が追加され作られてしまうことが多い

Long Method

• 特徴・問題点
  • コードの行数が多すぎるメソッド．一般的に10行以上のメソッドはリファクタリングを検討したほうが良い
  • メソッドの目的が理解しづらく，メンテナンス性が低い
  • 新たなメソッドを作るよりも既存のメソッドにコードを追加したほうが精神的負荷が低く行われてしまう傾向がある
• 改善方法
  • コメントが必要だと思った箇所は新しいメソッドに切り出，その処理内容を端的に説明したメソッド名つける．
• 改善のメリット
  • メソッド名がメソッドの役割を端的に表していれば，切り出したメソッドの詳細を追わなくても処理全体の流れを理解することができる

Large Class

• 特徴・問題点
  • 多くのフィールド，メソッド，行数を抱えているクラス
  • 複数の役割を同一クラス内に担ってしまっている場合が多い
  • 新たなクラスを作るよりも既存のクラスにコードを追加したほうが精神的負荷が低く行われてしまう傾向がある
• 改善方法
  • クラスの役割をそれぞれ別のクラス，サブクラス等に切り出す
• 改善のメリット
  • 役割を複数のクラスに分割することで，一つのクラスの役割について覚えるべき内容が減る．その結果全体の処理の流れを追いやすくなる，
  • 役割ごとに切り出すことで，再利用が可能．類似したコードや機能の複製を防ぐことができる．

Primitive Obsession

• 特徴・問題点
  • クラスのフィールドに多数の変数が存在する状態，個々のフィールドがどの処理に関わるのか，どんな役割を担うのかが分かりづらい
  • 配列に複数の役割をになう変数が格納されているが，各要素が 例) data = ['Adam', '20', 'Japan', 'ITEngineer']
  • 簡単な処理に必要なだけだからと，フィールドがどんどん追加されるが，それらをオブジェクトにまとめ直すことは精神的負荷が高く行われないことが多い
• 改善方法
  • 同一の処理に用いられるフィールドをまとめたオブジェクト（クラス等）を作成して，一つにまとめる
• 改善のメリット
  • 特定のデータに対する処理が同じオブジェクトに対して行われるため，コードの流れがわかりやすくなる．
  • 変数の役割について推測する必要が減る
  • 同一の処理がまとめられるので，コードの複製に気づきやすくなる

Long Parameter List

• 特徴・問題点
  • メソッドに4以上のパラメータが存在する
  • 複数種類のアルゴリズムが一つのメソッドに統合された際に起こりやすい．どのアルゴリズムがどのように走るのかを制御するためにパラメータが増える
  • 他のメソッドの処理結果を引数で受けている場合，その変数が何を表しているのか，どのように作成されたのか等が分かりづらい
  • パラメータ間の関係が把握しづらく，コードの可読性が低い
• 改善方法
  • 関連しない複数の引数がある場合は，それらを別のオブジェクトにまとめる
  • 他のメソッドの処理結果を引数で受ける代わりに，メソッド内でその処理を実行するようにする -> 引数で受ける必要がなくなる
• 改善のメリット
  • 可読性が高い，短いコードになる
  • 以前は気づかなかった，コードの複製に気づくかもしれない
• 注意
  • ２つのクラスの間に依存関係を生んでしまう場合は無視しても良い

Data Clumps

• 特徴・問題点
  • 異なるコードで全く同じ引数グループが存在する
  • コピーペーストでプログラムを作成していると起こりがち
• 改善方法
  • 引数を一つのクラスにまとめる
• 改善のメリット
  • コードの量が減り，コードの構成がわかりやすくなる
• 注意
  • ２つのクラスの間に依存関係を生んでしまう場合は無視しても良い
    

Object-Orientatoin Abusers 未熟なオブジェクト指向

• オブジェクト指向プログラミングをしっかり適用できていないケース

Switch Statements

• 特徴・問題点
  • 複雑な switch 文や連続する if 文が存在する
• 改善方法
  • 切り替えている処理をメソッドに切り出す
  • 同じメソッドを異なるパラメータで呼び出している場合は，処理全体を異なるメソッド，クラスに分割してしまい，それぞれを必要に応じて呼び出す
• 改善のメリット
  • コードの構成がわかりやすくなる

Temporary Field

• 特徴・問題点
  • 特定の状況下でのみ用いられ，それ以外では使われないフィールドがクラス内に存在する
  • 多数の引数が必要となるアルゴリズムのために作られる傾向がある．特定のメソッドの引数を増やす代わりに，フィールドを複数作成して対応するが，特定のメソッド以外ではそれらのフィールドは使われない．
  • オブジェクト内のフィールドにデータが入っていない，入っていてもどれが本当に使われるものか分からない という状態になりコードの可読性が下がる
• 改善方法
  • 特定のメソッドと，それに関わる引数をまとめた新たなクラス（メソッドオブジェクト）を作成し，分離する
• 改善のメリット
  • コードの流れがわかりやすくなる

Refused Bequest

• 特徴・問題点
  • サブクラスが，継承した親クラスのproperty, method の一部しか使用していない
  • 親クラスの内のコードを一部再利用したいために継承されるが，親クラスとサブクラスの目的が異なる場合に発生する傾向がある
    • 例：4本足という理由で chair クラスが dog クラスを継承している
• 改善方法
  • 親クラスとサブクラスの間に関係がない場合には，継承(Inheritane)をやめて委譲(delegate)する
  • 継承が妥当な場合は，サブクラス内の不必要なフィールド，メソッドを削除する．親クラスから，サブクラスで必要とされている全てのフィールドとフィールドと抽出し，新しいクラスサブクラスに入れる．そして，元々の親クラス，サブクラスに新しく作ったサブクラスを継承させる
• 改善のメリット
  • コードが整理され，各クラスの役割がはっきりする
    • 例：足が4本という理由で，dog クラスが chair クラスに継承されることがなくなる (leg クラスを抽出して作成し，dog, chair クラスに継承させる)

Alternative Classes with Difference Interfaces

• 特徴・問題点
  • ２つのクラスが，名前は異なるが機能は全く同じクラスをメソッドを有している
  • 既存のクラスのへの理解が不足していたために新たに作成されてしまう傾向
• 改善方法
  • インターフェースを統一し，親クラスにメソッドを抽出する．共通の機能を利用しているクラスに新たに作成したクラスを継承させる
    • 同じ機能を持つメソッドは，全てのクラスで同じ名前に統一する．処理をパラメタライズする等でパラメータが一部異なるメソッド統一する (5%discount(), 10%discount() を discount(x) に統一するなど)
• 改善のメリット
  • 不必要に重複していたコードが取り除かれ，コード可読性が上がる
• 注意
  • 同様のメソッドを持っているクラスが，他のライブラリーに含まれる一部のクラスで，異なるバージョン管理をされているものだったりすると，クラスを統合することが難しい場合もあるので適宜対応

Change Preventers 変更を妨げるもの

• コードの一部に変更を加えようとすると，他の箇所にも多くの変更が必要になってしまう状態
• 放置しているとコードの開発・改修が複雑で困難になっていしまう，

Divergent Change

• 特徴・問題点
  • クラスに変更を加える際に，関係のない多数のメソッドに変更を加えなければいけない状態
  • 例えば，新しい product type を加える際に，product を find, display, orderするメソッドにも変更を加えなければいけない
  • プログラムが構造化されておらずコピペで作成されている場合に必要となる傾向がある
• 改善方法
  • クラスの機能を複数のクラスに分割する
  • 異なるクラスが同じ機能を実装している場合は，共通の機能を親クラスに抽出し，各クラスに作成した親クラスを継承させる
• 改善のメリット
  • コードが構造化され，重複が削減される結果，可読性が上がる
  • コードのメンテナンス性が向上する

Shotgun Surgery

• 特徴・問題点
  • 何か一つの変更を加える際に，多くの異なるクラスに小さな変更を加えなければならない状態
  • 一つの役割に対する責務が複数のクラスに散らばっていると発生してしまう
• 改善方法
  • 既存のクラスの挙動を一つのクラスに統合する
• 改善のメリット
  • コードが構造化され，重複が削減される結果，可読性が上がる
  • コードのメンテナンス性が向上する

Parallel Inheritance Hierarchies ?

• 特徴・問題点
  • あるクラスに対してサブクラスを作成する際に，もう一つの別のクラスに対してもサブクラスを作成しなければならないと感じる状態
  • 類似した継承構造を持つクラス群が複数存在する状態
• 改善方法
  • 1. 一方の継承構造に含まれるインスタンスに，もう一方の継承構造のインスタンスを参照させる
  • 2. 参照されたクラスにおける継承構造を削除する
• 改善のメリット
  • コードの重複がなくなり，可読性が向上する
  • コードのメンテナンス性が向上する
• 注意
  • 類似した継承構造は，プログラム全体の構造がよりひどいものになるものを回避するために作成されていることがあるので注意

Dispensables なくてもよいもの

• 存在しないほうが，コードがキレイで読みやすくなるもの

Comments

• 特徴・問題点
  • 説明文が長々と書き込まれているメソッド
  • コメントは，コードが実現しようとしていることが分かりづらいと作者が思ったときに加えられるものだが，それは臭いものに対して香水を吹きかけてごまかしているに過ぎない（場合もある）
• 改善方法
  • 複雑な処理について説明しようとしているときには，理解しやすい，途中過程を格納する変数を用意することで，処理を分割しすることで，説明を不要にする
  • 一つのまとまった処理を行うコードブロックに対して説明をしている場合には，そのコードブロックで実現している機能をメソッドに抽出し，コメントで説明している内容を端的に示したメソッド名をつける
  • システムが動くために必要な条件を警告しているようなコメントの場合は代わりに assert 文を用いる
• 改善のメリット
  • コードが実現しようとしている機能が明確になる
• 注意
  • 下記のようなコメントは有用なので残す
    • 何を ではなく 何故 実装されているのかを説明するコメント
    • 複雑なアルゴリズムを説明する時（上記の方法はすでに行われ，コード自体は十分端的になっている場合）

Duplicate Code

• 特徴・問題点
  • ２つのコードがほとんど同じにみえるケース
  • 複数人で同じプログラムの異なる部分を実装している場合に，互いに何を実装しているのか十分に理解していないと発生してしまう
  • コードの一部が異なるが，実際には同じ機能を実現している というコードの重複もありうる
• 改善方法
  • 同じ継承レベルの２つのサブクラスに同じコードが存在する場合
    • ２つのクラスから共通の機能とそれらで使われているフィールドを親クラスのメソッドとフィールドとして抽出する
    • コンストラクタに共通部分がある場合は，親クラスのコンストラクタに抽出
    • 類似しているが，完全には一致していない場合は，共通している部分までを親クラスのメソッドに抽出し，異なる部分のみをそれぞれのクラスに残す
  • 異なる２つの暮らしに同じコードが存在する場合
    • 継承構造に含まれていない場合は，共通の親クラスを作成し，共通部分を親クラスに抽出する
    • 親クラスを作成するのが難しい場合は，共通部分を新たなクラスに抽出し，それを両方のクラスで用いる
  • 複数の条件文が存在し，条件が違うのみで，同じコードが実行されている場合（条件は違うが全て 0 を返す等）は条件文を一つのメソッドにまとめてしまう
  • 各条件分岐の中で，同じコードが実行されている場合は共通部分を関数化するなどして条件分岐内から取り除く
• 改善のメリット
  • コードの重複がなくなり，全体が短くなるため可読性が向上する

Lazy Class

• 特徴・問題点
  • コードのリファクタリングが行われた結果，担う役割がほとんどなくなってしまっているクラス
  • 将来的に拡張が考えられていたがそのままで放置されているクラス
• 改善方法
  • 他のクラスに役割を統合してしまう
  • 少数の機能しか持たないサブクラスは親クラスに統合してしまう
• 改善のメリット
  • 不要なクラスが減ることでメンテナンスしやすくなる

Data Class

• 特徴・問題点
  • フィールドとそれらの getter, setter のみを有するクラス
• 改善方法
  • data class が用いられている箇所で，data class 内部に持つべき機能がある場合は， data class 内のメソッドに抽出する
  • 変数を必要に応じてプライベート化する（配列等の変数の場合は，意図せず要素の値が替えられてしまわないように immutable なものを用いるように変更する)
• 改善のメリット
  • 特定のデータに関する処理が一つのクラスにまとめられることで，コードの可読性が向上する

Dead Code

• 特徴・問題点
  • 使われていない変数，メソッド，クラス
  • 要件の変更等があった際に発生しやすい．
  • 複雑な条件分岐の中に埋もれている場合がある
• 改善方法
  • 不要なコードは削除する
  • IDEを活用して unreachable な箇所を検出する
• 改善のメリット
  • コードがシンプルになり，可読性が向上する

Speculative Generality

• 特徴・問題点
  • 将来必要だからと実装されたが，結局使われていない変数，メソッド，クラス
  • 不必要なコードがあることで可読性が下がる
• 改善方法
  • 削除！!
• 改善のメリット
  • 不要なコードが削除され可読性が向上する
• 注意
  • フレームワークを作成してる際には，フレームワーク内では使われていなくても，フレームワークのユーザーが使う機能は当然残す
  • 削除する前に，ユニットテストでそれらが使用されていないか注意，テストに使う情報を得るためにそうしたクラス，メソッドが使われているかもしれない

Couplers 不適切な結合・分離

• クラスをまとめ過ぎたり，もしくは分割しすぎたりした際に発生するスメル

Feature Envy

• 特徴・問題点
  • 自身のクラスのデータよりも，他のクラスのデータに多くアクセスしているメソッド
  • あるクラスのフィールドがデータクラスに移された場合に発生する傾向がある．
  • データと，それを使用するメソッドが別々に管理されているため，可読性が低い，変更を加えづらい
• 改善方法
  • 明らかに移すべきメソッドはデータを保持しているクラスに移してしまう．
  • メソッドの一部のみ外部データにアクセスしている場合は，その部分を別のメソッドに抽出し，そのメソッドをデータが存在するクラスに移してしまう
• 改善のメリット
  • データを扱うメソッドがデータの側に記述されるため理解しやすくなる．
• 注意
  • データを扱う処理を動的に変更するために，意図的にデータと，そのデータを扱う処理が分離されている場合もあるので注意

Inappropriate Intiacy

• 特徴・問題点
  • あるクラスが他のクラスのメソッドやフィールドを多く用いている状態
  • 他のクラスに強く依存しているクラスは維持・再利用が難しい
• 改善方法
  • 用いているメソッド，フィールドが存在するクラスを継承する．
  • 共通して用いられているメソッド，フィールドを新たな別のクラスに抽出し，Hide Delegate することで code relation を "official" にする？ -> 呼び出す流れを明示的にすることで処理の流れを追いやすくするということか？
• 改善のメリット
  • クラス間の依存関係が減少することで，コードが理解しやすくなる
  • 各クラスを再利用しやすくなる

Message Chains

• 特徴・問題点
  • 呼び出し元が特定のオブジェクトを呼び出した際に，そのオブジェクトがまた別のオブジェクトを呼び出して・・・と処理が数珠つなぎになっている場合 (a->b()->c()->d())
  • 呼び出し元は呼び出した先に存在するクラス群の関係性に依存してしまう．この関係性が変わった際には，呼び出し元も変更しなければならない
• 改善方法
  • Hide Delegate する ?
• 改善のメリット
  • クラスの間の連鎖の依存性を減らすことができる

Middle Man

• 特徴・問題点
  • 他のクラスの機能に delegate する一つの 機能しか持たないクラス
  • 単に，他のクラスの間の中間に存在するだけなのでなくても良い
• 改善方法
  • 削除して，呼び出し元と呼び出し先を直接つなげる
• 改善のメリット
  • 不要なコードが減り，処理の流れを追いやすくなる．

参考

コードの臭い (WIKIPEDIA)
Code smell (WIKIPEDIA)
コードスメルの深刻度がリファクタリングの実施に与える影響の実証的研究
- コードスメルの意義等が簡潔にまとめられている，最初の掴みにちょうどよい
- コードスメルの種類と対応するリファクタリングが表形式でまとめられている
Code Smells and Its type (With Example)
- コードスメルについてまとめられたスライド
How to find common code smells and avoid them
Code Refactoring: Learn Code Smells And Level Up Your Game!
- コードスメルについての講演動画
- 動画内で実際のコードスメルの例も登場

あとで読む

Episode #150: Technical Lessons Learned from Pythonic Refactoring
Python Design Patterns: For Sleek And Fashionable Code View all articles
PyCon.DE 2017 Yenny Cheung - Technical Lessons Learned from Pythonic Refactoring
What are some common Python code smells and how should one refactor them?