Code Smell Detector Skill

Purpose

This skill identifies anti-patterns that violate amplihack's development philosophy and provides constructive, specific fixes. It ensures code maintains ruthless simplicity, modular design, and zero-BS implementations.

When to Use This Skill

• Code review: Identify violations before merging

• Refactoring: Find opportunities to simplify and improve code quality

• New module creation: Catch issues early in development

• Philosophy compliance: Ensure code aligns with amplihack principles

• Learning: Understand why patterns are problematic and how to fix them

• Mentoring: Educate team members on philosophy-aligned code patterns

Core Philosophy Reference

Amplihack Development Philosophy focuses on:

• Ruthless Simplicity: Every abstraction must justify its existence

• Modular Design (Bricks & Studs): Self-contained modules with clear connection points

• Zero-BS Implementation: No stubs, no placeholders, only working code

• Single Responsibility: Each module/function has ONE clear job

Code Smells Detected

1. Over-Abstraction

What It Is: Unnecessary layers of abstraction, generic base classes, or interfaces that don't provide clear value.

Why It's Bad: Violates "ruthless simplicity" - adds complexity without proportional benefit. Makes code harder to understand and maintain.

Red Flags:

• Abstract base classes with only one implementation

• Generic helper classes that do very little

• Deep inheritance hierarchies (3+ levels)

• Interfaces for single implementations

• Over-parameterized functions

Example - SMELL:

BAD: Over-abstracted

class DataProcessor(ABC): @abstractmethod def process(self, data): pass

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

Example - FIXED:

GOOD: Direct implementation

def process_data(data): """Process data by doubling it.""" return data * 2

Detection Checklist:

• Abstract classes with only 1-2 concrete implementations

• Generic utility classes that don't encapsulate state

• Type hierarchies deeper than 2 levels

• Mixins solving single problems

Fix Strategy:

• Identify what the abstraction solves

• Check if you really need multiple implementations now

• Delete the abstraction - use direct implementation

• If multiple implementations needed later, refactor then

• Principle: Avoid future-proofing

2. Complex Inheritance

What It Is: Deep inheritance chains, multiple inheritance, or convoluted class hierarchies that obscure code flow.

Why It's Bad: Makes code hard to follow, creates tight coupling, violates simplicity principle. Who does what becomes unclear.

Red Flags:

• 3+ levels of inheritance (GrandparentClass -> ParentClass -> ChildClass)

• Multiple inheritance from non-interface classes

• Inheritance used for code reuse instead of composition

• Overriding multiple levels of methods

• "Mixin" classes for cross-cutting concerns

Example - SMELL:

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

Example - FIXED:

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")

Detection Checklist:

• Inheritance depth > 2 levels

• Multiple inheritance from concrete classes

• Methods overridden at multiple inheritance levels

• Inheritance hierarchy with no code reuse

Fix Strategy:

• Use composition instead of inheritance

• Pass services as constructor arguments

• Each class handles its own responsibility

• Easier to test, understand, and modify

3. Large Functions (>50 Lines)

What It Is: Functions that do too many things and are difficult to understand, test, and modify.

Why It's Bad: Violates single responsibility, makes testing harder, increases bug surface area, reduces code reusability.

Red Flags:

• Functions with >50 lines of code

• Multiple indentation levels (3+ nested if/for)

• Functions with 5+ parameters

• Functions that need scrolling to see all of them

• Complex logic that's hard to name

Example - SMELL:

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

Example - FIXED:

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

Detection Checklist:

• Function body >50 lines

• 3+ levels of nesting

• Multiple unrelated operations

• Hard to name succinctly

• 5+ parameters

Fix Strategy:

• Extract helper functions for each concern

• Give each function a clear, single purpose

• Compose small functions into larger workflows

• Each function should fit on one screen

• Easy to name = usually doing one thing

4. Tight Coupling

What It Is: Modules/classes directly depend on concrete implementations instead of abstractions, making them hard to test and modify.

Why It's Bad: Changes in one module break others. Hard to test in isolation. Violates modularity principle.

Red Flags:

• Direct instantiation of classes inside functions (db = Database() )

• Deep attribute access (obj.service.repository.data )

• Hardcoded class names in conditionals

• Module imports everything from another module

• Circular dependencies between modules

Example - SMELL:

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)

Example - FIXED:

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())

Detection Checklist:

• Class instantiation inside methods (Service() )

• Deep attribute chaining (3+ dots)

• Hardcoded class references

• Circular imports or dependencies

• Module can't be tested without other modules

Fix Strategy:

• Accept dependencies as constructor parameters

• Use dependency injection

• Create test doubles (mocks) easily

• Swap implementations without changing code

• Each module is independently testable

5. Missing all Exports (Python)

What It Is: Python modules that don't explicitly define their public interface via all .

Why It's Bad: Unclear what's public vs internal. Users import private implementation details. Violates the "stud" concept - unclear connection points.

Red Flags:

• No all in init.py

• Modules expose internal functions/classes

• Users uncertain what to import

• Private names (_function ) still accessible

• Documentation doesn't match exports

Example - SMELL:

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?

Example - FIXED:

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

Detection Checklist:

• Missing all in init.py

• Internal functions (prefixed with _ ) exposed

• Unclear what's "public API"

• All imports at module level

Fix Strategy:

• Add all to every init.py

• List ONLY the public functions/classes

• Prefix internal implementation with _

• Update documentation to match all

• Clear = users know exactly what to use

Analysis Process

Step 1: Scan Code Structure

• Review file organization and module boundaries

• Identify inheritance hierarchies

• Scan for large functions (count lines)

• Note all presence/absence

• Check for tight coupling patterns

Step 2: Analyze Each Smell

For each potential issue:

• Confirm it violates philosophy

• Measure severity (critical/major/minor)

• Find specific line numbers

• Note impact on system

Step 3: Generate Fixes

For each smell found:

• Provide clear explanation of WHY it's bad

• Show BEFORE code

• Show AFTER code with detailed comments

• Explain philosophy principle violated

• Give concrete refactoring steps

Step 4: Create Report

• List all smells found

• Prioritize by severity/impact

• Include specific examples

• Provide actionable fixes

• Reference philosophy docs

Detection Rules

Rule 1: Abstract Base Classes

Check: class X(ABC) with exactly 1 concrete implementation

BAD pattern detection

• Count implementations of abstract class
• If count <= 2 and not used as interface: FLAG

Fix: Remove abstraction, use direct implementation

Rule 2: Inheritance Depth

Check: Class hierarchy depth

BAD pattern detection

• Follow inheritance chain: class -> parent -> grandparent...
• If depth > 2: FLAG

Fix: Use composition instead

Rule 3: Function Line Count

Check: All function bodies

BAD pattern detection

• Count lines in function (excluding docstring)
• If > 50 lines: FLAG
• If > 3 nesting levels: FLAG

Fix: Extract helper functions

Rule 4: Dependency Instantiation

Check: Class instantiation inside methods/functions

BAD pattern detection

• Search for "= ServiceName()" inside methods
• If found: FLAG

Fix: Pass as constructor argument

Rule 5: Missing all

Check: Python modules

BAD pattern detection

• Look for all definition
• If missing: FLAG
• If all incomplete: FLAG

Fix: Define explicit all

Common Code Smells & Quick Fixes

Smell: "Utility Class" Holder

BAD

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

Fix: Use direct function

GOOD

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

Smell: "Manager" Class

BAD

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

Fix: Split into focused services

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

Smell: God Function

BAD - 200 line function doing everything

def process_order(order_data, validate, save, notify, etc...): # 200 lines mixing validation, transformation, DB, email, logging

Fix: Compose small functions

GOOD

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

Smell: Brittle Inheritance

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()?

Fix: Use clear, testable composition

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)

Smell: Hidden Dependencies

BAD

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

Fix: Inject dependencies explicitly

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}")

Usage Examples

Example 1: Review New Module

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

Example 2: Identify Tight Coupling

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

Example 3: Simplify Inheritance

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

Analysis Checklist

Philosophy Compliance

• No unnecessary abstractions

• Single responsibility per class/function

• Clear public interface (all )

• Dependencies injected, not hidden

• Inheritance depth <= 2 levels

• Functions < 50 lines

• No dead code or stubs

Code Quality

• Each function has one clear job

• Easy to understand at a glance

• Easy to test in isolation

• Easy to modify without breaking others

• Clear naming reflects responsibility

Modularity

• Modules are independently testable

• Clear connection points ("studs")

• Loose coupling between modules

• Explicit dependencies

Success Criteria for Review

A code review using this skill should:

• Identify all violations of philosophy

• Provide specific line numbers

• Show before/after examples

• Explain WHY each is a problem

• Suggest concrete fixes

• Include test strategies

• Reference philosophy docs

• Prioritize by severity

• Be constructive and educational

• Help writer improve future code

Integration with Code Quality Tools

When to Use This Skill:

• During code review (before merge)

• In pull request comments

• Before creating new modules

• When refactoring legacy code

• To educate team members

• In design review meetings

Works Well With:

• Code review process

• Module spec generation

• Refactoring workflows

• Architecture discussions

• Mentoring and learning

Resources

• Philosophy: ~/.amplihack/.claude/context/PHILOSOPHY.md

• Patterns: ~/.amplihack/.claude/context/PATTERNS.md

• Brick Philosophy: See "Modular Architecture for AI" in PHILOSOPHY.md

• Zero-BS: See "Zero-BS Implementations" in PHILOSOPHY.md

Remember

This skill helps maintain code quality by:

• Catching issues before they become technical debt

• Educating developers on philosophy

• Keeping code simple and maintainable

• Preventing tightly-coupled systems

• Making code easier to understand and modify

Use it constructively - the goal is learning and improvement, not criticism.