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:

1. Identify what the abstraction solves
2. Check if you really need multiple implementations now
3. Delete the abstraction - use direct implementation
4. If multiple implementations needed later, refactor then
5. 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:

1. Use composition instead of inheritance
2. Pass services as constructor arguments
3. Each class handles its own responsibility
4. 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:

1. Extract helper functions for each concern
2. Give each function a clear, single purpose
3. Compose small functions into larger workflows
4. Each function should fit on one screen
5. 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):