code-smell-detector

コードスメル検出スキル

目的

このスキルは、amplihack の開発哲学に反するアンチパターンを特定し、建設的で具体的な修正方法を提示します。徹底したシンプルさ、モジュール設計、無駄のない実装をコードで維持できるようにします。

このスキルを使用する場面

• コードレビュー：マージ前に違反を特定します
• リファクタリング：コードを簡素化し、品質を向上させる機会を見つけます
• 新しいモジュールの作成：開発の早い段階で問題を検出します
• 哲学への準拠：コードが amplihack の原則に沿っていることを確認します
• 学習：パターンに問題がある理由と、その修正方法を理解します
• メンタリング：哲学に沿ったコードパターンをチームメンバーに教えます

中核となる哲学の参照

Amplihack の開発哲学が重視するもの：

• 徹底したシンプルさ：すべての抽象化には、その存在意義が必要です
• モジュール設計（ブロックと接合部）：明確な接続点を持つ、自己完結したモジュール
• 無駄のない実装：スタブやプレースホルダーを使わず、動作するコードだけを実装します
• 単一責任：各モジュール／関数には、明確な役割が1つだけあります

検出されるコードスメル

1. 過剰な抽象化

概要：明確な価値をもたらさない、不要な抽象化レイヤー、汎用基底クラス、インターフェースです。

問題点：「徹底したシンプルさ」に反し、見合うメリットがないまま複雑さを増やします。コードの理解と保守が難しくなります。

要注意の兆候：

• 実装が1つしかない抽象基底クラス
• ほとんど何もしない汎用ヘルパークラス
• 深い継承階層（3階層以上）
• 実装が1つしかないインターフェース
• パラメーターが過剰な関数

例 - 問題あり：

# BAD: Over-abstracted
class DataProcessor(ABC):
    @abstractmethod
    def process(self, data):
        pass

class SimpleDataProcessor(DataProcessor):
    def process(self, data):
        return data * 2

例 - 修正後：

# GOOD: Direct implementation
def process_data(data):
    """Process data by doubling it."""
    return data * 2

検出チェックリスト：

• 具象実装が1～2個しかない抽象クラス
• 状態をカプセル化しない汎用ユーティリティクラス
• 2階層を超える型階層
• 1つの問題だけを解決するミックスイン

修正方針：

1. 抽象化が解決する問題を特定します
2. 今、本当に複数の実装が必要かを確認します
3. 抽象化を削除し、直接実装を使います
4. 後で複数の実装が必要になったら、その時点でリファクタリングします
5. 原則：将来に備えた過剰な設計を避けます

---

2. 複雑な継承

概要：コードの流れが分かりにくくなる、深い継承チェーン、多重継承、または複雑なクラス階層です。

問題点：コードを追いにくくし、密結合を生み、シンプルさの原則に反します。誰が何をするのかが不明瞭になります。

要注意の兆候：

• 継承が3階層以上（GrandparentClass -> ParentClass -> ChildClass）
• インターフェースではないクラスからの多重継承
• コードの再利用にコンポジションではなく継承を使用している
• 複数の階層にわたってメソッドをオーバーライドしている
• 横断的関心事のための「ミックスイン」クラス

例 - 問題あり：

# BAD: Complex inheritance
class Entity:
    def save(self): pass
    def load(self): pass

class TimestampedEntity(Entity):
    def add_timestamp(self): pass

class AuditableEntity(TimestampedEntity):
    def audit_log(self): pass

class User(AuditableEntity):
    def authenticate(self): pass

例 - 修正後：

# GOOD: Composition over inheritance
class User:
    def __init__(self, storage, timestamp_service, audit_log):
        self.storage = storage
        self.timestamps = timestamp_service
        self.audit = audit_log

    def save(self):
        self.storage.save(self)
        self.timestamps.record()
        self.audit.log("saved user")

検出チェックリスト：

• 継承の深さが2階層を超えている
• 具象クラスからの多重継承
• 複数の継承階層でメソッドがオーバーライドされている
• コードの再利用がない継承階層

修正方針：

1. 継承ではなくコンポジションを使用します
2. サービスをコンストラクター引数として渡します
3. 各クラスがそれぞれの責任を担います
4. テスト、理解、変更が容易になります

---

3. 大きな関数（50 行超）

内容：多くの処理を詰め込みすぎており、理解、テスト、変更が難しい関数です。

問題点：単一責任の原則に違反し、テストを難しくし、バグが発生する範囲を広げ、コードの再利用性を低下させます。

要注意の兆候：

• 50 行を超えるコードを含む関数
• インデントの階層が複数ある（if/for のネストが 3 階層以上）
• 引数が 5 個以上ある関数
• すべてを確認するのにスクロールが必要な関数
• 名前を付けにくい複雑なロジック

例 - 問題あり：

# BAD: Large function doing multiple things
def process_user_data(user_dict, validate=True, save=True, notify=True, log=True):
    if validate:
        if not user_dict.get('email'):
            raise ValueError("Email required")
        if not '@' in user_dict['email']:
            raise ValueError("Invalid email")

    user = User(
        name=user_dict['name'],
        email=user_dict['email'],
        phone=user_dict['phone']
    )

    if save:
        db.save(user)

    if notify:
        email_service.send(user.email, "Welcome!")

    if log:
        logger.info(f"User {user.name} created")

    # ... 30+ more lines of mixed concerns
    return user

例 - 修正済み：

# GOOD: Separated concerns
def validate_user_data(user_dict):
    """Validate user data structure."""
    if not user_dict.get('email'):
        raise ValueError("Email required")
    if '@' not in user_dict['email']:
        raise ValueError("Invalid email")

def create_user(user_dict):
    """Create user object from data."""
    return User(
        name=user_dict['name'],
        email=user_dict['email'],
        phone=user_dict['phone']
    )

def process_user_data(user_dict):
    """Orchestrate user creation workflow."""
    validate_user_data(user_dict)
    user = create_user(user_dict)
    db.save(user)
    email_service.send(user.email, "Welcome!")
    logger.info(f"User {user.name} created")
    return user

検出チェックリスト：

• 関数本体が 50 行を超える
• ネストが 3 階層以上ある
• 関連性のない処理が複数ある
• 簡潔な名前を付けにくい
• 引数が 5 個以上ある

修正方法：

1. 処理ごとにヘルパー関数を切り出します
2. 各関数に、明確で単一の目的を持たせます
3. 小さな関数を組み合わせて、大きなワークフローを構成します
4. 各関数が 1 画面に収まるようにします
5. 簡単に名前を付けられるなら、通常は処理が 1 つに絞られています

---

4. 強い結合

内容：モジュールやクラスが抽象化ではなく具体的な実装に直接依存しており、テストや変更が難しくなっています。

問題点：あるモジュールの変更がほかのモジュールを壊します。個別にテストするのが難しく、モジュール性の原則に違反します。

要注意の兆候：

• 関数内でクラスを直接インスタンス化している（db = Database()）
• 深い属性アクセス（obj.service.repository.data）
• 条件分岐にクラス名がハードコードされている
• あるモジュールから別のモジュールのすべてをインポートしている
• モジュール間に循環依存がある

例 - 問題あり：

# BAD: Tight coupling
class UserService:
    def create_user(self, name, email):
        db = Database()  # Hardcoded dependency
        user = db.save_user(name, email)

        email_service = EmailService()  # Hardcoded dependency
        email_service.send(email, "Welcome!")

        return user

    def get_user(self, user_id):
        db = Database()
        return db.find_user(user_id)

例 - 修正済み：

# GOOD: Loose coupling via dependency injection
class UserService:
    def __init__(self, db, email_service):
        self.db = db
        self.email = email_service

    def create_user(self, name, email):
        user = self.db.save_user(name, email)
        self.email.send(email, "Welcome!")
        return user

    def get_user(self, user_id):
        return self.db.find_user(user_id)

# Usage:
user_service = UserService(db=PostgresDB(), email_service=SMTPService())

検出チェックリスト：

• メソッド内でクラスをインスタンス化している（Service()）
• 属性の連鎖が深い（ドットが 3 個以上）
• クラスへの参照がハードコードされている
• 循環インポートまたは循環依存がある
• ほかのモジュールなしではモジュールをテストできない

修正方法：

1. 依存関係をコンストラクターのパラメーターとして受け取ります
2. 依存性注入を使用します
3. テスト用の代替オブジェクト（モック）を簡単に作成できるようにします
4. コードを変更せずに実装を差し替えられるようにします
5. 各モジュールを個別にテストできるようにします

---

5. __all__ によるエクスポートの欠如（Python）

内容：__all__ を使って公開インターフェースを明示的に定義していない Python モジュールです。

問題点：公開 API と内部実装の区別が不明確です。利用者が非公開の実装詳細をインポートしてしまいます。「スタッド」の概念に反しており、接続点が不明確です。

要注意の兆候：

• __init__.py に __all__ がない
• モジュールが内部関数やクラスを公開している
• 利用者が何をインポートすべきか分からない
• 非公開の名前（_function）にもアクセスできる
• ドキュメントとエクスポート内容が一致していない

例 - 問題あり：

# BAD: No __all__ - unclear public interface
# module/__init__.py
from .core import process_data, _internal_helper
from .utils import validate_input, LOG_LEVEL

# What should users import? All of it? Only some?

例 - 修正済み：

# GOOD: Clear public interface via __all__
# module/__init__.py
from .core import process_data
from .utils import validate_input

__all__ = ['process_data', 'validate_input']

# Users know exactly what's public and what to use

検出チェックリスト：

• __init__.py に __all__ がない
• 内部関数（_ が接頭辞に付いたもの）が公開されている
• 「公開 API」の範囲が不明確
• すべてのインポートがモジュールレベルにある

修正方法：

1. すべての __init__.py に __all__ を追加します
2. 公開する関数やクラスのみを記載します
3. 内部実装には _ を接頭辞として付けます
4. __all__ に合わせてドキュメントを更新します
5. 明確にすることで、利用者が何を使えばよいか分かるようにします

---

分析プロセス

ステップ 1：コード構造を確認する

1. ファイル構成とモジュールの境界を確認します
2. 継承階層を特定します
3. 大きな関数がないか確認します（行数を数えます）
4. __all__ の有無を確認します
5. 強い結合のパターンがないか確認します

ステップ 2：各問題を分析する

問題の可能性がある箇所ごとに、次を行います。

1. 設計思想に反していることを確認します
2. 深刻度（重大／大／小）を評価します
3. 該当する行番号を特定します
4. システムへの影響を記録します

ステップ 3：修正案を作成する

見つかった問題ごとに、次を行います。

1. なぜ問題なのかを明確に説明します
2. 修正前のコードを示します
3. 詳細なコメント付きで修正後のコードを示します
4. 違反している設計原則を説明します
5. 具体的なリファクタリング手順を示します

ステップ 4：レポートを作成する

1. 見つかった問題をすべて列挙します
2. 深刻度と影響に基づいて優先順位を付けます
3. 具体例を含めます
4. 実行可能な修正案を提示します
5. 設計思想に関するドキュメントを参照します

---

検出ルール

ルール 1：抽象基底クラス

確認項目：class X(ABC) に具象実装が 1 つだけある

# BAD pattern detection
- Count implementations of abstract class
- If count <= 2 and not used as interface: FLAG

修正方法：抽象化を取り除き、直接実装を使用します

ルール 2：継承の深さ

確認項目：クラス階層の深さ

# BAD pattern detection
- Follow inheritance chain: class -> parent -> grandparent...
- If depth > 2: FLAG

修正方法：代わりにコンポジションを使用します

ルール 3：関数の行数

確認項目：すべての関数本体

# BAD pattern detection
- Count lines in function (excluding docstring)
- If > 50 lines: FLAG
- If > 3 nesting levels: FLAG

修正方法：ヘルパー関数を抽出します

ルール 4：依存関係のインスタンス化

確認項目：メソッド／関数内でのクラスのインスタンス化

# BAD pattern detection
- Search for "= ServiceName()" inside methods
- If found: FLAG

修正方法：コンストラクターの引数として渡します

ルール 5：all の欠落

確認項目：Python モジュール

# BAD pattern detection
- Look for __all__ definition
- If missing: FLAG
- If __all__ incomplete: FLAG

修正方法：明示的な __all__ を定義します

---

よくあるコードの問題と簡単な修正方法

問題：「ユーティリティクラス」による保持

# BAD
class StringUtils:
    @staticmethod
    def clean(s):
        return s.strip().lower()

修正方法：直接関数を使用します

# GOOD
def clean_string(s):
    return s.strip().lower()

---

問題：「マネージャー」クラス

# BAD
class UserManager:
    def create(self): pass
    def update(self): pass
    def delete(self): pass
    def validate(self): pass
    def email(self): pass

修正方法：役割を絞ったサービスに分割します

# GOOD
class UserService:
    def __init__(self, db, email):
        self.db = db
        self.email = email

    def create(self): pass
    def update(self): pass
    def delete(self): pass

def validate_user(user): pass

---

問題：神クラス的な関数

# BAD - 200 line function doing everything
def process_order(order_data, validate, save, notify, etc...):
    # 200 lines mixing validation, transformation, DB, email, logging

修正方法：小さな関数を組み合わせます

# GOOD
def process_order(order_data):
    validate_order(order_data)
    order = create_order(order_data)
    save_order(order)
    notify_customer(order)
    log_creation(order)

---

問題：壊れやすい継承

# BAD
class Base:
    def work(self): pass
class Middle(Base):
    def work(self):
        return super().work()
class Derived(Middle):
    def work(self):
        return super().work()  # Which work()?

修正方法：明確でテスト可能なコンポジションを使用します

# GOOD
class Worker:
    def __init__(self, validator, transformer):
        self.validator = validator
        self.transformer = transformer

    def work(self, data):
        self.validator.check(data)
        return self.transformer.apply(data)

---

問題：隠れた依存関係

# BAD
def fetch_data(user_id):
    db = Database()  # Where's this coming from?
    return db.query(f"SELECT * FROM users WHERE id={user_id}")

修正方法：依存関係を明示的に注入します

# GOOD
def fetch_data(user_id, db):
    return db.query(f"SELECT * FROM users WHERE id={user_id}")

# Or in a class:
class UserRepository:
    def __init__(self, db):
        self.db = db

    def fetch(self, user_id):
        return self.db.query(f"SELECT * FROM users WHERE id={user_id}")

---

使用例

例 1：新しいモジュールをレビューする

User: Review this new authentication module for code smells.

Claude:
1. Scans all Python files in module
2. Checks for each smell type
3. Finds:
   - Abstract base class with 1 implementation
   - Large 120-line authenticate() function
   - Missing __all__ in __init__.py
4. Provides specific fixes with before/after code
5. Explains philosophy violations

例 2：密結合を特定する

User: Find tight coupling in this user service.

Claude:
1. Traces all dependencies
2. Finds hardcoded Database() instantiation
3. Finds direct EmailService() creation
4. Shows dependency injection fix
5. Includes test example showing why it matters

例 3：継承をシンプルにする

User: This class hierarchy is too complex.

Claude:
1. Maps inheritance tree (finds 4 levels)
2. Shows each level doing what
3. Suggests composition approach
4. Provides before/after refactoring
5. Explains how it aligns with brick philosophy

---

分析チェックリスト

哲学への準拠

• 不要な抽象化がない
• クラス／関数ごとに単一の責任を持つ
• 公開インターフェイス（__all__）が明確
• 依存関係は隠さず注入する
• 継承の深さは 2 階層以下
• 関数は 50 行未満
• デッドコードやスタブがない

コードの品質

• 各関数の役割が 1 つに明確化されている
• ひと目で理解しやすい
• 単独でテストしやすい
• 他の部分を壊さずに変更しやすい
• 責任を反映した明確な命名になっている

モジュール性

• モジュールを個別にテストできる
• 接続点（「スタッド」）が明確
• モジュール間の結合度が低い
• 依存関係が明示されている

---

レビューの成功基準

このスキルを使ったコードレビューでは、次のことを行います。

• 哲学に反するすべての箇所を特定する
• 具体的な行番号を示す
• 修正前／修正後の例を示す
• それぞれが問題である理由を説明する
• 具体的な修正方法を提案する
• テスト戦略を含める
• 哲学に関するドキュメントを参照する
• 重大度に応じて優先順位を付ける
• 建設的で学びにつながる内容にする
• 執筆者が今後より良いコードを書けるよう支援する

---

コード品質ツールとの連携

このスキルを使う場面：

• コードレビュー中（マージ前）
• プルリクエストのコメント
• 新しいモジュールを作成する前
• レガシーコードをリファクタリングするとき
• チームメンバーへの教育
• 設計レビュー会議

相性の良いもの：

• コードレビュープロセス
• モジュール仕様の生成
• リファクタリングのワークフロー
• アーキテクチャに関する議論
• メンタリングと学習

---

リソース

• 哲学：~/.amplihack/.claude/context/PHILOSOPHY.md
• パターン：~/.amplihack/.claude/context/PATTERNS.md
• ブリック哲学：PHILOSOPHY.md の「Modular Architecture for AI」を参照
• Zero-BS：PHILOSOPHY.md の「Zero-BS Implementations」を参照

---

忘れないでください

このスキルは、次の方法でコードの品質を保つのに役立ちます。

1. 問題が技術的負債になる前に発見する
2. 哲学について開発者に伝える
3. コードをシンプルで保守しやすくする
4. 密結合したシステムを防ぐ
5. コードを理解し、変更しやすくする

建設的に活用してください。目的は批判ではなく、学習と改善です。