smell について
smell は、アーキテクチャや設計、コードスメル、計算量を調べる開発者向けのコード分析スキルです。候補をコードの文脈に照らして検証し、確認済みの指摘を Markdown レポートにまとめます。
主な機能
- 01アーキテクチャと依存関係の調査
- 02コードスメルと設計上の問題の検出
- 03アルゴリズムの複雑さの確認
- 04文脈に基づく指摘の検証
- 05日時入り Markdown レポートの生成
活用シーン
- 01コードベース全体や特定モジュールのレビュー
- 02最近の変更における設計上の問題の確認
- 03依存関係や複雑さのホットスポット調査
インストール
コマンドをコピーしてターミナルで実行します。
npx skills add https:/ / github.com/ smallnest/ goal-workflow --skill smellインストール後、Claude Code で/smellを入力して読み込まれたか確認します。
同梱ファイルREADME.mdtest-prompts.json
詳細
Smell — アーキテクチャのバッドスメル検出ツール
コードベースを分析し、ソフトウェアアーキテクチャの原則への違反、アンチパターン、コードの「バッドスメル」、アルゴリズム計算量のホットスポットを見つけます。包括的で実行可能な Markdown レポートを作成します。
ナレッジベース: このスキルには、Martin Fowler/Kent Beck による古典的なコードスメルのカタログ(refactoring.guru では Bloaters、Object-Orientation Abusers、Change Preventers、Dispensables、Couplers として整理)を含む、業界の研究や実践に基づいたアーキテクチャパターン、アンチパターン、コードスメル、アルゴリズム計算量のヒューリスティックが組み込まれています。
作業内容
- 分析範囲を把握します。プロジェクト全体、特定のモジュール、最近の変更など、どの部分を分析するか尋ねます。
find、grep、Agent(Explore サブエージェント)を使ってコードベースをスキャンし、候補となる兆候や証拠を収集します。- 確認済みの指摘として扱う前に、コンテキスト、呼び出し元、履歴、ワークロード、測定結果と照らし合わせて候補を検証します。
- 詳細な Markdown レポートを
tasks/smell-report-[timestamp].mdに保存します。 - 確認済みの指摘の概要と、候補を分けてユーザーに提示します。
ステップ 1:範囲の確認
ユーザーに次のように尋ねます。
What scope should I analyze?
A. Entire project (thorough, may take time)
B. Specific module/directory: [please specify]
C. Only recently changed files (git diff)
D. Only architectural-level issues (skip low-level code smells)
ユーザーが指定しない場合、小規模プロジェクト(ファイル数が 100 未満)ではオプション A、大規模プロジェクトではオプション C をデフォルトとして使用します。
ステップ 2:証拠の収集
Agent を subagent_type: "Explore" に設定して Explore サブエージェントを使用し、コードベースをスキャンしてアーキテクチャパターンやアンチパターンを調べます。複数の調査を並行して実行します。
調査コマンド
証拠を効率的に収集するため、次の調査を並行して実行します。
- プロジェクト構造のスキャン: ディレクトリツリーを把握し、アーキテクチャスタイル(レイヤード、モジュラーモノリス、マイクロサービスなど)を特定します。
- 依存関係の分析: インポート/インクルードのパターンを調べ、循環依存を確認し、結合のホットスポットを特定します。
- モジュール/コンポーネントのスキャン: ゴッドオブジェクト(500 行を超えるファイル)を特定し、凝集度と単一責任の原則への違反を確認します。
- パターンの検出: 既知のアンチパターンの兆候(static cling、サービスロケーターの乱用、リーキー抽象化)を探します。
- テストのスキャン: テストカバレッジのパターン、テストファイルの場所、テストとコードの比率を確認します。
- 命名と明瞭さのスキャン: 誤解を招く名前、汎用的すぎる名前(Manager、Helper、Util)、一貫性のない命名規則を指摘します。
- 計算量のスキャン: アルゴリズム計算量のホットスポット(ネストしたループ、N+1 クエリ、スキャンの繰り返し、ループ内のソート、レンダリングパスでの高コストな再計算)を検出します。
主要なヒューリスティック
ヒューリスティックは候補となる兆候であり、指摘そのものではありません。行数、ネスト、命名、Big-O の一致は、コードの責務、呼び出し元、変更履歴、ワークロード、意図的な制約を踏まえて検証する必要があります。しきい値だけを根拠に重大度を割り当ててはいけません。
| カテゴリ | 臭い | 検出ヒューリスティック |
|---|---|---|
| アーキテクチャ | ビッグ・ボール・オブ・マッド | ディレクトリ構造が明確でない。すべてがルートまたは単一のフラットなフォルダーにある。関心事が分離されていない |
| アーキテクチャ | レイヤー境界の違反 | 内側のレイヤーが外側のレイヤーをインポートしている。ドメイン/コアレイヤーにインフラストラクチャコードがある |
| アーキテクチャ | アーキテクチャの欠如 | src/、lib/、core/ が分離されていない。SQL が UI コードにインラインで記述されている。HTTP ハンドラーとビジネスロジックが混在している |
| アーキテクチャ | 分散モノリス | マイクロサービス間でデータベースを共有している。サービスを独立してデプロイできない |
| アーキテクチャ | 貧血ドメインモデル | モデル/エンティティクラスにゲッター/セッターしかなく、振る舞いがない。すべてのロジックがサービスにある |
| アーキテクチャ | 必要のない CQRS | 単純な CRUD に読み取り/書き込みモデルを分けている。不必要に複雑である |
| アーキテクチャ | 過剰なレイヤー化 | 実質的な価値のない中継コードを追加するレイヤー/階層が過剰にある |
| アーキテクチャ | 過剰な抽象化 | 間接参照/インターフェース/ジェネリクスが多すぎて、コードを追うと迷ってしまう |
| アーキテクチャ | 未来予測型アーキテクチャ | 実現しないかもしれない要件に対する、推測に基づいた柔軟性(未来予測) |
| アーキテクチャ | 技術偏重型アーキテクチャ | 適合するからではなく、新しいという理由で、目新しい/実績のない技術を本番環境に採用している |
| アーキテクチャ | 過剰なアーキテクチャ | 単純な問題に大がかりなアーキテクチャ/技術を投入している |
| アーキテクチャ | クラウド/Visio アーキテクチャ | 実際のコードや実行時の状況から乖離した図 |
| 結合 | 循環依存 | モジュール A が B をインポートし、B が A をインポートしている。インポートグラフの分析で検出できる |
| 結合 | 内容結合 | あるモジュールが別のモジュールの内部/プライベートメンバーに直接アクセスしている |
| 結合 | 共通結合 | グローバル変数/共有された可変状態が過剰にある。シングルトンの乱用 |
| 結合 | スタンプ結合 | 必要なのは一部のフィールドだけなのに、大きなデータ構造を渡している |
| 凝集度 | 神オブジェクト | 1 つのクラス/モジュールが 500 行を超える。パブリックメソッドが 20 個を超える。無関係な関心事を扱っている |
| 凝集度 | ショットガン手術 | 1 つの変更を加えるのに、無関係なモジュールにまたがる 5 個以上のファイルを変更する必要がある |
| 凝集度 | フィーチャーエンヴィ | メソッドが、自身のクラスのメソッドよりも他のクラスのメソッドを多く呼び出している |
| 凝集度 | データクランプ | 同じ 3 個以上のパラメーター群が、複数のメソッドシグネチャに繰り返し現れる |
| 設計 | 漏れのある抽象化 | 実装の詳細(DB クエリ、HTTP 呼び出し)がインターフェースを通じて露出している |
| 設計 | 静的メソッドへの固執 | 静的メソッドを過剰に使用している。テストを妨げる静的状態がある |
| 設計 | サービスロケーターの乱用 | 適切なコンストラクターインジェクションの代わりに、DI コンテナーをあちこちに渡している |
| 設計 | SOLID 原則違反 | SRP 違反、OCP 違反(型に対する switch/if-else の連鎖)、ISP 違反(肥大化したインターフェース) |
| 設計 | switch 文 | 型コードに対する同じ switch/if-else の連鎖が複数の場所にある。ポリモーフィズムを使うべき |
| 設計 | 継承の拒否 | サブクラスが使用しないメソッド/フィールドを継承している。または、それらをオーバーライドして例外をスローする/何もしないようにしている |
| 設計 | インターフェースの異なる代替クラス | 同じ処理を行う 2 つのクラスで、メソッド名が異なる |
| 設計 | 並行継承階層 | 一方の階層にサブクラスを作成すると、もう一方の階層にも対応するサブクラスが必要になる |
| 設計 | 推測に基づく一般化 | 「将来必要になるかもしれない」という理由で、使われていない抽象クラス、フック、パラメーター、ジェネリクスがある(YAGNI) |
| 設計 | 不完全なライブラリークラス | 必要なメソッドがないため、サードパーティーのクラスをラップ/修正している |
| 凝集度 | 発散する変更 | 1 つのモジュールが多くの無関係な理由で変更される(ショットガン手術の逆) |
| 凝集度 | データクラス | フィールドとゲッター/セッターしかなく、振る舞いのないクラス(貧血データバッグ) |
| 凝集度 | 怠惰なクラス | 存在を正当化するほどの処理を行わないクラス/モジュール |
| 結合 | 不適切な親密さ | 2 つのクラスが互いのプライベート/内部部分に過剰にアクセスしている |
| 結合 | メッセージチェーン | 長い呼び出しチェーン a.getB().getC().getD()(デメテルの法則違反) |
| 結合 | ミドルマン | 他のクラスへのすべての呼び出しを中継するだけのクラス |
| コード | 一時フィールド | 特定の状況でのみ設定/使用され、それ以外では空のインスタンスフィールド |
| コード | コードの重複 | 同一/類似のロジックが 3 か所以上にある。コピー&ペーストのパターン |
| コード | 長いメソッド | 50 行を超えるメソッド。深いネスト(3 階層を超える) |
| コード | 長いパラメーターリスト | パラメーターが 4 個を超えるメソッド |
| コード | プリミティブ執着 | ドメイン型の代わりに文字列/整数を使っている(例:Email 型の代わりに string email を使う) |
| コード | マジックナンバー/文字列 | 名前付き定数を使わずにリテラルをハードコードしている |
| コード | 臭い消しとしてのコメント | リファクタリングの代わりに、悪いコードを説明するコメントが過剰にある |
| コード | デッドコード | 未使用のインポート、到達不能なコード、コメントアウトされたブロック |
| テスト | テストなし | テストカバレッジがゼロのモジュール |
| テスト | テストと実装の結合 | 振る舞いではなく、内部実装の詳細を検証するテスト |
| テスト | 遅いテスト | モックを使わず、実際の I/O、データベース呼び出し、ネットワークリクエストを行うテスト |
| 命名 | 曖昧な名前 | Manager、Handler、Processor、Helper、Util、Service、Data、Info が文脈なしに過剰に使われている |
| 命名 | 命名の不統一 | snake_case と camelCase が混在している。同じ概念に異なるパターンを使っている |
| 可読性 | 深いネスト(アローアンチパターン) | ループ/条件分岐が 3 階層を超えてネストしている。右に流れる「矢印」状の形で追跡しにくい |
| 複雑性 | ネストしたループ(O(n^2)+) | ループの中にループがある。for の中に forEach がある。map の中に map がある。多項式計算量を示唆するネストした反復処理 |
| 複雑性 | 線形スキャンの繰り返し | ループ内に includes()/indexOf()/.find() がある。Set/Map ではなくリストを使った、O(n*m) のメンバーシップ確認 |
| 複雑性 | ループ内のソート | 反復処理内で .sort() または sorted() を呼び出している。一度ソートすれば十分なのに、O(n log n) の処理を繰り返している |
| 複雑性 | N+1 クエリパターン | ループ内でデータベース/API/HTTP を呼び出している。バッチ処理ではなく、反復ごとに fetch/query/execute/findMany を実行している |
| 複雑性 | 描画パスでの再計算 | コンポーネントの描画本体内に .filter().map().sort() のチェーンがある。メモ化せずに高コストな変換を行っている |
| 複雑性 | ペアごとの比較 | すべての要素をほかのすべての要素と比較するネストした反復処理。ソートとツーポインターなら O(n log n) で済むのに O(n^2) になっている |
| 複雑性 | 不必要な再計算 | 同じ高コストな計算をキャッシュせずに繰り返している。useMemo/memo/遅延評価を使っていない |
| 複雑性 | 不適切なデータ構造 | O(1) で検索できる Set/Map の代わりに配列を使っている。キュー/ヒープ/スタックが自然な用途でリストを使っている |
ステップ 3:レポートの生成
問題の特定と根拠
各候補を次の 3 段階で処理します。
- 候補の検出: 静的パターン、ファイル指標、依存関係のスキャンによって候補のみを抽出します。
- コンテキストの検証: 実装と関連する呼び出し元を読み、変更頻度、入力サイズ、実行頻度、フレームワークの制約、生成コード/ベンダーコードに該当するかどうか、既存の緩和策を確認します。
- 問題の確定: 同じ根本原因、影響を受けるパス、障害/変更のシナリオ、修正方針を持つ候補を 1 つの問題にまとめます。
問題の数は、関係する原則の数ではなく、独立した根本原因の数で数えます。主な原則を 1 つ指定し、必要に応じて関連する原則を追加します。SOLID は包括的なラベルです。証拠が特定の観点を支持する場合は、主なラベルとして SRP、OCP、または DIP を使用し、別個の SOLID の問題を重複して作成しないでください。
11 原則の標準マトリックス
| 原則 | 確認できる根拠 | よくある誤検出/制約 |
|---|---|---|
| SOLID | 設計上の問題が複数の SOLID の観点にまたがる、またはより限定的な観点では確実に判断できない | 特定の SRP/OCP/DIP の問題と重複させないでください |
| DRY | 同じビジネスルールや知識を複数箇所で変更する必要がある | 似た構文でも、独立して変更されることが想定されている場合 |
| KISS | 余分なレイヤー、間接化、または仕組みが、明確なメリットなしにコストを増やしている | 実際の複雑さを取り除く小さな抽象化 |
| YAGNI | 使用されていない拡張ポイント、パラメーター、アダプター、または推測に基づく要件 | 既存の境界や変更に必要な、テスト済みの接続点 |
| SRP | 責務や変更履歴に裏付けられた、複数の独立した変更理由がある | ファイルサイズやメソッド数だけに基づくもの |
| 開放/閉鎖原則(OCP) | 既知のバリエーションを追加するたびに、安定した分岐ロジックを繰り返し変更する必要がある | 単純で局所的な条件分岐 |
| 依存関係逆転の原則(DIP) | 上位のポリシーが具体的なインフラストラクチャに直接依存し、置き換えやテストを困難にしている | 安定した実装が 1 つしかないのに、インターフェースを追加すること |
| コンポジション | 継承により、不要な結合、望ましくない振る舞い、または分離できない変化の軸が生じている | 有効な継承関係をすべて機械的に置き換えること |
| 関心の分離 | ビジネスポリシー、I/O、プレゼンテーション、または永続化の関心事が境界を越えて漏れている | 意図的に薄く設計された境界アダプター |
| フェイルファスト | 無効な入力、状態、または依存関係が、離れた処理で失敗するまで伝播する | 意図的な集約、再試行、または遅延検証のセマンティクス |
| 先に測定 | パフォーマンス、スケール、または最適化に関する主張に、ベースラインや代表的なワークロードがない | 測定されたボトルネックではなく、静的な複雑さを報告しているもの |
確定した問題ごとに、根拠の強さ(Measured、Observed、Inferred)と確信度(High、Medium、Low/Candidate)を分けて記録します。根拠の強さは、重大度を示すものではありません。
重大度の基準
- 重大: 正確性、信頼性、セキュリティ、データの一貫性、またはシステム全体への測定済みの影響。通常は高い確信度が必要です。
- 警告: 具体的な保守上または実行時の影響を伴う、明確な影響範囲、または繰り返し発生する変更/実行コスト。
- 提案: 根拠のある、局所的、低頻度、または影響の小さい改善。
- 測定が必要な候補: 影響が不明な静的シグナル。重大度の集計からは除外し、レポートの別セクションに記載します。
次の構成でレポートを作成します。
# Architecture Smell Report
**Project:** [project-name]
**Scope:** [scope description]
**Date:** [date]
**Analyzer:** smell skill (Ducc)
---
## Executive Summary
[2-3 paragraph summary of confirmed findings only: architectural style detected, overall health assessment, and top 3-5 critical issues. Mention candidates separately.]
---
## Architectural Style Detected
[Identify the architectural style: Layered, Modular Monolith, Microservices, Hexagonal, Clean Architecture, or Big Ball of Mud]
### Style Expectations vs. Reality
| Expectation | Reality | Status |
|-------------|---------|--------|
| [e.g., Clear layer separation] | [what was found] | ✅/⚠️/🔴 |
---
## Findings by Category
### 🔴 Critical Issues (Must Fix)
[Issues that fundamentally undermine architecture]
### 🟡 Warnings (Should Fix)
[Issues that degrade maintainability but don't block function]
### 🔵 Suggestions (Nice to Fix)
[Minor improvements that would increase quality]
### Candidates Requiring Measurement
[Static candidates whose runtime impact, change frequency, or workload is not yet established. These do not count toward severity totals.]
---
## Detailed Findings
### Finding #1: [Title]
- **Category:** [Architecture/Coupling/Cohesion/Design/Code/Testing/Naming/Complexity]
- **Severity:** 🔴 Critical / 🟡 Warning / 🔵 Suggestion
- **Anti-Pattern:** [Name of anti-pattern]
- **Location:** [file:line references and relevant callers]
- **Confidence:** [High/Medium]
- **Evidence strength:** [Measured/Observed/Inferred]
- **Failure or change scenario:** [Concrete scenario]
- **Primary principle:** [Most specific principle]
- **Related principles:** [Explanatory only; do not count separately]
- **Description:** [What was found and why it's a problem]
- **Evidence:** [Code, dependency, history, or measurement]
- **Measured/observed impact:** [Reach, frequency, consequence, or baseline]
- **Recommendation:** [Smallest justified refactoring]
- **Verification:** [How to prove behavior and impact]
---
## Dependency Graph Analysis
[Summary of module dependencies, circular dependencies found, coupling hotspots]
---
## Module Health Scorecard
| Module | Lines | God Object Risk | Coupling | Cohesion | Test Coverage | Health |
|--------|-------|----------------|----------|----------|---------------|--------|
| [name] | [N] | [Low/Med/High] | [Low/Med/High] | [Low/Med/High] | [% or N/A] | 🟢/🟡/🔴 |
---
## Smell Distribution
Count only deduplicated confirmed findings by their primary category. Related principles and candidates do not affect these totals.
| Category | Count | Critical | Warning | Suggestion |
|----------|-------|----------|---------|------------|
| Architecture | [N] | [N] | [N] | [N] |
| Coupling | [N] | [N] | [N] | [N] |
| Cohesion | [N] | [N] | [N] | [N] |
| Design | [N] | [N] | [N] | [N] |
| Code | [N] | [N] | [N] | [N] |
| Testing | [N] | [N] | [N] | [N] |
| Naming | [N] | [N] | [N] | [N] |
| Complexity | [N] | [N] | [N] | [N] |
---
## Refactoring Roadmap
Order work by impact, confidence, dependency sequence, and verification cost—not by principle count or smell name. Put only high-confidence, verifiable findings in Immediate Actions; for candidates, recommend the next measurement instead of a rewrite.
### Immediate Actions (This Sprint)
1. [Actionable fix 1]
2. [Actionable fix 2]
### Short-Term (1-3 Months)
1. [Structural improvement 1]
2. [Structural improvement 2]
### Long-Term (3-12 Months)
1. [Architectural transformation 1]
2. [Architectural transformation 2]
---
## Appendix: Anti-Pattern Reference
[A condensed reference of anti-patterns checked, with brief descriptions]
ステップ 4:保存と提示
レポートを tasks/smell-report-[YYYY-MM-DD-HHmm].md に保存し、ユーザーに簡潔な概要を提示します。
アンチパターンのナレッジベース
このセクションでは、このスキルが把握しているアーキテクチャ上のアンチパターンとコードの臭いについて説明します。
アーキテクチャ上のアンチパターン
ビッグボール・オブ・マッド
最も一般的な事実上のアーキテクチャです。無秩序な構造で、認識できるアーキテクチャがない、肥大化したシステムです。特徴:
- 離れた要素間で情報が無秩序に共有される
- 重要な状態がグローバル化または重複している
- 構造が認識できないほど崩れている、または最初から定義されていない
- その場しのぎの修繕が繰り返される(「ダクトテープと針金での応急処置」)
- 要因:時間的なプレッシャー、コスト、経験不足、複雑さ、変更、規模
- 対策: アーキテクチャの境界を定義し、段階的にリファクタリングし、SHEARING LAYERS と KEEP IT WORKING を適用します
分散モノリス
一緒にデプロイする必要があるマイクロサービスです。症状:
- サービスがデータベースを共有している
- サービス呼び出しが同期的に連鎖している
- 変更に連携したデプロイが必要になる
- 対策: データストアを分離し、非同期メッセージングを導入し、境界づけられたコンテキストを徹底します
貧血ドメインモデル
ゲッターとセッターしか持たない(データの入れ物にすぎない)ドメインオブジェクトで、ロジックはすべてサービスにあります。これは次に違反します:
- 「尋ねるな、命じよ」の原則
- DDD のリッチドメインモデルパターン
- 対策: 振る舞いをドメインオブジェクトに移し、ドメインサービスは集約をまたぐ操作にのみ使用します
ゴッドオブジェクト
知識を持ちすぎている、または処理をしすぎているクラスです。特徴:
- 500 行超、または 20 個を超えるパブリックメソッド
- 関連のない関心事を処理する
- 単独でテストするのが難しい
- 単一責任の原則に違反する
- 対策: まとまりのあるメソッド群を専用クラスに切り出します
漏れのある抽象化
実装の詳細を露呈する抽象化です。兆候:
- 実装にちなんだ名前のインターフェースメソッド(例:
SaveToPostgres、FetchFromRedis) - 利用側が実装固有の例外を捕捉する
- 抽象化を通じて設定の詳細が露呈する
- 対策: 利用側の視点からインターフェースを設計し、実装の詳細を隠します
静的メソッドへの固執
静的メソッドや静的状態を過剰に使用することです。問題:
- テストできない(静的呼び出しをモックできない)
- 依存関係が隠れる
- 静的状態によるスレッドセーフ上の問題
- 対策: 依存性注入を使用し、状態を持たない静的メソッドをインスタンスメソッドに変換します
サービスロケーターの乱用
依存性注入の代わりにサービスロケーターを使用することです。問題:
- 依存関係が隠れる(コンストラクターに依存関係が表示されない)
- コンパイル時エラーではなく実行時エラーが発生する
- テストが難しい
- 対策: コンストラクターインジェクションを使用し、コンポジションルートで依存関係を登録します
レイヤー境界の侵害(クリーン/オニオン/ヘキサゴナルアーキテクチャ)
レイヤーアーキテクチャでは:
- クリーンアーキテクチャ: 外側のレイヤー(フレームワーク)が内側のレイヤー(ユースケース、エンティティ)に漏れ込む
- オニオンアーキテクチャ: ドメインの中核にインフラストラクチャの関心事が入り込む
- ヘキサゴナルアーキテクチャ: ビジネスロジックがポートではなく特定のアダプターに結合する
- 対策: 依存関係逆転を適用し、明確なポートインターフェースを定義します
CQRS の過剰な適用
単純な CRUD に CQRS を適用することです。兆候:
- 単純なデータアクセスのために読み取りモデルと書き込みモデルを分ける
- イベントがビジネス上の価値を加えないのにイベントソーシングを使う
- 不要な複雑さ
- 対策: 読み取りモデルと書き込みモデルが実際に異なる場合、または異なるスケーリング要件がある場合にのみ CQRS を使用します
垂直スライスの汚染
垂直スライスアーキテクチャでは:
- スライス間の結合(一方の機能が別の機能を直接呼び出す)
- 共有サービスクラスがスライスの独立性を損なう
- 対策: スライス間の通信にはイベント/メッセージを使用し、必要に応じて単純なロジックを重複させます
ソフトウェアアーキテクチャにおけるよくある間違いトップ 10
過剰設計と設計不足を示す、アーキテクチャレベルのアンチパターンをまとめたものです。共通する問題は、アーキテクチャが実際のニーズや現実から乖離していることです。 反対の極端な状態(アーキテクチャが少なすぎること)も同様に問題の兆候です。
過剰なレイヤー化/多層アーキテクチャ
「レイヤーの上にレイヤーを重ねる」ことです。問題が必要とする以上に層を追加します:
- 各レイヤーが変換も価値の提供もせず、次のレイヤーに呼び出しを転送するだけ
- 単純な読み取りに、4 つのレイヤーにまたがる 6 つ以上のクラスを扱う必要がある
- 対策: 中継するだけのレイヤーを統合し、実質的な責任を担うレイヤーだけを残します
過剰な抽象化
コードを追うのが不可能になるほど抽象化を積み重ねています。
- 単一の実装に対して、インターフェース、ジェネリクス、ファクトリー、間接参照が過剰です
- 多くの段階を追わないと、実際に何が実行されるのか分かりません
- 対処法: 実装が1つだけの抽象化はインライン化し、実際にバリエーションが生じる箇所でのみ抽象化します(三度のルール)
未来を先取りしたアーキテクチャ
誰にも予測できない、想像上の将来の要件に向けて構築されたソリューションです。
- 誰も使わない拡張ポイント、プラグインシステム、設定項目
- 憶測に基づく柔軟性の大半は労力の無駄です。これは「投機的な一般化」や YAGNI と密接に関連しています
- 対処法: 現時点で分かっている要件に合わせて構築し、実際に2つ目のケースが現れたときに柔軟性を追加します
技術愛好家のアーキテクチャ
アーキテクトが気に入ったという理由で、新しい、または流行の技術を本番環境に導入しています。
- 問題に適合するか、またはスケールするかを検証せず、実績のない技術を採用しています
- 安定性よりも流行を追っています
- 対処法: 実際の要件に照らして技術を評価し、実績のあるツールを優先します。採用を決める前にプロトタイプを作成します
過剰なアーキテクチャ
シンプルな問題を、釣り合わない量のアーキテクチャや技術で解決しています。
- 少数のユーザー向けの CRUD アプリにマイクロサービス、イベントソーシング、k8s を導入しています
- 対処法: アーキテクチャの規模を問題の大きさに合わせます(KISS)。シンプルに始め、正当な理由ができたら発展させます
クラウド/Visio アーキテクチャ
見栄えのよい図の中だけに存在し、コードや実行時の実態から切り離された「アーキテクチャ」です。
- 図が実際のデプロイ内容と一致していません。箱や矢印が、対応するコードなしに描かれています
- 対処法: アーキテクチャのドキュメントを実システムに基づいたものにし、実システムと照らして検証します
反対の極端な例について: アーキテクチャがまったくない状態(境界も構造もない)も、同様に問題の兆候です。泥団子とアーキテクチャの欠如を参照してください。過剰設計も設計不足も、どちらも失敗です。
結合度と凝集度に関する問題
循環依存
モジュール A → モジュール B → モジュール A。次の方法で検出できます。
- インポートグラフの分析
- 「初期化前にアクセスできません」というエラー
- 対処法: 共通のインターフェースやモジュールを切り出し、依存関係逆転を適用します
内容結合
あるモジュールが、別のモジュールの内部状態を直接変更しています。兆候:
- モジュール境界を越えたフィールドへの直接アクセス
friend/パッケージプライベートの乱用- 対処法: 公開 API を使用し、内部状態をカプセル化します
共通結合(グローバル状態)
複数のモジュールが、共有されたグローバルな可変状態に依存しています。
- グローバル変数、可変状態を持つシングルトン
- 暗黙的なコンテキスト(例:
CurrentUserの静的プロパティ) - 対処法: パラメーター化し、依存性注入を使用して、状態を明示的にします
スタンプ結合
必要なフィールドが少数であるにもかかわらず、データ構造全体を渡しています。
- 大きな DTO を受け取りながら、1つのフィールドしか使わない関数
- 対処法: 用途に絞ったパラメーターや、より小さなインターフェースを作成します(ISP)
ショットガン・サージェリー
1つの変更に、複数のファイルの修正が必要です。
- フィールドを1つ追加するだけで、異なるモジュールの5つ以上のファイルに手を入れます
- 対処法: 関連する振る舞いを集約し、単一責任の原則を適用します
機能の羨望
あるメソッドが、自身のクラスのメソッドよりも、別のクラスのメソッドを多く使っています。
- メソッド内で、自身のクラスのメソッドをほとんど呼び出さずに、
other.foo()、other.bar()、other.baz()を呼び出しています - 対処法: そのメソッドを、羨望の対象となっているクラスに移動します
データの塊
同じフィールドの組み合わせが、複数の箇所に繰り返し現れています。
(street, city, zip)が5つのメソッドシグネチャに登場しています- 対処法: 値オブジェクトとして切り出します
変更の分散
1つのモジュールやクラスが、互いに無関係な多くの理由で繰り返し変更されています(ショットガン・サージェリーの反対です)。
- 同じクラスで「DB の変更ではこの3つのメソッドを、UI の変更ではあの2つのメソッドをいつも変更している」状態です
- 対処法: 変更の軸に沿ってクラスを分割します(単一責任の原則)
不適切な密結合
2つのクラスが互いの内部実装に過度に依存しています。
- 他のクラスのプライベートフィールドにアクセスする、双方向の参照が密接に結び付いている
- 解決策: メソッドやフィールドを本来属するクラスに移動する、共通クラスを抽出する、または委譲に置き換えます
メッセージチェーン
a.getB().getC().getD().doThing()のような長いナビゲーションチェーンです。
- クライアントがオブジェクトグラフ全体に結合しており、デメテルの法則に違反しています
- 解決策: 委譲を隠します。最初のオブジェクトに、クライアントが必要とするものを返すメソッドを追加します
ミドルマン
ほとんどすべての作業を別のクラスに委譲するクラスです。
- ほとんどのメソッドが呼び出しを転送するだけで、価値を生まない間接性を追加しています
- 解決策: ミドルマンを削除し、クライアントが本来のオブジェクトと直接やり取りするようにします(委譲をインライン化します)
並行する継承階層
一方の階層にサブクラスを追加するたびに、もう一方にも追加する必要があります。
Shape/ShapeRenderer、Employee/EmployeePermissionが同じペースで増えていきます- 解決策: 階層を統合するか、一方の階層をもう一方に対応させるのではなく、参照させます
コードレベルのコードスメル
長いメソッド
- 50行を超えるメソッド(または、言語に応じて適切な長さを設定します)
- ネストが3階層を超えて深くなっている
- 複数の抽象化レベルが混在している
- 解決策: 同じ抽象化レベルでメソッドを抽出し、組み合わせます
長いパラメーターリスト
- パラメーターが4個を超えるメソッド
- 動作を切り替えるブール値フラグ
- 解決策: パラメーターオブジェクトを導入する、メソッドを分割する、フラグ引数を削除します
コードの重複
- 3か所以上にある同一またはほぼ同一のロジック
- わずかな違いだけがあるコピー&ペースト
- 解決策: 共通メソッドを抽出する、テンプレートメソッドパターンまたはStrategyパターンを適用します
プリミティブ執着
ドメイン型ではなくプリミティブ型を使用しています。
- メールアドレス、電話番号、URLに
stringを使用する - 金額、年齢、数量に
intを使用する - 通貨のコンテキストなしで
decimalを使用する - 解決策: 検証と振る舞いを備えた値オブジェクトを作成します
マジックナンバー/マジック文字列
- 説明のないハードコードされたリテラル
if (status == Status.COMPLETED)ではなくif (status == 3)を使用する- 解決策: 名前付き定数または列挙型を抽出します
臭い消しとしてのコメント
- コードの動作を説明するコメント(コード自体が自己説明的であるべきです)
- コメントアウトされたコードブロック
- 解決されないまま蓄積していく「TODO」コメント
- 解決策: コードが明確になるようリファクタリングし、不要なコードを削除し、TODOを課題として管理します
深いネスト(矢印アンチパターン)
ループや条件分岐が深くネストし、コードが右へ右へとずれて「矢印」の形になります。
if { if { for { if { ... } } } }— ある時点でどの条件が成立しているのか追いにくくなります- 1つの関数内でインデントが通常3階層を超えています
- 解決策: ガード節/早期リターンを使用する、ネストしたブロックをメソッドに抽出する、条件を反転する、条件分岐をポリモーフィズムに置き換えます
デッドコード
- 使用されていないインポート、変数、関数
- 到達できない分岐
- バージョン管理に残されたコメントアウト済みコード
- 解決策: 削除します(必要であればGitの履歴から復元できます)
データクラス
フィールドとゲッター/セッターだけで、意味のある振る舞いを持たないクラスです。
- 他のクラスが内部にアクセスして外部から操作する「データの入れ物」です
- クラスレベルでの貧血ドメインモデルに密接に関連しています
- 解決策: データを操作する振る舞いをクラス内に移動します(「尋ねるな、命じよ」)
レイジークラス
存在する理由を正当化できるほどの役割を果たさなくなったクラス/モジュールです。
- リファクタリング後に残ったもの、または十分に発展しなかった抽象化です
- 解決策: 呼び出し元にインライン化するか、階層を統合します
投機的な一般化
将来必要になるかもしれないという想定で追加された抽象化、フック、パラメーター、ジェネリックです。
- 使用されていない抽象基底クラス、未使用のパラメーター、「念のため」の設定
- YAGNIに違反しています
- 解決策: 未使用の抽象化を削除し、実際に2つ目のユースケースが現れたときに追加します
一時フィールド
特定の状況でのみ設定・使用され、それ以外では空のままのインスタンスフィールドです。
- あるアルゴリズムの実行中にのみ値が設定され、それ以外の時間は読者を混乱させるフィールド
- 対処法: フィールドとそれを使用するメソッドを別のクラスに抽出します(クラスの抽出/メソッドオブジェクトの導入)
テストのコードスメル
テストがない
- テストカバレッジがゼロのモジュール
- 単体テストのないビジネスロジック
- 対処法: まず特性テストを書き、その後で振る舞いのテストを追加します
テストと実装の結合
- 内部メソッドの呼び出し、プライベート状態、または実装の詳細を検証するテスト
- 振る舞いに変更がないリファクタリングでも失敗するテスト
- 対処法: パブリック API を通じてテストし、実装ではなく振る舞いを検証します
テスト環境への依存
- モックを使わずにファイルシステム、ネットワーク、データベース、システム時計に依存するテスト
- 非決定的なテスト(不安定なテスト)
- 対処法: テストダブルを使い、環境を制御し、DI を使用します
複雑性のコードスメル(アルゴリズムのアンチパターン)
静的に見つかった複雑性パターンは候補であり、ボトルネックの証拠ではありません。まず測定することを徹底してください。
- 実際の入力サイズ、呼び出し頻度、I/O のレイテンシ、その処理経路が頻繁に実行されるかを確認します。
- プロファイラー、代表的なベンチマーク、クエリログ、トレース、または明示的な処理回数を優先します。
- ワークロードと影響が妥当な場合にのみ、その候補を指摘事項として扱います。
- 修正後、同じワークロードを再測定します。ベースラインがなければ、パフォーマンスが改善したとは主張しないでください。
複雑性の候補ごとに、測定対象、指摘事項となる条件、指摘しない条件を明記してください。以下の Big-O 解析と正しさの確認は維持しつつ、構文だけから深刻度を推測しないでください。
ネストしたループ(O(n^2) 以上)
2 つ以上のループが互いにネストされ、多項式時間の複雑性を生みます。
- 検出方法: 別の
for/whileの中にあるfor/while、forEach/mapの中にあるforEach/map、または別のループを含むループ(深さは問わない) - 影響: 二重ネストでは O(n^2)、三重ネストでは O(n^3) となり、中程度のデータサイズでも処理量が急増します
- 対処法:
- 内側のコレクションのインデックスを Map/Set で構築します → O(n+m)
- ソートと二ポインター方式を使用します → O(n log n)
- 反復処理の前にデータをグループ化/バケット化します
- 区間/範囲の問題にはスイープラインを使用します
- 正しさの確認: 順序は重要ですか?重複キーはありますか?元の処理では最初/最後/すべての一致を選んでいますか?
N+1 クエリパターン
ループ本体内にデータベースクエリ、API 呼び出し、または I/O 操作があります。
- 検出方法: ループ構文内にある
fetch()/axios()/query()/execute()/findMany()/findOne()/findUnique()/select()/where() - 影響: 1 回ではなく 1 + N 回のラウンドトリップが発生し、ネットワークレイテンシが項目数に応じて増加します
- 対処法:
- ID を使って一括取得します:
SELECT * FROM x WHERE id IN (...)を実行し、メモリ内で結合します - ORM のイーガーローディング/
include/preload/DataLoader を使用します - 配列を受け付ける一括 API エンドポイントを使用します
- 認証フィルター、テナント分離、順序、ページネーション、エラーの意味を維持します
- ID を使って一括取得します:
- 正しさの確認: 元の項目ごとの処理では認可されないレコードを取得しないでください。レコードが見つからない場合の動作を維持します
線形探索の繰り返し(インデックスの欠如)
Set/Map なら O(1) で検索できる箇所で、ループ内に線形探索(includes、indexOf、.find、in_array)があります。
- 検出方法: ループ本体内にある
.includes()/.indexOf()/.find()/.findIndex()/in_array()/contains() - 影響: O(n+m) ではなく O(n*m) となり、各反復でコレクション全体を探索します
- 対処法: ループの前に一度だけ
Set(メンバーシップ確認用)またはMap(キーから値への検索用)を構築します - 正しさの確認: Set への変換によって等価性の意味が変わりませんか?JavaScript のオブジェクト同一性と値の等価性、Python のハッシュ可能性を確認します
ループ内ソート
ループ本体内でソートを行い、不要な O(n log n) の処理を繰り返しています。
- 検出方法: あらゆる反復ブロック内にある
.sort()/sorted()/sort() - 影響: O(n log n) ではなく O(k * n log n) になります。ソートが k 回繰り返されます。
- 対処方法:
- ループの外で一度だけソートします
- インクリメンタルなトップ K が必要な場合は、ヒープ(PriorityQueue)を維持します
- 二分探索でソート済みコレクションに挿入します
- 正しさの確認事項: ソート途中の状態は外部から観測可能ですか?比較関数はループ内の状態に依存しますか?
描画パスでの再計算(UI の複雑性)
UI コンポーネントの描画本体内で高コストなデータ変換(filter→map→sort の連鎖)を行い、描画のたびに再計算しています。
- 検出方法: React/Vue/Svelte コンポーネントの関数本体内、JSX/TSX の
function Component()またはconst Component = () =>内にある.filter().map().sort().reduce()の連鎖 - 影響: 入力が変わっていなくても状態変更のたびに再計算され、大規模なコレクションではカクつきが発生します
- 対処方法:
- 適切な依存配列を指定して
useMemo/computed/derivedを使用します - 派生処理をセレクター、ローダー、またはサーバー側に移します
- 長いリストを仮想化します(ウィンドウ化)
- 子コンポーネントの再描画に影響する場合に限り、コールバックとオブジェクトの props を安定させます
- 適切な依存配列を指定して
- 正しさの確認事項: 依存配列には意味上の入力をすべて含める必要があります。ミューテーション可能な入力への変更をメモ化で隠してはいけません
ペアワイズ比較
二重にネストした反復処理ですべての要素をほかのすべての要素と比較しています。
- 検出方法: 同じ、または類似したコレクションを反復し、要素のペアを比較する二重のネストループ
- 影響: ペアの照合、重なりの検出、競合チェック、最近傍探索で O(n^2) になります
- 対処方法:
- ペアや範囲の照合には、ソートと二つのポインターを使用します
- 区間の重なりには、スイープラインを使用します
- 近接性の判定には、空間ハッシュまたはグリッドによるバケット分けを使用します
- 連結性の判定には、Union-Find を使用します
- 正しさの確認事項: 順序の安定性、同値の場合のタイブレーク
不要な再計算(メモ化の欠如)
同じ入力に対する同一の純粋計算が、キャッシュなしで繰り返されています。
- 検出方法: ホットパスで同じ引数を使った同一の関数呼び出し、繰り返される高コストな変換、メモ化されていない再帰呼び出し
- 影響: 線形または多項式の無駄な処理が発生します。特に、再帰的なフィボナッチ数列のようなパターンでは悪影響が大きくなります(メモ化により O(2^n) → O(n))
- 対処方法: 適切な無効化処理を備えたメモ化/キャッシュを追加します。状況に応じて
lru_cache/memoize/useMemoを使用します
不適切なデータ構造
アクセスパターンに対して最適でないデータ構造を使用しています。
- 検出方法:
- 頻繁なメンバーシップテストに Array/List を使用している場合 → Set を使用します
- キーと値の検索に Array/List を使用している場合 → Map/Object を使用します
shift()/pop(0)を使って Array をキューとして使用している場合(デキューごとに O(n))→ 適切な Queue を使用します- Array へのソート済み挿入(挿入ごとに O(n))→ Heap を使用します
- 対処方法: アクセスパターンに合った計算量のデータ構造に置き換えます。
- Set → has/add/delete が O(1)
- Map → get/set が O(1)
- Heap → 優先度に基づく push/pop が O(log n)
- Queue/Deque → enqueue/dequeue が O(1)
指摘しないもの
- 低頻度のパス: 起動時、設定の読み込み時、または N が小さい場合(< 100)にしか実行されない計算量の問題は、修正する価値がほとんどありません
- 意図的なトレードオフ: 明快で読みやすい O(n) のコード。O(n log n) にしても複雑さが増すだけで、測定可能な改善がない場合
- すでに最適化済み: Map/Set をすでに使用している、バッチ読み込みがすでに実装されている、またはメモ化がすでに行われている場合
設計原則への違反
報告の基準として、ステップ 3 の標準マトリクスを使用します。これらの設計チェックは候補の検出を精緻化するものであり、原則ごとに 1 件の指摘を作成するものではありません。
- SOLID 全般: 複数の SOLID に関する問題が同じ根本原因を持つ場合、またはより限定的な観点では確実に判断できない場合にのみ使用します。
- SRP: 変更理由が互いに独立していることを確認します。サイズだけでは不十分です。
- OCP: 実際のバリエーションに対して編集が繰り返されていることを確認します。単純なローカル条件分岐を、推測に基づくポリモーフィズムに置き換えてはいけません。
- LSP: サブタイプは、基底クラスの契約における事前条件、事後条件、不変条件を維持する必要があります。
SOLID/LSPとして報告します。 - ISP: クライアントが使用しないメソッドに依存していることを確認します。
SOLID/ISPとして報告します。 - DIP: 高水準の方針が具体的な仕組みに結合され、その結果、テストや置き換えに支障があることを確認します。実装が 1 つだけであることは、インターフェースを設ける根拠にはなりません。
- DRY: たまたま似ている構文ではなく、共有されている知識を重複排除します。
- KISS と YAGNI: 将来の変更可能性や活用効果が実証されていない抽象化は採用しません。
- コンポジション: 継承によって不要な動作が公開される場合や、独立した変更軸が結び付けられる場合に優先します。機械的なルールとして適用するものではありません。
- 関心の分離: 意図された境界を越えて、方針、プレゼンテーション、永続化、I/O が漏れ出していないか確認します。
- フェイルファスト: 確立されたエラー、再試行、トランザクション、クリーンアップのセマンティクスを維持しつつ、無効な状態をその発生源の近くで検出します。
- 測定してから最適化: 測定されていない最適化の主張は、2 件目の「原則違反」として報告せず、候補として扱います。
オブジェクト指向の濫用(Fowler/refactoring.guru より)
switch 文(型コードによる条件分岐)
型コードや列挙型に基づいて分岐する switch/if-else チェーンが繰り返されている場合:
- 同じ条件分岐の構造が複数箇所で重複している
- 新しい型を追加するたびに、すべての switch の編集が必要になる(OCP 違反)
- 対処法: 条件分岐をポリモーフィズム(Strategy/State)に置き換えるか、型コードをサブクラスに置き換えます
継承の拒否
サブクラスが不要なメソッドやフィールドを継承している場合:
- 継承したメソッドをオーバーライドし、例外をスローする、何もしない、または無関係な処理を行う
- 継承関係が適切でないことを示しています
- 対処法: 未使用のメンバーをサブクラスに移動するか、継承を委譲に置き換えます
インターフェースの異なる代替クラス
同じ役割を担いながら、名前の異なるメソッドを公開する 2 つのクラス:
sort()とarrange()、または置き換え可能なクラス間のgetUser()とfetchUser()- 対処法: インターフェースを統一します(メソッド名を変更するか、共通のスーパークラス/インターフェースを抽出します)
不完全なライブラリクラス
必要なメソッドがなく、変更もできないサードパーティ製/ライブラリのクラス:
- ライブラリの周囲にヘルパー関数やコピー&ペーストしたラッパーが散在している
- 対処法: 外部メソッドを導入するか、アダプター/ローカル拡張クラスでラップします
その他の refactoring.guru のコードの臭いは、上記の各テーマのセクションに記載されています: 変更の発散、データクラス、怠け者クラス、投機的一般化、 一時フィールド、並行継承階層、不適切な親密さ、 メッセージチェーン、仲介者。
エッジケースとフォールバック
| シナリオ | 対応 |
|---|---|
| ユーザーが対象範囲を指定していない | ファイル数が 200 を超えるリポジトリでは最近の変更(git diff)を対象とし、それ以外では全体を分析します |
| プロジェクトに明確なアーキテクチャがない | 根拠とともに「Big Ball of Mud」と報告し、段階的なリファクタリングを推奨します |
| 空のプロジェクト/モノレポ | アーキテクチャ分析にはコードが必要であることを報告し、対象モジュールを指定するようユーザーに求めます |
| サポートされていない言語 | 一般的な構造上の所見を報告し、言語固有のチェックには限界があることを記載します |
| レポートのファイルパスが競合する | ファイル名に -2、-3 などを追加します |
| ユーザーが簡易チェックを希望している | 重大度が「Critical」のスキャンのみ実行し、「Code」と「Naming」カテゴリはスキップします |
| ユーザーがカテゴリを 1 つだけ希望している | そのカテゴリに分析を絞り、その他はスキップします |
レポート出力例
🔍 Architecture Smell Analysis Complete
Project: goal-workflow
Style: Modular Monolith (with some layering violations)
Files Analyzed: 47
Health: 🟡 Fair
Critical: 3 | Warnings: 6 | Suggestions: 9
🔴 Critical Issues:
1. Anemic Domain Model — `models/` classes have only getters/setters,
all logic in `services/`. Violates DDD Rich Domain Model principle.
2. N+1 Query Pattern — `services/order.ts:142` fetches user per order in loop;
should batch-load users by IDs (O(n*m) → O(n+m)).
3. Static Cling — `util/ApiClient.ts` uses all static methods,
making consumer code untestable.
🟡 Warnings:
1. God Object — `services/workflow.ts` at 847 lines handles too many concerns
2. Nested Loop O(n^2) — `analytics.ts:89` pairwise comparison of events;
sort+two-pointer would be O(n log n)
3. Leaky Abstraction — `repositories/user.ts` exposes MongoDB query syntax
4. Duplicated Code — validation logic duplicated across 4 controllers
5. Circular Dependency — `auth` ↔ `user` modules depend on each other
6. Magic Numbers — ~23 hardcoded values without named constants
Full report: tasks/smell-report-2026-05-27-1530.md