コードの臭い
目次
主要項目のみを表示しています。詳細な小見出しは本文内で確認できます。
- 概要
- コードの臭いとは
- なぜ臭いとして名前を付けるのか
- 臭いの分類
- 肥大化した臭い
- 長いメソッド
- 大きすぎるクラス
- 長い引数リスト
- 基本型への執着
- データの塊
- OOPの誤用
- switch文
- 拒絶された遺産
- 一時フィールド
- インターフェースの異なる代替クラス
- 変更を妨げる臭い
- 発散的変更
- 散弾銃手術
- 並行継承階層
- 不要なもの
- コメント
- 重複したコード
- 怠惰なクラス
- データクラス
- 死んだコード
- 投機的一般化
- 結合に関する臭い
- 機能の横恋慕
- 不適切な親密さ
- メッセージの連鎖
- 仲介人
- リファクタリング第二版で追加された臭い
- 臭いとリファクタリングの対応
- 臭いの見つけ方
- 現代開発への接続
- よくある誤解
- 臭いの言語別の現れ方
- 臭いを検出するツール
- 臭いと技術的負債
- 臭いの優先順位付け
- 臭いとレビュー文化
- 臭いの教育
- 臭いと現代の言語進化
- まとめ
- 参考文献
概要
コードの臭い(code smell)は、コードのある場所が「何かおかしい」と感じさせる表面的な兆候です。バグではなく、設計上の弱さの指標です。臭いを嗅ぎ取る感覚は、リファクタリングのきっかけになります。
コードの臭いは、Kent Beck が Martin Fowler の 「リファクタリング」 執筆を手伝う中で命名した語彙で、コードを観察したときに違和感を覚える典型的な兆候を整理したものです。長いメソッドや神クラスのような名前で呼ばれ、それぞれに対応するリファクタリング技法が用意されています。臭いは欠陥ではなく仮の指標であり、必ずしも修正を必要としませんが、放置するほど技術的負債が積み上がります。
コードの臭いとは
コードの臭いという言葉は、Kent Beck が Martin Fowler の 「リファクタリング」(1999年初版)の執筆を手伝っていたときに命名しました。Fowler 自身は 「コードの臭い」 の Bliki(個人 Wiki)で次のように説明しています。
コードの臭いとは、たいていシステム内のより深い問題に対応している表面的な兆候である。
特徴は次の通りです。
- バグではない: コードは正しく動く
- 設計上の弱さを示す: 変更や拡張がしにくい
- 経験的: 数値で厳密には決まらない
- リファクタリングの動機: 必ず直すべきではないが、観察対象になる
- 名前が付いている: チーム内で短く共有できる
「リファクタリング」 の章 「コードの悪い臭い」 には、Fowler が長年のレビュー経験から抽出した22個の臭いがリストアップされています。第二版(2018)では再整理され、いくつかが追加・統合されました。
なぜ臭いとして名前を付けるのか
「臭い」という比喩は、コードの問題を客観的・絶対的なものとして扱わず、違和感を直感で捉える という性質を表しています。
- 健康診断のような数値判定ではない
- レビューで「ここは Long Method っぽい」と短く伝えられる
- 経験を積むほど嗅覚が鋭くなる
- 必ずしも修正を要求しない(指標であり、判定ではない)
デザインパターンが「うまくいく解決策」、アンチパターンが「失敗の典型」、コードの臭いが「修正候補のシグナル」です。臭いを嗅ぎ取れるようになると、レビューやリファクタリングの会話が短くなります。
臭いの分類
Refactoring Guru などのリソースでは、コードの臭いは5つのカテゴリに分類されます。
| カテゴリ | 性質 | 主な臭い |
|---|---|---|
| 肥大化した臭い | コードが肥大化している | Long Method, Large Class, Long Parameter List, Primitive Obsession, Data Clumps |
| OOPの誤用 | OOPの不完全/誤った適用 | Switch Statements, Refused Bequest, 一時フィールド, Alternative Classes |
| 変更を妨げる臭い | 変更が広く波及する | Divergent Change, Shotgun Surgery, Parallel Inheritance Hierarchies |
| 不要なもの | 不要なもの | Comments, Duplicated Code, Lazy Class, Data Class, Dead Code, Speculative Generality |
| 結合に関する臭い | クラス間の結合が強すぎる | Feature Envy, Inappropriate Intimacy, Message Chains, Middle Man |
すべて暗記する必要はありません。コードを見たときに違和感を覚えたら、どのカテゴリに近いかを当てはめられると、対処の方向が決まります。
肥大化した臭い
Bloaters は、コードや構造が膨らみすぎて扱いづらくなった状態を指します。一気には現れず、時間とともに少しずつ大きくなります。Refactoring Guru は次のように述べています。
肥大化した臭いとは、扱うのが難しいほど巨大になってしまったコード、メソッド、クラスである。
肥大化は、変更を恐れたり、責務の境界を引き直さなかったりすると進行します。早期発見と継続的な分割が予防策です。
長いメソッド
長いメソッド(Long Method)は、Fowler の 「リファクタリング」 で繰り返し強調される臭いです。
手続きが長くなるほど、理解は難しくなる。
メソッドが長くなるほど、理解が難しく、変更がしにくくなります。目安として、一画面に収まらないなら長すぎる という経験則がよく使われます。
兆候:
- スクロールしないと全体が見えない
- 内部に小見出し的なコメントが必要
- 多重ネストの条件文や繰り返し
- 一時変数が多く、責務がぼやけている
リファクタリング:
- Extract Method(メソッド抽出): 論理的なまとまりを別メソッドに切り出す
- Replace Temp with Query(一時変数の問い合わせ置換): 一時変数を関数呼び出しに変える
- Decompose Conditional(条件記述の分解): 複雑な条件を別メソッドに抽出する
- Replace Method with Method Object(メソッドオブジェクトによる置換): 巨大メソッドを別オブジェクトに移す
Long Method は最も頻繁に出会う臭いの一つです。コメントで「ここまで初期化、ここから処理、ここから返却値構築」のように区切りを示したくなったら、それぞれを別メソッドにする合図です。
大きすぎるクラス
大きすぎるクラス(Large Class)は、Long Method の高次元版です。クラスに多くのフィールドとメソッドが詰まり、複数の責務を抱えている状態です。
兆候:
- フィールド数が10以上
- メソッド数が30以上(粗い目安)
- クラス名が
Manager、System、Helper等で抽象的 - 一部のフィールドが特定のメソッドでしか使われない
- クラスの説明が「〇〇と〇〇と〇〇を担当する」と複数つながる
リファクタリング:
- Extract Class(クラス抽出): 関連するフィールドとメソッドを別クラスに切り出す
- Extract Subclass(サブクラス抽出): 一部の振る舞いだけを別サブクラスに移す
- Extract Interface(インターフェース抽出): 利用されているメソッド群をインターフェースとして抽出する
単一責任原則 と密接に関連します。アンチパターンの God Object と地続きの臭いで、「リファクタリング」 の This class is too large という記事で Fowler が詳しく論じています。
長い引数リスト
長い引数リスト(Long Parameter List)は、メソッドの引数が3〜4個を超え、呼び出し時に何が何だか分からなくなる状態です。
# 悪い例
def create_user(name, email, age, country, role, plan, currency, lang, tz):
...
問題:
- 呼び出し側で引数の順序や意味が混乱する
- 引数を一つ追加するたびに全呼び出し元に変更が波及する
- テストが書きにくい
リファクタリング:
- Introduce Parameter Object: 関連する引数をひとつのオブジェクトにまとめる
- Preserve Whole Object: 個別フィールドではなく親オブジェクトを渡す
- Replace Parameter with Method Call: 計算可能な引数は呼び出し先で計算する
- Remove Setting Method: 不変オブジェクトを使う場合の整理
Python のキーワード引数、TypeScript の interface、Kotlin の data class など、現代の言語機能を使えばパラメータオブジェクト化が低コストで実現できます。
基本型への執着
基本型への執着(Primitive Obsession)は、本来ドメイン概念であるべき値を、文字列や整数のような基本型のままで扱い続けることを指します。
# 悪い例
def send_money(from_account: str, to_account: str, amount: int):
...
# 改善
def send_money(from_account: AccountId, to_account: AccountId, amount: Money):
...
兆候:
- 電話番号、メールアドレス、IDが文字列のまま
- 金額が整数のまま(通貨情報がない)
- 状態がint定数で表現されている
- 型がドキュメントなしには意味を持たない
リファクタリング:
- Replace Data Value with Object: 値オブジェクトを作る
- Replace Type Code with Class/Subclass: 型コードを型に置き換える
- Introduce Parameter Object: 関連プリミティブをまとめる
ドメイン駆動設計 の Value Object パターンと深く関連します。型システムを使ってドメインを表現することで、引数取り違え、不正な値、ロジックの分散を防げます。
データの塊
データの塊(Data Clumps)は、いくつかのデータが常に一緒に現れる現象を指します。
例:
start_dateとend_dateがいつもセットで渡されるstreet、city、zip、countryがいつもセットで現れるfirstName、lastNameがいつもセットで使われる
これらは無名のドメイン概念です。DateRange、Address、FullName のような型で表現すべきです。
リファクタリング:
- Extract Class(クラス抽出): データの塊を新しいクラスに抽出する
- Introduce Parameter Object: 引数として塊で渡されるなら、まずそこから整える
- Preserve Whole Object: 親オブジェクト経由でアクセスする
「3つは群れである」 というFowlerの直感則があります。三つ以上のデータが繰り返し一緒に現れるなら、それは一つのクラスになりたがっているサインです。
OOPの誤用
このカテゴリは、OOPの機能を不完全または誤って使っている状態を指します。継承、多態性、カプセル化を活用すれば自然に解消されるが、命令型の発想で書かれているために違和感が残るコードです。
switch文
switch文(Switch Statements)は、switch や if-else if 連鎖で型コードや状態を分岐する書き方です。OOP では多態性で置き換えることが推奨されます。
# 悪い例
def area(shape):
if shape.type == "circle":
return 3.14 * shape.radius ** 2
elif shape.type == "square":
return shape.side ** 2
elif shape.type == "triangle":
return 0.5 * shape.base * shape.height
# 改善
class Shape:
def area(self): ...
class Circle(Shape):
def area(self): return 3.14 * self.radius ** 2
class Square(Shape):
def area(self): return self.side ** 2
リファクタリング:
- Replace Conditional with Polymorphism: 条件分岐を多態性に置き換える
- Replace Type Code with Subclasses: 型コードをサブクラスに変える
- Replace Type Code with State/Strategy: 状態やストラテジーパターンに変える
ただし、すべての switch が悪というわけではありません。値の単純な変換、列挙型のマッピング、最近のSealed Classes/Tagged Unions では、網羅性チェック付きの switch/match が安全に使えます。新しい型が増えるたびに既存コードを書き換える必要があるかどうかが判断基準です。
拒絶された遺産
拒絶された遺産(Refused Bequest)は、サブクラスが親クラスから継承したメソッドやフィールドを使わない状態を指します。
兆候:
- 親クラスのメソッドが空実装でオーバーライドされている
- 親クラスの一部のフィールドが意味を持たない
NotSupportedExceptionを投げるオーバーライド
これは「is-a」関係が成立していない、つまり LSP(Liskov Substitution Principle)に違反する典型例です。
リファクタリング:
- Push Down Method/Field: 一部のサブクラスでしか使わないものは下に押し下げる
- Replace Inheritance with Delegation: 継承を委譲に変える
- Extract Superclass: 共通部分だけ別の親クラスに切り出す
継承は「再利用」のためではなく「概念上の包含関係」のために使う、というのがOOPの本来の考え方です。Refused Bequest は、誤った包含関係を見直すきっかけになります。
一時フィールド
一時フィールド(Temporary Field)は、特定のメソッドの実行中だけ意味を持つフィールドが、クラスのインスタンス変数になっている状態です。
例:
class ReportGenerator:
def __init__(self):
self.intermediate_results = {} # generate() 中だけ使う
self.error_count = 0 # generate() 中だけ使う
def generate(self):
self.intermediate_results = ...
...
問題:
- インスタンスの状態が、メソッド呼び出し前後で意味を持たない
- スレッドセーフでない
- メソッド外からは「どのフィールドがいつ有効か」が読めない
リファクタリング:
- Extract Class(クラス抽出): 計算プロセスを別クラスに切り出す
- Replace Method with Method Object: メソッドそのものをオブジェクト化する
- Introduce Null Object: 不在を null ではなく Null Object で表現する
メソッドの内部変数として閉じ込められるなら、クラスのフィールドにする必要はありません。
インターフェースの異なる代替クラス
インターフェースの異なる代替クラス(Alternative Classes with Different Interfaces)は、似た役割のクラスが複数あるが、インターフェースがバラバラな状態です。
例:
- 異なるストレージ実装が
save()とstore()で名前が違う - ロガー実装が
log()とwrite()で異なる - バリデータが
validate()とcheck()で混在
リファクタリング:
- Rename Method: 同じ意味のメソッドは同じ名前にする
- Extract Superclass / Extract Interface(インターフェース抽出): 共通の抽象を作る
- Move Method, Move Field: 機能を一致させる
統一されたインターフェースは、利用側で抽象に依存できる(DIP)ようになり、テストやモックが容易になります。
変更を妨げる臭い
このカテゴリは、コードが変更しにくい構造になっていることを示す臭いです。一つの変更が複数箇所に波及したり、複数の変更が一つの場所に集中したりする状態です。
発散的変更
発散的変更(Divergent Change)は、一つのクラスが複数の理由で変更される状態です。
兆候:
- データベースの変更でクラスを修正
- UI の変更で同じクラスを修正
- バリデーションルールの変更でも同じクラスを修正
これは 単一責任原則 違反の典型です。複数の関心事が一つのクラスに混ざっています。
リファクタリング:
- Extract Class(クラス抽出): 関心ごとに別クラスに分ける
- Move Method/Field: 責務に合った場所へ移動
「このクラスを変更する理由を一文で説明できるか」と問うと、Divergent Change の存在に気付けます。
散弾銃手術
散弾銃手術(Shotgun Surgery)は、Divergent Change の逆で、一つの変更のために複数の場所を修正する必要がある状態です。
例:
- 通貨フォーマットを変更したいが、20箇所で別々にフォーマットしている
- 認証ルールを変えるのに、10個のサービスを直す必要がある
- 設定値の変更がコードベース全体に散らばっている
兆候:
- 変更時のレビュー範囲が常に広い
- どこを直せばよいか把握しにくい
- 一部だけ修正されて整合性が崩れる
リファクタリング:
- Move Method/Field: 散らばった変更点を一箇所に集める
- Inline Class: 過剰に分割されたクラスを統合する
Divergent Change と Shotgun Surgery は対の関係です。Divergent は「一つの場所に多くの変更」、Shotgun は「一つの変更が多くの場所」。両方とも、責務の配置の見直しが必要です。
並行継承階層
並行継承階層(Parallel Inheritance Hierarchies)は、Shotgun Surgery の特殊形で、一方のクラス階層にサブクラスを追加するたびに、もう一方の階層にも対応するサブクラスを追加しなければならない状態です。
例:
Employee階層にEngineerを追加すると、EmployeeReport階層にもEngineerReportを追加する必要がある
リファクタリング:
- Move Method/Field: 一方の階層に処理を集約する
- Replace Inheritance with Delegation: 階層を平坦化する
並行する階層は、設計の対称性に見えるが、実は変更の二重コストを生みます。
不要なもの
このカテゴリは、なくても問題ないか、むしろない方がいいコードを指します。コードベースをスリム化し、ノイズを減らす対象です。
不要なものとは、取り除くことでコードがよりきれいで効率的になり、理解しやすくなる、意味のないものや不要なものである。
コメント
Comments(コメント)は、Refactoring Guru で意外なことに「臭い」として扱われています。
コメントはたいてい善意から生まれる。書き手が、自分のコードが直感的でも明白でもないと気づいたときに書かれる。
つまり、コメントは「コード自体が読めない」ことの回避策である場合が多い、という指摘です。
兆候:
- コードの意図を説明するコメントが多い
- 何をしているかではなく、なぜそうするかを書かないと分からない
- コメントとコードが乖離している(メンテされていない)
リファクタリング:
- Extract Method + 適切な命名: コメントの代わりにメソッド名で意図を表す
- Rename Method/Variable: 名前で意図を表現する
- Introduce Assertion: コメントの前提条件をアサーションで明示する
ただし、コメントすべてが悪ではありません。
- 採用理由(rationale)の記録
- 使い方のドキュメント
- 著作権情報
- 公開API のドキュメンテーションコメント
- なぜそうしないか(避けた選択肢)の説明
「コメントは消臭剤である」(コメントは消臭剤)という比喩が知られます。臭うコードに振り掛けるのではなく、まずコード自体を改善することを考えましょう。
重複したコード
重複したコード(Duplicated Code)は、Fowler が 「最悪の臭い」 の一つと呼ぶ最重要な臭いです。
同じコード構造が複数の場所に現れたなら、それらを統合する方法を見つければ、プログラムはより良くなる。
DRY 原則と直接対応します。重複したコードは、保守時に片方だけ修正されて整合性が崩れる典型原因です。
リファクタリング:
- Extract Method: 共通部分を関数に抽出する
- Pull Up Method: サブクラスに重複したメソッドを親に集める
- Form Template Method: 共通の骨格を残し、差分だけサブクラスに任せる
- Substitute Algorithm: より良いアルゴリズムに置き換える
ただし、Rule of Three(Don Roberts)と組み合わせて、3度目の重複で抽象化する 慎重さも必要です。早すぎる抽象化は別の臭い(Speculative Generality)を生みます。
怠惰なクラス
怠惰なクラス(Lazy Class)は、十分な責務を持たないクラスのことです。
例:
- メソッドが1〜2個しかない
- フィールドが1個だけ
- 他のクラスへ単純に委譲しているだけ
過去のリファクタリングで分割しすぎたか、将来を見越して作った抽象が結局使われなかった結果として生じます。
リファクタリング:
- Inline Class: クラスをそれを使う側に統合する
- Collapse Hierarchy: サブクラスと親クラスを統合する
「このクラスがなくなったら何が困るか」と問うと、Lazy Class かどうか判断できます。
データクラス
データクラス(Data Class)は、フィールドとアクセサ(getter/setter)だけで、振る舞いを持たないクラスです。
class User:
def __init__(self, name, email, age):
self.name = name
self.email = email
self.age = age
# 何もない
問題:
- データを操作するロジックが他のクラスに散らばる
- Tell Don’t Ask 違反
- ドメイン知識がフィールド名以外にない
リファクタリング:
- Move Method: 関連するロジックをこのクラスに移す
- Encapsulate Field: フィールドを直接公開せず、振る舞いで囲む
ただし、DTO(Data Transfer Object)、Value Object、最近の Python の dataclass、Java の record、Kotlin の data class のような「意図的にデータだけ」のクラスは別物です。役割が明確で、ドメインロジックが他の場所にあるべきなら、Data Class は適切な選択です。
死んだコード
Dead Code(デッドコード)は、実行されることのないコードです。
例:
- 一度も呼ばれない関数
- 到達しない条件分岐
- 削除された機能の残骸
- 古いインポート
リファクタリング:
- Delete: 削除する
- 静的解析ツールで検出(dead code analyzer)
「使うかもしれない」という理由で残すのは、Boat Anchor アンチパターンと同じです。バージョン管理から復活できるので、潔く削除するのが現代の常識です。
投機的一般化
Speculative Generality(推測による一般化)は、いつか必要になるかも という推測で抽象、フック、設定を増やすことです。
兆候:
- 単一実装の抽象クラス
- 使われていない拡張ポイント
- 過剰なジェネリクス
- 取られない引数
YAGNI 原則と直接対応します。アンチパターンの章でも詳しく扱いましたが、コード臭としても重要です。
リファクタリング:
- Inline Class: 単一実装の抽象を実装に取り込む
- Collapse Hierarchy: 階層を平坦化する
- Remove Parameter: 使われない引数を取り除く
- Rename Method/Class: より具体的な名前にする
Rule of Three と組み合わせて、3度目の必要性が見えてから抽象化を導入する姿勢が、Speculative Generality を防ぎます。
結合に関する臭い
このカテゴリは、クラス間の結合が強すぎる、または不適切な状態の臭いです。OOP の本質である「カプセル化と独立性」を脅かします。
機能の横恋慕
Feature Envy(メソッドの引っ越し願望)は、あるクラスのメソッドが、自クラスより他クラスのフィールドやメソッドを多く使う状態です。
# Feature Envy
class Order:
def total_with_tax(self):
return self.amount + self.amount * self.customer.address.tax_rate
# 改善
class Order:
def total_with_tax(self):
return self.amount + self.customer.tax_for_amount(self.amount)
リファクタリング:
- Move Method: メソッドを羨んでいるクラスへ移動する
- Extract Method + Move Method: 一部だけが羨んでいるなら抽出してから移動
Tell Don’t Ask 原則と Law of Demeter とも関連します。「データのある場所で計算する」のが、結合を弱める基本です。
不適切な親密さ
Inappropriate Intimacy(不適切な親密さ)は、二つのクラスが互いの内部に深く立ち入っている状態です。
兆候:
- クラスが他クラスのプライベートフィールドや実装詳細にアクセスする
- クラス同士が双方向参照を持ち、片方を変えるともう片方も壊れる
- 抽象化が機能していない
リファクタリング:
- Move Method/Field: 関連の強いものを一方に集める
- Extract Class(クラス抽出): 共通の中間概念を抽出する
- Hide Delegate: 委譲関係を隠蔽する
- Replace Inheritance with Delegation: 過剰な親密さを切る
カプセル化を保つには、必要最小限のインターフェースだけを公開し、内部実装は隠す姿勢が重要です。
メッセージの連鎖
Message Chains(メッセージチェーン)は、a.getB().getC().getD().doSomething() のような長い呼び出し連鎖を指します。
問題:
- 中間の構造変更で呼び出し元すべてが壊れる
- Law of Demeter 違反
- テストが書きにくい
リファクタリング:
- Hide Delegate: 中間の取得をメソッドで隠す
- Extract Method: チェーンを意図のある名前に置き換える
- Move Method: 操作をデータの近くに移す
ただし、すべての連鎖が悪ではありません。Fluent インターフェース(builder.set(...).set(...).build())や関数型のチェーン(stream.filter(...).map(...).collect(...))は、意図的な設計です。問題は「内部構造の漏洩」であり、「チェーンの存在」自体ではありません。
仲介人
Middle Man(仲介者)は、自分では何もせず、ほぼ全てのメソッドが他クラスへ委譲しているクラスを指します。
例:
class Order:
def __init__(self, customer):
self.customer = customer
def name(self): return self.customer.name()
def email(self): return self.customer.email()
def address(self): return self.customer.address()
# 全部 customer に委譲しているだけ
過剰な委譲は、Message Chains を避けようとしすぎた結果として生まれることもあります。
リファクタリング:
- Remove Middle Man: 仲介を削除し、利用側が直接アクセスする
- Inline Method: 委譲を直接呼び出しに置き換える
- Replace Delegation with Inheritance: 委譲が常に同じ対象なら継承を検討
ただし、Decorator、Proxy、Adapter といったデザインパターンでは、意図的に仲介クラスを使います。Middle Man の臭いは「目的のない委譲」であり、パターンに沿った委譲は別物です。
リファクタリング第二版で追加された臭い
Martin Fowler は2018年の Refactoring: Improving the Design of Existing Code, 2nd Edition で、いくつかの新しい臭いを追加・整理しました。
謎めいた名前
Mysterious Name(謎めいた名前)は第二版で第一の臭いに位置付けられました。Fowler は次のように述べています。
名前を付けることはプログラミングで難しいことの一つだが、定義をよく考えた良い名前は、何時間もの理解不能を避けさせてくれる。
兆候:
- 変数名が
x、tmp、data、resultのように汎用すぎる - 関数名が
process()、handle()、do()のように動作不明 - クラス名が
Manager、Helperのように責務不明 - コメントなしには読めない名前
リファクタリング:
- Rename Variable / Method / Class
- 命名は単独でも意味が伝わるように
- ドメインの語彙を借りる
- レビューで命名を議論する
「2 hard things in programming」は Phil Karlton のジョーク(There are only two hard things in Computer Science: cache invalidation and naming things.)から来ています。
グローバルデータ
Global Data(グローバルデータ)は、グローバル変数、シングルトン、ミュータブルなクラス変数など、どこからでも変更可能な状態です。
# 悪い例
config = {}
def init_app():
global config
config["env"] = "production"
def use_config():
print(config["env"]) # どこで設定されたか追えない
問題:
- どこから変更されたか追えない
- テスト間で状態が漏れる
- 並行アクセスでrace condition
リファクタリング:
- Encapsulate Variable: グローバル変数を関数経由でアクセス
- Move to Class: 状態を持つクラスに移動
- Dependency Injection: 必要なところに明示的に渡す
可変データ
Mutable Data(変更可能なデータ)は、可変オブジェクトの過剰な使用が、バグや並行性問題の温床になるという臭いです。
問題:
- いつ誰が変更したか追跡困難
- 並行アクセスでの一貫性
- デバッグ困難
リファクタリング:
- Encapsulate Variable: 直接アクセスから関数経由へ
- Split Variable: 異なる用途で再利用される変数を分ける
- Replace Derived Variable with Query: 派生値は計算する
- 不変オブジェクト化(イミュータブル)
関数型プログラミングの文化(Clojure、Elixir、Haskell、Erlang)で重視される視点が、Refactoring 第二版で取り入れられました。
ループ
Loops(ループ)は、第二版で初めて臭いとして言及されました。Java Stream、JavaScript Array methods、Python list comprehension など、関数型のパイプラインが普及した現代では、命令的なループが時代遅れに感じられる場合があります。
# Loops(命令的)
result = []
for item in items:
if item.active:
result.append(item.name.upper())
# Pipeline(関数型)
result = [item.name.upper() for item in items if item.active]
# あるいは
result = list(map(
lambda item: item.name.upper(),
filter(lambda item: item.active, items)
))
リファクタリング:
- Replace Loop with Pipeline
- 言語のコレクション操作(Java Stream、LINQ、Ruby Enumerable)を活用
ただし、ループが必ず悪いわけではありません。早期終了、複雑な状態遷移、副作用ありの処理ではループの方が自然です。
繰り返されるswitch
Repeated Switches(繰り返される switch)は、同じ条件分岐が複数箇所に現れる状態です。第一版の Switch Statements を拡張して、同じ switch が 繰り返し 出てくる点を強調しました。
# Repeated Switches
def get_role_label(role):
if role == "admin": return "Administrator"
elif role == "user": return "User"
elif role == "guest": return "Guest"
def get_role_permissions(role):
if role == "admin": return ["read", "write", "delete"]
elif role == "user": return ["read", "write"]
elif role == "guest": return ["read"]
def get_role_color(role):
if role == "admin": return "red"
elif role == "user": return "blue"
elif role == "guest": return "gray"
新しいロールを追加するたびに、3箇所すべてを更新する必要があります。
リファクタリング:
- Replace Conditional with Polymorphism
- Replace Type Code with Subclass / State / Strategy
- Map / Dictionary でルックアップテーブル化
内部者取引
Insider Trading(インサイダー取引)は、モジュール間で内部情報をこっそり交換している状態を指します。Inappropriate Intimacy の発展形です。
兆候:
- パッケージ間の private アクセス
- friend 宣言の濫用
- 非公開フィールドへの間接アクセス
- 隠れたリフレクション
リファクタリング:
- Move Method/Field: 共有が必要なら統合
- Hide Delegate: 間接アクセスを隠蔽
- Extract Class(クラス抽出): 共通部分を独立させる
モジュールの境界を保つには、どの情報が公開で、どの情報が私的か を明示する設計が必要です。
ループとパイプライン
これはループ(Loops)の項目とほぼ同じですが、第二版では明確にループから関数パイプラインへの移行が推奨されています。
// Loops
const total = 0;
for (const order of orders) {
if (order.status === "completed") {
total += order.amount;
}
}
// Pipelines
const total = orders
.filter(o => o.status === "completed")
.reduce((sum, o) => sum + o.amount, 0);
パイプライン(Pipelines)は意図が明確で、変更しやすい傾向があります。
臭いとリファクタリングの対応
コードスメルを見つけたら、臭いの種類に応じたリファクタリング技法を選びます。代表的な対応を整理します。
| 臭い | 主なリファクタリング |
|---|---|
| Long Method | Extract Method, Replace Temp with Query |
| Large Class | Extract Class, Extract Subclass, Extract Interface |
| Long Parameter List | Introduce Parameter Object, Preserve Whole Object |
| Primitive Obsession | Replace Data Value with Object, Replace Type Code |
| Data Clumps | Extract Class, Introduce Parameter Object |
| Switch Statements | Replace Conditional with Polymorphism, Replace Type Code with State |
| Refused Bequest | Push Down Method/Field, Replace Inheritance with Delegation |
| 一時フィールド | Extract Class, Replace Method with Method Object |
| Divergent Change | Extract Class |
| Shotgun Surgery | Move Method/Field, Inline Class |
| Parallel Inheritance | Move Method/Field |
| Comments | Extract Method + Rename, Introduce Assertion |
| Duplicated Code | Extract Method, Pull Up Method, Form Template Method |
| Lazy Class | Inline Class, Collapse Hierarchy |
| Data Class | Move Method, Encapsulate Field |
| Dead Code | Delete |
| Speculative Generality | Inline Class, Remove Parameter, Collapse Hierarchy |
| Feature Envy | Move Method |
| Inappropriate Intimacy | Move Method/Field, Extract Class, Hide Delegate |
| Message Chains | Hide Delegate, Extract Method |
| Middle Man | Remove Middle Man, Inline Method |
これらの技法は Refactoring Guru(refactoring.guru)や Sourcemaking(sourcemaking.com/refactoring)に詳しい解説があります。
臭いの見つけ方
臭いを嗅ぎ取る感覚は、経験で磨かれます。以下のような実践が役立ちます。
- ペアプログラミング、モブプログラミング: 別の人の視点で違和感を発見する
- コードレビューの観察: 「ここが読みにくい」を言語化する
- 静的解析ツール: SonarQube、Code Climate、ESLint、Pylint などが多くの臭いを自動検出する
- 行数とサイクロマティック複雑度: 機械的な閾値で見つける(Long Method、Large Class)
- 依存関係グラフ: クラス間の結合を可視化する
- Hot Spot 分析: 変更頻度とバグの集中箇所を見る
ツールに頼りすぎると見えなくなる臭いもあります。Speculative Generality や Cargo Cult Programming は、ドメイン理解と意図のレビューがないと検出しにくいです。
現代開発への接続
コードの臭いの概念は、OOP/Java 時代に確立されましたが、現代の言語やパラダイムにも適用できます。
| 領域 | 現れ方 |
|---|---|
| 関数型プログラミング | Long Method(純粋関数でも長すぎる)、Primitive Obsession(型を作らない) |
| マイクロサービス | Shotgun Surgery(一機能が複数サービスにまたがる)、Middle Man(無意味な中継サービス) |
| フロントエンド | Large Class(巨大コンポーネント)、Feature Envy(コンポーネント間の不適切な依存) |
| インフラ as Code | Duplicated Code(似た Terraform モジュール)、Speculative Generality(過剰なパラメータ化) |
| テスト | Long Method(巨大テスト)、Duplicated Code(テスト間の重複)、Comments(説明過多のテスト) |
| データパイプライン | Long Parameter List(多数の設定)、Switch Statements(型ごとの処理分岐) |
新しい言語機能(パターンマッチ、record、data class、algebraic data type、nullable types)は、Primitive Obsession や Switch Statements のような古典的な臭いを減らす道具になっています。
よくある誤解
コードの臭いに関する誤解を整理します。
- 臭いは欠陥ではない: 動作するコードに対する違和感の指標です。
- すべての臭いを直す必要はない: 変更頻度と影響範囲を見て優先順位を決めます。
- 臭いの名前は攻撃の道具ではない: コードレビューで相手を貶めるのではなく、議論の起点にします。
- 自動検出だけでは見えない: ツールはヒントを提供しますが、最終判断は人間の理解が必要です。
- 文脈次第で臭いではないことも: 意図的なFluentインターフェース、DTO、デザインパターンに基づく仲介クラスは、表面的には臭いに見えても問題ない場合があります。
臭いを学ぶ目的は、コードを読む語彙を増やし、改善の対話を始めること です。完璧なコードは存在しないので、嗅ぎ取った臭いをどう扱うかが、開発文化の質を決めます。
臭いの言語別の現れ方
同じ臭いでも、言語ごとに表現や対処法が変わります。
Python での臭い
# Long Method
def process_user_data(data):
# 100行...
# Primitive Obsession
def transfer(from_id: int, to_id: int, amount: int):
pass
# Mutable Data
class Cart:
items = [] # クラス変数として共有される(バグ)
対処:
- @dataclass で値オブジェクトを軽量に作成
- typing モジュールで型ヒント
- frozenset、tuple で不変構造を使う
Java での臭い
// God Class
public class UserManager {
// 50メソッド、20フィールド
}
// Long Parameter List
public User createUser(String name, String email, int age,
String country, String role, ...) { }
// Switch Statements
public String getStatus(int code) {
switch (code) {
case 1: return "Active";
case 2: return "Inactive";
}
}
対処:
- record(Java 14+)で値オブジェクト
- sealed class(Java 17+)で型階層を制限
- pattern matching(Java 21+)で条件分岐をスマートに
JavaScript/TypeScriptでの臭い
// Magic Numbers
if (user.age > 17) { ... }
// Mutable Data
const state = { count: 0 };
state.count++; // 直接変更
// Callback Hell(Spaghetti の現代版)
fetchUser(id, (user) => {
fetchOrders(user.id, (orders) => {
fetchItems(orders[0].id, (items) => {
// ...
});
});
});
対処:
- const + readonly で不変
- async/await でフラットに
- TypeScript のリテラル型・union 型で Magic Strings を排除
Ruby での臭い
# Feature Envy
class Order
def total_with_tax
@amount + @amount * @customer.address.tax_rate
end
end
# Data Class(DTO の濫用)
class User
attr_accessor :name, :email
end
対処:
- ActiveSupport::Concern で振る舞いを mixin
- Value Object で Primitive Obsession 解消
- POROs で SRP を守る
Go での臭い
// Long Parameter List
func CreateUser(name, email, country, role string, age int, ...) error { }
// Inappropriate Intimacy
type Server struct {
db *Database
}
func (s *Server) HandleRequest(r *Request) {
s.db.connection.Lock() // 内部にアクセス
}
対処:
- struct で関連パラメータをまとめる
- Go の interface は実装側で宣言不要なので、ISP 自然に守りやすい
- 公開非公開を大文字小文字で明示
Rust での臭い
Rust の所有権システムは、Mutable Data や Inappropriate Intimacy を構造的に防ぎます。一方、Long Method、Primitive Obsession のような臭いは普通に発生します。
// Primitive Obsession
fn transfer(from: u64, to: u64, amount: u64) { }
// 改善
struct AccountId(u64);
struct Money(u64);
fn transfer(from: AccountId, to: AccountId, amount: Money) { }
newtype パターンが Rust の Primitive Obsession 対策として頻繁に使われます。
臭いを検出するツール
静的解析ツールで多くの臭いを自動検出できます。
多言語対応
- SonarQube: 多言語、組織レベルの集約
- Code Climate: GitHub 連携、Maintainability スコア
- Codacy: GitHub/GitLab 連携
言語特化
- Java: SpotBugs、PMD、Checkstyle
- Kotlin: Detekt、ktlint
- Python: Pylint、Flake8、Ruff、mypy
- JavaScript/TypeScript: ESLint、TSLint(廃止)、Biome
- Go: go vet、staticcheck、golangci-lint
- Rust: Clippy
- C/C++: Cppcheck、Clang-Tidy
- Ruby: RuboCop、Reek
- C#: Roslyn analyzers、SonarLint
メトリクス
- Cyclomatic Complexity: McCabe の複雑度。10超過で要注意、20以上は危険
- Cognitive Complexity: 人間の理解しやすさを測る
- Lines of Code (LOC): 関数100行、クラス500行が目安
- Coupling Between Objects (CBO): クラス間結合
- Lack of Cohesion of Methods (LCOM): メソッド間の凝集度
これらをCIで継続的に監視すると、臭いの蓄積を早期発見できます。
臭いと技術的負債
コードの臭い(Code Smell)は技術的負債の早期警告です。Ward Cunningham の 技術的負債の比喩と組み合わせると次のように捉えられます。
- 臭いは技術的負債の
利子の発生源 - 放置すると複利で増える
- リファクタリングは元本の返済
- 計画的な返済(リファクタリング予算)が長期健康を保つ
Steve McConnell は技術的負債を四つに分類しました(「技術的負債の管理」、ICSE 2007):
- 不注意かつ意図的: 「とりあえず動かせばいい」と意識して妥協
- 不注意かつ非意図的: スキル不足、設計の未熟
- 慎重かつ意図的: 「今は急ぐ、後で返す」と意識的に
- 慎重かつ非意図的: 「正しいと思っていたが、後から気付いた」
慎重かつ意図的 な技術的負債は、ビジネス判断として正当化できます。問題は他の3つで、認識されないまま蓄積する負債です。
Code Smell を検出して可視化することは、技術的負債を意識的に管理する第一歩です。
臭いの優先順位付け
すべての臭いを同時に直すのは不可能です。優先順位付けが重要です。
優先度を高くする条件:
- 変更頻度が高いコード(hot spot)
- セキュリティリスクのあるコード
- バグが集中する場所(バグ集中の Pareto Principle)
- 新機能開発を阻害する場所
- 性能ボトルネック
優先度を低くする条件:
- 死んでいるコード(Boat Anchor)でリスクが小さい
- 変更頻度が低い安定した場所
- 影響範囲が限定される
「犯罪現場としてのコード」 という書籍(Adam Tornhill)は、Git の履歴と複雑度を組み合わせて、ホットスポットを可視化する手法を提唱しています。これは臭いの優先順位付けに役立ちます。
臭いとレビュー文化
臭いを継続的に検出し改善するには、文化的な仕組みが必要です。
コードレビューチェックリスト
- 命名は意図を表現しているか(Mysterious Name)
- 関数は短いか(Long Method)
- クラスの責務は単一か(Large Class)
- 引数は多すぎないか(Long Parameter List)
- 重複はないか(Duplicated Code)
- 抽象化は今必要か(Speculative Generality)
- 依存方向は正しいか(Inappropriate Intimacy)
ペアプログラミング
二人で書くことで、臭いの早期発見と知識共有が同時に進みます。ペアプログラミングは、目の数を増やす というよりも、書きながら気付く 文化の構築に効きます。
モブプログラミング
3人以上で一つのコードを書く形式。ドライバーとナビゲーター のロールを交代しながら進めます。臭いに対する集合知が働き、合意形成も同時に行えます。
TDD/BDD
テストファーストで書くと、テスト容易性が悪い構造(God Object、Inappropriate Intimacy、Singleton 濫用)を自然に避けやすくなります。
完了の定義
完了 の基準にコードスメルチェックを含めます。SonarQube の Quality Gate、ESLint のエラーなしなどを義務化することで、臭いの蓄積を防ぎます。
臭いの教育
新人や経験の浅い開発者に臭いを教えるのは、ソフトウェアエンジニアリング教育の重要な部分です。
教育的アプローチ
- 悪い例と良い例を対比して見せる
- 段階的にリファクタリングして変化を体感
- 過去の自分のコードを臭いの観点で振り返る
- ペアレビューでベテランから学ぶ
- リファクタリング演習(Kata)
推奨教材
- Robert C. Martin,
Clean Code: 章ごとにリファクタリングのケーススタディ - Martin Fowler,
Refactoring (2nd ed.): 臭いとリファクタリングの対応 - Joshua Kerievsky,
Refactoring to Patterns: パターンへのリファクタリング - Kent Beck,
Implementation Patterns: 細かな実装の判断 - Adam Tornhill,
Your Code as a Crime Scene: ホットスポット分析
これらの書籍は、臭いを教える教材として優れています。
臭いと現代の言語進化
近年の言語進化は、臭いを構造的に防ぐ機能を増やしています。
| 言語 | 機能 | 防げる臭い |
|---|---|---|
| Java 14+ | record | Data Class、Long Parameter List |
| Java 17+ | sealed class | Refused Bequest、Switch Statements |
| Kotlin | data class | Data Class、Long Parameter List |
| Kotlin | sealed class | Switch Statements |
| Rust | newtype | Primitive Obsession |
| Rust | Ownership | Mutable Data、Inappropriate Intimacy |
| TypeScript | Discriminated Union | Switch Statements |
| Python | dataclass | Data Class |
| Swift | enum with associated values | Switch Statements |
| C# 9+ | record | Data Class |
| Scala 3 | enum、type class | Switch Statements |
| OCaml/F# | パターンマッチ | Switch Statements |
新しい言語機能を学ぶことで、古い臭いを構造的に避けられます。
臭いと AI コーディング
LLMによるコード生成・補完が普及する中、臭いとの関係も変わりつつあります。
LLM が生成するコードの傾向:
- 安全寄りで保守的(既存パターンに従う)
- 訓練データに含まれる臭いも再生する
- 最新ベストプラクティスへの追随が遅い
- 文脈の制約(プロジェクトの規約)を見落とす
利点:
- 一般的な臭いの修正提案ができる
- リファクタリング候補の発見
- ボイラープレートの削減
注意点:
- LLMの提案を盲目的に採用すると Cargo Cult Programming になる
- 大量のコードを生成すると、臭いの量も増える
- レビューと検証は人間の責任
近年は、LLM がコードレビューで臭いを指摘するツール(GitHub Copilot Code Review、CodeRabbit など)も登場しています。これらは静的解析ツールの自然言語版として位置付けられます。
臭いを学ぶための演習
臭いの嗅覚は、座学ではなく実践で磨かれます。
演習1: 過去のコードレビュー
- 自分が書いたコードを6ヶ月後にレビューする
- どこに臭いがあるかを言語化する
- リファクタリングしてみる
演習2: オープンソース
- 大規模なオープンソースの一部を読む
- 臭いを見つけて issue を立てる(または PR を出す)
- 受け入れられるかどうかでフィードバックを得る
演習3: コードカタ
Gilded Rose Kata: レガシーコードのリファクタリング演習Bowling Kata: TDDとリファクタリングの定番Yatzy Kata: Switch Statementsとの戦い
演習4: ペアレビュー
- ベテランとペアでコードを読む
- ベテランが嗅ぎ取る臭いを言語化してもらう
- 自分でも同じように嗅ぎ取れるようになる練習
演習5: 比較
- 同じ問題の解を複数の実装で読み比べる
- どちらが臭うかを判定する
- 理由を言語化する
臭いと心理
臭いを嗅ぎ取る感覚は、技術スキルだけでなく心理的要因も関わります。
過剰な完璧主義
すべての臭いを直そうとすると、開発が進みません。完璧主義は臭いを見過ぎる害があります。
対処:
- 優先順位付け
- 「これは今は許す」と意識的に決める
- 進化的設計を信じる
自分のコードへの愛着
自分が書いたコードは臭いに気付きにくくなります。
対処:
- 時間を置いてから読み直す
- 他人のレビューを尊重する
- 過去の自分を批判できる成長を目指す
インポスター症候群
「自分は臭いを嗅ぎ取れない」と過小評価する傾向。
対処:
- 小さな臭いから言語化する
- ペアプログラミングで自信をつける
- 学びは継続的なプロセスと割り切る
確証バイアス
「このコードは臭う」と決めつけて、根拠なく批判する傾向。
対処:
- 具体的な改善案を出す
- 反例を考える
- 議論を建設的に進める
臭いと組織文化
組織全体で臭いに敏感になるには、文化的な工夫が必要です。
心理的安全性
「あなたのコードは臭う」と言われても、人格否定と受け取らない関係性が必要です。レビューはコードに対するものであり、人に対するものではない、という共有理解が大切です。
非難しないポストモーテム
障害の根本原因に臭いがあった場合、誰を責めるかではなく、なぜ臭いが残ったかを問います。プロセス、レビュー、教育の改善につなげます。
リファクタリング予算
機能開発の何%かをリファクタリングに割り当てる文化。Boy Scout Rule の制度化です。
知識共有
臭いと対処の事例を、内部 Wiki やランチ&ラーンで共有することで、組織全体の嗅覚を底上げします。
付録A: 臭い早見表
主な臭いとその対応リファクタリングを一覧にします。
| カテゴリ | 臭い | 主なリファクタリング |
|---|---|---|
| 肥大化した臭い | Long Method | Extract Method, Replace Temp with Query |
| 肥大化した臭い | Large Class | Extract Class, Extract Subclass |
| 肥大化した臭い | Long Parameter List | Introduce Parameter Object, Preserve Whole Object |
| 肥大化した臭い | Primitive Obsession | Replace Data Value with Object, Introduce Parameter Object |
| 肥大化した臭い | Data Clumps | Extract Class |
| OOP Abusers | Switch Statements | Replace Conditional with Polymorphism |
| OOP Abusers | Refused Bequest | Push Down Method, Replace Inheritance with Delegation |
| OOP Abusers | 一時フィールド | Extract Class, Replace Method with Method Object |
| OOP Abusers | Alternative Classes | Rename Method, Extract Superclass |
| 変更を妨げる臭い | Divergent Change | Extract Class |
| 変更を妨げる臭い | Shotgun Surgery | Move Method, Inline Class |
| 変更を妨げる臭い | Parallel Inheritance | Move Method, Move Field |
| 不要なもの | Comments | Extract Method, Rename Method |
| 不要なもの | Duplicated Code | Extract Method, Pull Up Method, Form Template Method |
| 不要なもの | Lazy Class | Inline Class, Collapse Hierarchy |
| 不要なもの | Data Class | Move Method, Encapsulate Field |
| 不要なもの | Dead Code | Delete |
| 不要なもの | Speculative Generality | Inline Class, Remove Parameter |
| 結合に関する臭い | Feature Envy | Move Method |
| 結合に関する臭い | Inappropriate Intimacy | Move Method, Hide Delegate |
| 結合に関する臭い | Message Chains | Hide Delegate, Extract Method |
| 結合に関する臭い | Middle Man | Remove Middle Man, Inline Method |
| 第二版追加 | Mysterious Name | Rename |
| 第二版追加 | Global Data | Encapsulate Variable |
| 第二版追加 | Mutable Data | Encapsulate Variable, Split Variable |
| 第二版追加 | Loops | Replace Loop with Pipeline |
| 第二版追加 | Repeated Switches | Replace Conditional with Polymorphism |
| 第二版追加 | Insider Trading | Move Method, Hide Delegate |
付録B: メトリクスと閾値
機械的に検出可能な臭いの目安を提示します。あくまで参考値で、絶対的な基準ではありません。
| メトリクス | 閾値 | 臭い |
|---|---|---|
| メソッドの行数 | > 30行 | Long Method |
| メソッドの行数 | > 100行 | 強い Long Method |
| Cyclomatic Complexity | > 10 | 高複雑度 |
| Cyclomatic Complexity | > 20 | 危険レベル |
| Cognitive Complexity | > 15 | 認知負荷高 |
| クラスの行数 | > 500行 | Large Class |
| クラスのメソッド数 | > 30 | Large Class |
| クラスのフィールド数 | > 10 | Large Class |
| メソッドの引数数 | > 4 | Long Parameter List |
| 継承の深さ | > 5 | Yo-yo Problem の可能性 |
| クラス間結合(CBO) | > 20 | 結合過多 |
| LCOM (Lack of Cohesion) | 高 | 凝集度低い |
| 重複行数 | > 6行 | Duplicated Code |
これらは SonarQube、Code Climate、Codacy などのツールでデフォルト設定として使われています。
付録C: リファクタリングの安全な進め方
臭いを見つけても、いきなり大胆なリファクタリングは危険です。安全な手順を整理します。
ステップ1: 安全網を作る
- 単体テストでカバレッジを確認
- 統合テストで動作を確認
- 必要なら Characterization Tests(既存挙動を固定するテスト)を追加
ステップ2: 小さなステップで進める
- 一度に大きく変えない
- 一回のコミットで一つの目的
- ビルドとテストが常に通る状態を保つ
ステップ3: 機能変更とリファクタリングを分ける
- 同じ PR で機能追加とリファクタリングを混ぜない
- レビューが容易になる
- バグ発生時の原因特定が容易
ステップ4: ブランチ戦略
- 短命ブランチで頻繁にマージ
- Feature flag で段階的デプロイ
- ストラングラーパターンで段階的に置き換え
ステップ5: 監視
- リファクタリング後の本番動作を監視
- エラー率、性能、ビジネス指標を確認
- 問題があれば即ロールバック
Refactoring(2nd ed.) の前半は、こうした安全な進め方を詳細に解説しています。
付録D: 臭いとアーキテクチャ
クラスやメソッドレベルの臭いは、アーキテクチャレベルの問題に繋がります。
| クラスレベルの臭い | アーキテクチャレベルの問題 |
|---|---|
| God Object | レイヤー違反、責務の集中 |
| Inappropriate Intimacy | モジュール境界の崩壊 |
| Shotgun Surgery | 横断的関心事の散乱 |
| Feature Envy | サービス境界の不適切な分割 |
| Lava Flow | 設計判断の不在、ADR欠如 |
| Speculative Generality | 過剰な抽象化レイヤー |
クラス単位の臭いを直すだけでなく、なぜこの臭いが出ているか を組織やアーキテクチャの視点で見直すと、根本対処につながります。
付録E: テストコードの臭い
製品コードと同様、テストコードにも臭いがあります。Gerard Meszaros の xUnit Test Patterns で詳述されています。
テストの臭いの例
- Eager Test: 一つのテストで多くの assertion
- Mystery Guest: テストが外部リソース(DB、ファイル)に依存
- Test Code Duplication: テストヘルパが整理されていない
- Conditional Test Logic: テスト内に if 文
- Excessive Setup: setUp が肥大化
- Indirect Test: テストが本物のロジックではなく中間層をテスト
- Slow Test: 遅すぎてCIで回せない
- Fragile Test: 内部実装の変更で壊れる
- Test Hides Failure: 例外を隠してパスする
対処
- Arrange/Act/Assert パターンで構造化
- Test Double(モック、スタブ、フェイク)の適切な使い分け
- テストデータビルダー(Builder Pattern)
- テストの単一責任原則
テストコードの品質は、製品コードと同じくらい重要です。臭いを意識して書くことで、保守可能なテストスイートを育てられます。
付録F: コミットメッセージとレビューの臭い
コードだけでなく、コミットメッセージやレビューにも臭いがあります。
コミットメッセージの臭い
WIP、fix、updateのような無意味なメッセージ- 多数の変更を一つのコミットに詰め込む
- 変更理由が書かれていない
- 「なぜ」より「何を」だけ書かれている
プルリクエストの臭い
- 数千行の変更(レビュー不可能)
- 機能変更とリファクタリングが混在
- 説明文が空
- テストが付随していない
- スクリーンショットや動作確認の証拠がない
レビューコメントの臭い
- 「これは良くない」だけで具体策がない
- 個人攻撃のニュアンス
- スタイルだけで本質的な議論がない
- LGTM の濫用(実際には読んでいない)
これらの臭いはコード品質に直接影響します。Conventional Commits、Conventional Comments といった規約が、改善の助けになります。
付録G: ドキュメントの臭い
コードのドキュメントにも臭いがあります。
- 自動生成されただけで意味のない doc
- コードの言い換えだけのコメント
- 「TODO」が永遠に残る
- 廃止された情報の残骸
- どこにあるか分からない情報
- 検索しにくい構造
対処:
- ドキュメントの目的を明確にする(API、設計、運用、チュートリアル)
- Diátaxis Framework のような分類を活用
- ドキュメント生成と運用を CI に組み込む
- 古い情報を定期的に整理
Documentation Writing の付録(同じappendices内)も参照してください。
付録H: 設定の臭い
設定ファイル(YAML、TOML、JSON、HCL、HOCON)にも臭いがあります。
- 同じ値が複数箇所に書かれている
- 命名規則が一貫していない
- コメントが古い
- 環境差を分岐ではなく重複で表現
- 必須項目とオプション項目が区別されない
- 検証されない(型、範囲、形式)
対処:
- 設定スキーマを定義する(JSON Schema など)
- 環境ごとの差分は overlay で表現
- DRY を意識する
- 起動時バリデーション
設定が複雑化すると、Inner Platform Effect(アンチパターン)に向かいます。
付録I: 言語ごとのおすすめツール
各言語で推奨されるリンター・解析ツールを再掲します。
| 言語 | 推奨ツール |
|---|---|
| Java | SpotBugs, PMD, Checkstyle, SonarLint |
| Kotlin | Detekt, ktlint |
| Python | Pylint, Flake8, Ruff, mypy, Black |
| JavaScript | ESLint, Prettier, Biome |
| TypeScript | ESLint, tsc strict, Biome |
| Go | go vet, staticcheck, golangci-lint, gofmt |
| Rust | Clippy, rustfmt |
| Ruby | RuboCop, Reek, Brakeman |
| C/C++ | Cppcheck, Clang-Tidy, Address Sanitizer |
| C# | Roslyn analyzers, SonarLint, StyleCop |
| Scala | Scalafmt, Scalafix, WartRemover |
| Swift | SwiftLint, SwiftFormat |
| PHP | PHPStan, Psalm, PHP_CodeSniffer |
これらを CI で実行し、定期的にレポートを確認することで、臭いの蓄積を抑えられます。
付録J: 臭いの哲学
最後に、コードの臭いという概念の哲学的側面を考察します。
臭いは主観的か客観的か
臭いは部分的に主観的です。「Long Method」と感じる行数は人やチームで変わります。一方、Cyclomatic Complexity のような数値で客観化できる部分もあります。
主観と客観の境界を意識することで、議論を建設的に進められます。「私はこれを長いと感じるが、君はどうか」という対話が、感性を共有していく過程です。
臭いと美
優れたコードには美的感覚があります。簡潔さ、整然さ、説明可能性、対称性、最小驚きの原則。これらは臭いの逆として表現できます。
Beautiful Code(Andy Oram, Greg Wilson 編、2007)は、優れたエンジニアたちが「美しいコード」を語る本です。臭いを学ぶことと美しいコードを書くことは、表裏一体の修練です。
臭いと禅
禅における「捨てること」と、コード臭いにおける「削ること」は通じます。Lazy Class、Speculative Generality、Dead Code、Comments の多くは、今は要らない ものです。捨てることに勇気を持つことが、コードを軽やかにします。
不要なものを削ることは、新しいものを作るより難しいスキルです。
臭いと進化
コードベースは生き物のように進化します。Lehman’s Laws が示すように、放置すれば複雑さが増し、品質が低下します。臭いはその進化の中で生まれる兆候であり、適切な「剪定」が長期の健康を保ちます。
剪定(リファクタリング)は破壊ではなく、生命を保つための営みです。
臭いと自分
自分のコードに対しても臭いを嗅ぎ分けるには、自分への愛着を一度脇に置く必要があります。「過去の自分が書いたコードに臭いがある」と認められることは、成長の証です。
逆に、すべての過去のコードを臭いとして否定するのも違います。書いた時点での最善であった可能性を尊重しつつ、今の視点で改善を考える、という両立が成熟です。
付録K: ドメイン別の臭い
製品コード以外のドメインにも臭いがあります。
データベースの臭い
- Schema Anti-patterns(Bill Karwin の
SQL Antipatterns由来) - 多次元属性: 一つのカラムに複数の値(カンマ区切り、JSON 内の配列)
- Naive Trees: 親IDだけで階層を表現、再帰クエリが必要
- ID Required: すべてのテーブルに無意味な
idカラム - Keyless Entry: 外部キー制約なし、整合性を失う
- Entity-Attribute-Value: 動的スキーマで Inner Platform Effect
- Polymorphic Association: 一つの外部キーが複数のテーブルを指す
- Multicolumn Attributes:
email1,email2,email3のような連番カラム - Metadata Tribbles: テーブルが年月で増殖(
orders_2024_01,orders_2024_02…) - Reading the Psychic: NULL の意味が文脈依存
リファクタリング:
- 関連テーブルを使う(1対多なら子テーブル、多対多なら中間テーブル)
- 適切な正規化(3NF まで)
- 必要に応じて非正規化(性能のため)
REST API の臭い
- Verbs in URL:
/getUser/123のような動詞含み - 動詞の濫用: PATCH と PUT を区別しない
- 巨大なレスポンス: ページネーションなし
- N+1: クライアントがループでAPIを叩く
- 隠れた状態: 同じURLが文脈で違うレスポンス
- バージョン管理なし
- エラー処理が不統一
- HATEOAS の欠如
リファクタリング:
- 名詞ベースの URL(
/users/123) - 適切な HTTP メソッド
- ページネーション、フィルタ、ソート
- バージョニング戦略(URL、ヘッダ、コンテンツ)
- 統一エラーフォーマット(RFC 7807 Problem Details)
GraphQL の臭い
- N+1 Query Problem: Resolver でDB を逐次呼ぶ
- Over-fetching the Server: クライアントが過剰なデータを要求
- 深すぎるネスト: 攻撃ベクトルになる
- Authorization in Resolvers: 全 Resolver で同じ権限チェックを繰り返す
- 巨大なスキーマ: 一つの GraphQL に全機能が集まる
リファクタリング:
- DataLoader で N+1 解消
- クエリ深さ制限
- スキーマ分割(Federation)
- 認可をミドルウェア化
フロントエンドの臭い
- Prop Drilling: 深いコンポーネント階層をプロパティが貫通
- 巨大コンポーネント: 1000行のReactコンポーネント
- State 散乱: 複数の useState が相互依存
- スタイル汚染: グローバル CSS が予期しない場所に効く
- アクセシビリティ無視: ARIA 属性なし、コントラスト不足
- バンドルサイズ肥大: 大量の依存関係
リファクタリング:
- Context API、状態管理ライブラリで Prop Drilling 解消
- コンポーネント分割(atomic design など)
- useReducer で複雑な状態管理
- CSS Modules、styled-components、Tailwind で局所化
- a11y ツール(axe、Lighthouse)でチェック
- バンドル分析(webpack-bundle-analyzer)
ML パイプラインの臭い
- データ漏洩(Data Leakage): 訓練・検証データが混入
- 特徴量の Hard Coding: 列名や閾値がコード内
- モデルの Data Class: モデル定義だけで、推論や評価が別箇所
- Reproducibility の欠如: シード値、データバージョンが固定されない
- 評価指標の Goodhart: ベンチマーク指標が真の品質と乖離
- Magic Hyperparameters: ハイパーパラメータの選択理由が不明
- Pipeline Spaghetti: ETLジョブの依存関係が追跡不能
リファクタリング:
- データバージョニング(DVC、LakeFS)
- 特徴量ストア(Feast、Tecton)
- 実験管理(MLflow、Weights & Biases)
- Reproducible なシード設定
- 多面的な評価指標
インフラ as Code の臭い
- Hard Coding: クラウドリソースの名前、リージョンが直書き
- 巨大モジュール: 一つの Terraform モジュールに全リソース
- 過剰汎用化: 使われない条件分岐
- 状態管理: tfstate が複数のチームで競合
- バージョン管理: モジュールのバージョンが固定されない
- ドリフト: 手動変更が IaC と乖離
リファクタリング:
- 環境ごとの変数ファイル
- モジュール分割(VPC、サービス、データベースなど)
- リモート state(S3 + DynamoDB lock)
- バージョンタグでの参照
- ドリフト検出の自動化
CI/CD パイプラインの臭い
- Long Pipeline: 完了まで30分以上
- Flaky Tests: 確率的に失敗するテスト
- Cache 不適切: ビルド毎に依存関係をダウンロード
- Sequential Steps: 並列化できる箇所が直列
- Secret in Logs: ログに秘密情報が漏れる
- 環境差: CI と本番で動作が違う
- 自動ロールバックなし
- 通知過剰: アラート疲れ
リファクタリング:
- ステップを並列化
- Flaky Test を隔離・修正
- 適切なキャッシュ戦略
- 環境を本番と一致させる(コンテナ)
- ロールバック自動化
- 通知の優先度設定
付録L: 言語別の臭いガイド
Python の臭い
# Mutable Default Arguments
def add_item(item, items=[]): # 危険
items.append(item)
return items
# 改善
def add_item(item, items=None):
if items is None:
items = []
items.append(item)
return items
# Comparing with == None
if value == None: # 動くが慣習に反する
pass
# 改善
if value is None:
pass
# Bare except
try:
do_something()
except: # KeyboardInterrupt も捕まえる
pass
# 改善
try:
do_something()
except SpecificException as e:
handle(e)
# Lambda の濫用
sorted(items, key=lambda x: x.name) # OK
filter(lambda x: x.active and x.amount > 100, items) # 読みにくい
# 改善
def is_relevant(x):
return x.active and x.amount > 100
filter(is_relevant, items)
Java の臭い
// Telescoping Constructor
public class Pizza {
public Pizza(int size) { ... }
public Pizza(int size, boolean cheese) { ... }
public Pizza(int size, boolean cheese, boolean pepperoni) { ... }
public Pizza(int size, boolean cheese, boolean pepperoni, boolean bacon) { ... }
}
// 改善: Builder Pattern または Record
public record Pizza(int size, boolean cheese, boolean pepperoni, boolean bacon) {}
// Null チェック地獄
if (user != null && user.getAddress() != null && user.getAddress().getCity() != null) {
String city = user.getAddress().getCity();
}
// 改善
String city = Optional.ofNullable(user)
.map(User::getAddress)
.map(Address::getCity)
.orElse("Unknown");
// Exception 握りつぶし
try {
riskyOp();
} catch (Exception e) {
// ignore
}
// 改善: ログするか、再スロー
try {
riskyOp();
} catch (Exception e) {
log.error("Operation failed", e);
throw new RuntimeException("Operation failed", e);
}
JavaScript / TypeScript の臭い
// Callback Hell
fetchUser(id, (err, user) => {
if (err) return cb(err);
fetchOrders(user.id, (err, orders) => {
if (err) return cb(err);
fetchItems(orders[0].id, (err, items) => {
if (err) return cb(err);
cb(null, items);
});
});
});
// 改善: async/await
async function getItems(id) {
const user = await fetchUser(id);
const orders = await fetchOrders(user.id);
return await fetchItems(orders[0].id);
}
// any の濫用
function process(data: any): any {
return data.value;
}
// 改善
interface Data {
value: string;
}
function process(data: Data): string {
return data.value;
}
// 型アサーション濫用
const user = response.data as User; // 検証なし
// 改善
function isUser(data: unknown): data is User {
return typeof data === 'object' && data !== null && 'id' in data && 'name' in data;
}
const user = isUser(response.data) ? response.data : null;
Go の臭い
// エラー処理の握りつぶし
result, _ := riskyOp() // _ で捨てる
// 改善
result, err := riskyOp()
if err != nil {
return fmt.Errorf("riskyOp: %w", err)
}
// interface{} の濫用
func process(data interface{}) {
// 何でも受け付けるが、何も保証しない
}
// 改善: 型パラメータ(Go 1.18+)
func process[T any](data T) {
// 型パラメータ T で意図を表現
}
// 巨大な struct
type Server struct {
DB *Database
Logger *Logger
Config *Config
AuthService *AuthService
BillingService *BillingService
EmailService *EmailService
NotificationService *NotificationService
// ... 30個以上
}
// 改善: 関連サービスをグループ化
type Services struct {
Auth *AuthService
Billing *BillingService
// ...
}
type Server struct {
DB *Database
Logger *Logger
Config *Config
Services *Services
}
Rust の臭い
// unwrap の濫用
let value = some_option.unwrap(); // None なら panic
// 改善
let value = some_option.ok_or(MyError::Missing)?;
// clone の濫用
let owned_data = data.clone(); // 性能を犠牲に
// 改善: ライフタイムや参照を活用
fn process(data: &Data) -> Result<...> { ... }
// 過剰な lifetime 注釈
fn process<'a, 'b>(x: &'a str, y: &'b str) -> &'a str { ... }
// 改善: lifetime elision
fn process(x: &str, y: &str) -> &str { x } // コンパイラが推論
Ruby の臭い
# メタプログラミング濫用
class User
[:name, :email, :age].each do |attr|
define_method(attr) { instance_variable_get("@#{attr}") }
end
end
# 改善: シンプルに書く
class User
attr_reader :name, :email, :age
end
# Long method chains
result = User.where(active: true).joins(:orders).group("users.id")
.having("count(orders.id) > 5").order(:name).limit(10).pluck(:name)
# 改善: スコープに分割
class User
scope :active, -> { where(active: true) }
scope :with_many_orders, ->(min) { joins(:orders).group("users.id").having("count(orders.id) > #{min}") }
end
result = User.active.with_many_orders(5).order(:name).limit(10).pluck(:name)
付録M: リファクタリング技法の詳細
主要なリファクタリング技法を詳細に解説します。
Extract Method
最頻出のリファクタリング。長いメソッドや関数の一部を別の関数に切り出します。
# Before
def calculate_invoice(items, discount):
subtotal = 0
for item in items:
if item.discountable:
price = item.price * (1 - discount)
else:
price = item.price
subtotal += price * item.quantity
tax = subtotal * 0.1
return subtotal + tax
# After
def calculate_invoice(items, discount):
subtotal = calculate_subtotal(items, discount)
tax = calculate_tax(subtotal)
return subtotal + tax
def calculate_subtotal(items, discount):
return sum(item_price(item, discount) * item.quantity for item in items)
def item_price(item, discount):
return item.price * (1 - discount) if item.discountable else item.price
def calculate_tax(subtotal):
return subtotal * 0.1
Extract Class
大きなクラスから関連フィールドとメソッドを切り出します。
# Before
class Order:
def __init__(self):
self.customer_name = ""
self.customer_email = ""
self.customer_phone = ""
self.shipping_street = ""
self.shipping_city = ""
self.shipping_zip = ""
self.items = []
# After
class Order:
def __init__(self):
self.customer = Customer()
self.shipping = Address()
self.items = []
class Customer:
def __init__(self):
self.name = ""
self.email = ""
self.phone = ""
class Address:
def __init__(self):
self.street = ""
self.city = ""
self.zip = ""
Replace Conditional with Polymorphism
型コードによる分岐を多態性に置き換えます。
# Before
def calculate_pay(employee):
if employee.type == "engineer":
return employee.base_salary + employee.overtime * 50
elif employee.type == "manager":
return employee.base_salary + employee.bonus
elif employee.type == "salesperson":
return employee.base_salary + employee.commission * 0.1
# After
class Employee:
def calculate_pay(self): ...
class Engineer(Employee):
def calculate_pay(self):
return self.base_salary + self.overtime * 50
class Manager(Employee):
def calculate_pay(self):
return self.base_salary + self.bonus
class Salesperson(Employee):
def calculate_pay(self):
return self.base_salary + self.commission * 0.1
Move Method
メソッドを、それが最も使うクラスに移動します。
# Before(Order が Customer のデータを多く使う)
class Order:
def is_eligible_for_discount(self):
return self.customer.is_premium and self.customer.years_active > 5
class Customer:
is_premium: bool
years_active: int
# After
class Order:
def is_eligible_for_discount(self):
return self.customer.is_eligible_for_loyalty_discount()
class Customer:
def is_eligible_for_loyalty_discount(self):
return self.is_premium and self.years_active > 5
Replace Magic Number with Named Constant
# Before
def calculate_circle_area(r):
return 3.14159 * r * r
# After
PI = 3.14159
def calculate_circle_area(r):
return PI * r * r
# Better(言語の標準ライブラリを使う)
import math
def calculate_circle_area(r):
return math.pi * r * r
Introduce Parameter Object
# Before
def create_user(name, email, age, country, role, plan, currency, lang, tz):
...
# After
@dataclass
class UserCreationRequest:
name: str
email: str
age: int
country: str
role: str
plan: str
currency: str
lang: str
tz: str
def create_user(request: UserCreationRequest):
...
Encapsulate Variable
# Before
class Config:
def __init__(self):
self.api_url = "https://api.example.com"
# 利用側で直接変更可能
config.api_url = "evil"
# After
class Config:
def __init__(self):
self._api_url = "https://api.example.com"
@property
def api_url(self):
return self._api_url
def set_api_url(self, url):
if not url.startswith("https://"):
raise ValueError("must use https")
self._api_url = url
Replace Loop with Pipeline
# Before
result = []
for user in users:
if user.active:
result.append(user.name.upper())
# After (Python のリスト内包表記)
result = [user.name.upper() for user in users if user.active]
# あるいは関数型スタイル
result = list(map(
lambda u: u.name.upper(),
filter(lambda u: u.active, users)
))
// Before
let result = [];
for (const user of users) {
if (user.active) {
result.push(user.name.toUpperCase());
}
}
// After
const result = users
.filter(u => u.active)
.map(u => u.name.toUpperCase());
Inline Method
不要な間接層を取り除きます。
# Before
class Manager:
def is_admin(self):
return self._is_admin_user()
def _is_admin_user(self):
return self.role == "admin"
# After
class Manager:
def is_admin(self):
return self.role == "admin"
Hide Delegate
# Before
class Person:
department: Department
class Department:
manager: Person
# 利用側
manager = john.department.manager # Message Chain
# After
class Person:
department: Department
@property
def manager(self):
return self.department.manager
manager = john.manager # 委譲を隠す
付録N: 臭いをCIで検出する設定例
Python: pyproject.toml
[tool.ruff]
line-length = 100
select = [
"E", # pycodestyle errors
"F", # pyflakes
"C", # mccabe complexity
"W", # pycodestyle warnings
"B", # bugbear
"UP", # pyupgrade
]
ignore = [
"E501", # line too long (handled by formatter)
]
[tool.ruff.mccabe]
max-complexity = 10 # Cyclomatic Complexity threshold
[tool.mypy]
strict = true
warn_unused_ignores = true
TypeScript: .eslintrc.json
{
"rules": {
"complexity": ["error", { "max": 10 }],
"max-lines-per-function": ["error", { "max": 50 }],
"max-params": ["error", 4],
"max-depth": ["error", 4],
"no-magic-numbers": ["error", { "ignore": [0, 1, -1] }],
"@typescript-eslint/no-explicit-any": "error",
"@typescript-eslint/no-unused-vars": "error"
}
}
Java: SonarQube Quality Gate
quality_gate:
- metric: cognitive_complexity
operator: GREATER_THAN
value: 15
- metric: duplicated_lines_density
operator: GREATER_THAN
value: 3
- metric: code_smells
operator: GREATER_THAN
value: 0
on_new_code: true
- metric: coverage
operator: LESS_THAN
value: 80
GitHub Actions
name: Code Quality
on: [push, pull_request]
jobs:
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v5
- name: Install
run: pip install ruff mypy
- name: Lint
run: ruff check .
- name: Type check
run: mypy .
- name: Complexity
run: ruff check --select C90 .
これらを CI で常時動かすことで、臭いの蓄積を抑えられます。
付録O: 臭いの優先度マトリクス
すべての臭いを直すのは現実的でないため、優先度を付ける必要があります。
| 影響範囲 \ 修正コスト | 低 | 中 | 高 |
|---|---|---|---|
| 高(変更頻繁、ビジネスクリティカル) | 即対応 | 計画的に | 戦略的判断 |
| 中(時々変更、重要機能) | 余裕があれば | 機会があれば | 機能変更時に |
| 低(変更稀、補助機能) | 触れたついでに | 後回し | 放置 |
具体例:
- ホットスポットの God Object → 高影響・高コスト → 戦略的にリファクタリング計画
- 設定ファイルの Magic Number → 高影響・低コスト → 即対応
- レポート生成の Long Method → 中影響・中コスト → 機会があれば
- レガシースクリプトの Lava Flow → 低影響・高コスト → 放置
Code as Crime Scene(Adam Tornhill)は、Git の履歴と複雑度を組み合わせて、ホットスポットを可視化する手法を提唱しています。これは優先順位付けの実践的な道具です。
zy Class](https://refactoring.guru/smells/lazy-class)
- Refactoring Guru: Data Class
- Refactoring Guru: Speculative Generality
- Refactoring Guru: Feature Envy
- Refactoring Guru: Inappropriate Intimacy
- Refactoring Guru: Message Chains
- Refactoring Guru: Middle Man
- Sourcemaking: Code Smells
- Martin Fowler, Refactoring (book site)
- Martin Fowler, This class is too large
- Martin Fowler, 「リファクタリング」 (1st & 2nd ed.)
- Kent Beck,
Implementation Patterns - Robert C. Martin,
Clean Code: A Handbook of Agile Software Craftsmanship - Joshua Kerievsky,
Refactoring to Patterns
臭い検出の定量的基準
コードの臭いは主観的ですが、定量的な指標も参考になります。以下は一般的な閾値です。
メトリクスと閾値
| メトリクス | 警告値 | 危険値 | 説明 |
|---|---|---|---|
| メソッド長 | 20-30行 | 50行以上 | 複雑度とカップリング増加 |
| クラス行数 | 200-300行 | 500行以上 | 複数責務の兆候 |
| メソッド数 | 25-30個 | 40個以上 | God Object の可能性 |
| 引数数 | 3-4個 | 5個以上 | パラメータオブジェクト化の候補 |
| サイクロマティック複雑度 | 8-10 | 15以上 | 条件分岐の過剰 |
| ファンイン | 1-2 | 5以上 | 過度な依存 |
| ファンアウト | 3-5 | 10以上 | 責務範囲が広すぎる |
これらの値は目安です。言語、プロジェクト、ドメインによって調整が必要です。
リファクタリング実践例
Martin Fowler の著名な記事 「Refactoring: This class is too large」は、実際のコードベースから抽出した具体例を示しています。ReconciliationIntro という大きなクラスを複数のクラスに分割する過程を通じて、次の原則が強調されます。
- 小さなステップで進める: 大規模リファクタリングは、コミット単位で分割可能な小さな変更に分ける。
- テストを常に実行: リファクタリング後のテストは自動化されたビルドで検証。
- 関連性で分割: フィールドの使用パターンに基づいて、新しいクラスを抽出する。
例えば、ファイル読み込みロジックと報告ロジックが同じクラスに混在していた場合、次のように対処します。
# リファクタリング前: 責務が混在
class ReconciliationIntro:
def load_file(self, path):
... # 100行
def generate_report(self):
... # 100行
def display_ui(self):
... # 100行
# リファクタリング後: 責務を分離
class FileLoader:
def load(self, path):
...
class ReportGenerator:
def generate(self):
...
class UI:
def display(self):
...
この過程では、コードの移動とリネームを分けて実行し、各ステップでテストを通す慎重さが重要です。
臭いの根本原因
Refactoring Guru などのリソースでは、臭いが発生する根本的な理由を分析しています。
コードの臭いが生まれる3つのシナリオ
-
設計段階での過誤:
- クラスの責務を誤って理解
- パターンを過度に適用(YAGNI 違反)
- 委譲 vs 継承の判断ミス
-
段階的な劣化:
- 新機能の追加で既存クラスが膨張
- リファクタリングの延期が負債を増加
- 変更の局所性が失われる
-
レビュー文化の欠落:
- コードレビューで臭いを指摘しない
- 「動けば良い」という姿勢
- パターンやプリンシパルの理解不足
臭いの発展過程
ほとんどの臭いは一夜にして発生しません。段階的に進行します。
初期段階(匂う前)
↓ 変更が積み重なる
軽度な臭い(注意信号)
↓ リファクタリングをしない
中程度の臭い(実装困難、バグ増加)
↓ 放置されて技術的負債化
重度な臭い(大規模リファクタリング必須)
この段階で最適な対応は「軽度な臭い」の段階でのリファクタリングです。
臭いと技術的負債の関係
Kent Beck が名付けた「コードの臭い」は、テクニカルデブト(技術的負債)と密接に関連します。
臭いと負債メトリクス
| 臭い | 負債の種類 | 利息(メンテコスト) | 返済難度 |
|---|---|---|---|
| Long Method | 構造的負債 | 高(複雑度↑) | 低(分割で改善) |
| Large Class | 構造的負債 | 高(変更範囲↑) | 中(設計見直し) |
| Duplicated Code | 保守的負債 | 高(修正漏れ) | 低(統合で改善) |
| Switch Statements | 設計的負債 | 中(型追加時) | 中(多態化) |
| Dead Code | 認知的負債 | 中(理解コスト) | 低(削除) |
技術的負債の計算式は Ward Cunningham が提唱していますが、臭いはその前兆を示す指標です。
技術的負債とは、長期的なシステムの品質劣化に対して現在支払っている「利息」である。
臭いを検出して早期に対処することは、この利息を最小化する戦略です。
臭いの優先度付けマトリクス
技術的負債と同様に、臭いの対応優先度を体系的に決定できます。
影響度と修正コストの関係
優先度 HIGH(即座に対応)
影響範囲が広く、修正コストが低い
例: Magic Number の廃止、Duplicated Code の統合
優先度 MEDIUM(次回変更時に対応)
影響範囲か修正コストのどちらかが高い
例: Long Parameter List、Feature Envy
優先度 LOW(戦略的判断で対応)
影響範囲が狭く、修正コストが高い
例: Speculative Generality(将来機能が来たら)、レガシー部分の Lazy Class
また「Code as Crime Scene」(Adam Tornhill)では、Git の履歴とメトリクスを組み合わせて、変更頻度が高く複雑なホットスポットを検出し、優先順位付けの根拠にしています。
言語機能の進化と臭いの減少
現代のプログラミング言語は、古典的な臭いを減らすための機能を積極的に導入しています。
言語機能による臭い対策
| 言語機能 | 減少する臭い | 例 |
|---|---|---|
| data class / record | Primitive Obsession, Data Class | Java record, Python @dataclass |
| sealed class / union | Switch Statements | Java sealed class, Kotlin sealed class, TypeScript union |
| pattern matching | Repeated Switches | Java switch with pattern matching, Python match |
| nullable types | Null Pointer 問題 | Kotlin nullable, TypeScript strict null |
| immutability | Mutable Data | Rust ownership, Clojure persistent data structures |
| type system | Primitive Obsession | TypeScript strict mode, OCaml algebraic types |
| generics | Code Duplication | Java generics, C# generics, Go generics(1.18+) |
例えば、Java の record(Java 14+)は、ボイラープレートなしに値オブジェクトを定義できます。
// Before: Data Class の臭い
class Money {
private final BigDecimal amount;
private final String currency;
public Money(BigDecimal amount, String currency) {
this.amount = amount;
this.currency = currency;
}
public BigDecimal getAmount() { return amount; }
public String getCurrency() { return currency; }
@Override
public boolean equals(Object o) { ... }
@Override
public int hashCode() { ... }
}
// After: record で簡潔に
record Money(BigDecimal amount, String currency) {}
TypeScript の union type + switch with exhaustiveness check は、Switch Statements と Repeated Switches の臭いを大幅に減らします。
type Role = 'admin' | 'user' | 'guest';
function getPermissions(role: Role): string[] {
switch (role) {
case 'admin': return ['read', 'write', 'delete'];
case 'user': return ['read', 'write'];
case 'guest': return ['read'];
// 漏れがあればコンパイルエラー
}
}
AI コーディングと臭いの関係
LLM(Large Language Model)ベースのコード生成ツール(GitHub Copilot、Claude など)が普及する中、臭いの概念は新たな重要性を帯びています。
AI 生成コードでよく見られる臭い
- Long Method: トークン上限内で複数ロジックを詰め込む傾向
- Comments: AI は説明的なコメントを多く生成
- Duplicated Code: 類似パターンを繰り返し生成
- Magic Numbers: 具体値をハードコード
- Data Class: 単純な Getter/Setter のみ
臭いと AI レビューの組み合わせ
AI 生成コードの品質向上には、臭いの言語化が有効です。
ユーザー: "このメソッドを書いて"
AI: [Long Method の臭いのあるコード生成]
ユーザー: "Long Method になっています。Extract Method してください"
AI: [理解した上で改善]
臭いの語彙があれば、人間と AI の協働開発がより効率的になります。
組織文化と臭いの検出
コードの臭いを検出・対応する能力は、個人スキルだけでなく、組織文化に左右されます。
臭い検出を促進する組織文化
-
レビュー文化:
- コードレビューで臭いを指摘する習慣
- 「これは Long Method では」と名前で共有する
- 対立ではなく改善の議論
-
改善時間の確保:
- スプリント内にリファクタリングタスクを含める
- Boy Scout Rule(訪れた場所を前より良くする)
- 定期的なテクニカルデブト返済の時間
-
ツールの整備:
- SonarQube、Code Climate など自動検出ツール
- CI/CD パイプラインに臭い検出を組み込む
- 静的解析ツールのスコア追跡
-
教育と共有:
- 臭い検出の技術を教える
- 実例を通じたナレッジ共有
- コーディング標準に臭いの話を含める
臭い検出が当たり前の文化を作ることで、長期的な品質向上が実現されます。
まとめ
コードの臭いは、Kent Beck と Martin Fowler が 「リファクタリング」 書籍で整理した、コード上の違和感に対する語彙の集合です。Bloaters(肥大化)、Object-Orientation Abusers(OOPの誤用)、Change Preventers(変更を妨げる構造)、Dispensables(不要なもの)、Couplers(結合の問題)の5カテゴリに分類され、それぞれに対応するリファクタリング技法が用意されています。
臭いは欠陥ではなく、設計上の弱さの仮の指標です。すべてを直す必要はありませんが、コードを観察したときに「何かおかしい」と感じたら、どの臭いに近いかを判断し、対応するリファクタリングを検討するのが定石です。
重要なのは、臭いの名前を覚えることではなく、コードを読みながら違和感を言語化できるようになることです。デザインパターンが「正しい解決策」、プログラミング原則が「設計判断」、ソフトウェアの法則が「観察」、アンチパターンが「失敗の典型」だとすれば、コードの臭いは「リファクタリング候補のシグナル」として、これらと相補的に機能します。
臭いを嗅ぎ分ける感覚は、コードレビュー、ペアプログラミング、定期的なリファクタリングの中で育ちます。ツールで検出できる臭いも多いですが、最終判断は人間の理解と文脈に依存します。臭いを学ぶことは、自分のコードを冷静に評価し、長く保守可能なソフトウェアに育てるための基本的な訓練です。
参考文献
講義・記事
解説・補助
- Refactoring Guru: Lazy Class
- Refactoring Guru: Bloaters
- Refactoring Guru: Change Preventers
- Refactoring Guru: Code Smells
- Refactoring Guru: Comments
- Refactoring Guru: Couplers
- Refactoring Guru: Data Clumps
- Refactoring Guru: Dispensables
- Refactoring Guru: Divergent Change
- Refactoring Guru: Duplicated Code
- Refactoring Guru: Large Class
- Refactoring Guru: Long Method
- Refactoring Guru: Long Parameter List
- Refactoring Guru: Object-Orientation Abusers
- Refactoring Guru: Primitive Obsession
- Refactoring Guru: Refused Bequest
- Refactoring Guru: Shotgun Surgery
- Refactoring Guru: Switch Statements
- Refactoring Guru: 一時フィールド
- Wikipedia: Code smell