Skip to content

Code Smells — Athomic

"If it stinks, change it." — Kent Beck's grandmother

Detect: make smells (full report) / make smells-summary (counts only). Detector: scripts/check_smells.py — always exits 0, advisory only.

Current state (baseline 2026-06): 756 smells across 5 categories. Fix them incrementally. Track progress with make smells-summary.

A code smell is not a bug. It's a structural weakness that signals a deeper problem — code that grew without discipline, or OOP applied incorrectly, or coupling that makes change painful. Each smell has a canonical refactoring from Fowler.


CATEGORY 1: BLOATERS

Code that grows without discipline. Usually accumulate over time when no one refactors.


SMELL-B01 — Long Method (219 in codebase)

Definition: A function or method longer than 50 lines.

Why it stinks: A 100-line method has more than one responsibility. It's impossible to test all branches, impossible to read without a map, impossible to reuse any part of it.

Real instances:

197 lines  resilience/adaptive_concurrency/controller.py:149  def __init__
150 lines  ai/features/base.py:160                            def encode_batch
125 lines  resilience/adaptive_concurrency/controller.py:1073 def _build_telemetry

Refactoring: Extract Method

# ❌ BEFORE — 150 lines doing everything inline
async def encode_batch(self, raw_states):
    # normalize...
    # validate...
    # check cache...
    # execute...
    # write cache...
    # record metrics...

# ✅ AFTER — each concern is a named stage
async def encode_batch(self, raw_states):
    return await self._decorated.encode_tensor_batch(raw_states)
    # each decorator handles one concern

SMELL-B02 — Large Class / God Class (6 in codebase)

Definition: A file with 500+ lines. At 800+ lines it's a god class.

Why it stinks: One class that knows everything = one class you're afraid to touch. Every feature touches it. It's the epicenter of merge conflicts and regressions.

Real instances:

1267 lines  resilience/adaptive_concurrency/controller.py  ← CRITICAL
 818 lines  database/kvstore/base.py
 598 lines  ai/features/caching/proxy.py

Refactoring: Extract Class

# ❌ BEFORE — AdaptiveConcurrencyController owns:
# AIMD algorithm + DRF fairness + Slot management + Admission queue + Telemetry building

# ✅ AFTER — one file, one responsibility:
# controller.py      ← orchestrates only
# algorithms/aimd.py ← AIMD calculation only
# fairness/drf.py    ← DRF fairness only
# telemetry.py       ← metrics assembly only

SMELL-B03 — Long Parameter List (67 in codebase)

Definition: Functions with more than 5 parameters (excluding self/cls).

Why it stinks: Callers must know the order. Adding a parameter breaks every caller. Five or more parameters means the function probably has too many jobs.

Real instances:

13 params  performance/cache/handlers/cache_handler.py:32  def __init__
13 params  integration/messaging/decorators/consumer.py    def subscribe_to
 9 params  ai/features/base.py:84                         def __init__

Refactoring: Introduce Parameter Object

# ❌ BEFORE
def __init__(self, key_prefix, ttl, compress_above_bytes, compress_level,
             max_payload_bytes, dtype, semantic_version, epoch, vector_id,
             cache_provider, metrics_enabled, coalescing, backoff_base): ...

# ✅ AFTER — already the pattern in this codebase
def __init__(self, settings: FeatureCacheSettings, vector_id: str) -> None: ...

SMELL-B04 — Primitive Obsession (248 in codebase)

Definition: Using primitive types (dict, str, int) for data that has structure and behavior — things that should be objects.

Why it stinks: No validation at the boundary. No domain language. Business rules scattered across the codebase instead of encapsulated in a type.

Real instances:

param 'parameters: Dict[str, Any]' in execute_query() — should be QueryParameters
param 'parameters: Dict[str, Any]' in run_query() — same

Refactoring: Replace Primitive with Object

# ❌ BEFORE
def execute_query(cypher: str, parameters: Dict[str, Any]) -> List[Dict]: ...

# ✅ AFTER
@dataclass(frozen=True)
class GraphQuery:
    cypher: str
    parameters: dict[str, Any] = field(default_factory=dict)

    def __post_init__(self) -> None:
        if not self.cypher.strip():
            raise ValueError("Empty Cypher query")

def execute_query(query: GraphQuery) -> List[Dict]: ...

SMELL-B05 — Data Clumps (32 in codebase)

Definition: The same group of 3+ parameters appearing together in 4+ functions. They're a clump — they want to become a class.

Why it stinks: If you need to add a field to the "group", you must update every function signature. The group has no name, no validation, no documentation.

Refactoring: Introduce Parameter Object

# ❌ BEFORE — (host, port, timeout) appear in 8 different functions
def create_client(host: str, port: int, timeout: float) -> Client: ...
def connect(host: str, port: int, timeout: float) -> None: ...
def ping(host: str, port: int, timeout: float) -> bool: ...

# ✅ AFTER
@dataclass(frozen=True)
class ConnectionConfig:
    host: str
    port: int
    timeout: float

def create_client(config: ConnectionConfig) -> Client: ...

CATEGORY 2: OOP ABUSERS

Incomplete or incorrect application of object-oriented principles.


SMELL-O01 — Switch Statements / isinstance Chains (19 in codebase)

Definition: A function with 3+ isinstance() calls dispatching to different behavior.

Why it stinks: Every time you add a new type, you must find all dispatch points and add a new branch. This is an Open/Closed Principle violation — the code is not closed for modification.

Real instances:

23 isinstance() calls  serializer/base.py     (worst)
10 isinstance() calls  ai/features/caching/proxy.py
 8 isinstance() calls  event_sourcing/stores/kv_snapshot_store.py

Refactoring: Replace Conditional with Polymorphism (Strategy + Registry)

# ❌ BEFORE — Open for modification at every new type
def serialize(value):
    if isinstance(value, str): return value.encode()
    elif isinstance(value, int): return str(value).encode()
    elif isinstance(value, dict): return json.dumps(value).encode()
    # ... adding float? Must modify this function

# ✅ AFTER — Open for extension via Registry
class SerializerProtocol(Protocol):
    def serialize(self, value: Any) -> bytes: ...

_REGISTRY: dict[type, SerializerProtocol] = {
    str: StringSerializer(),
    int: IntSerializer(),
    dict: JsonSerializer(),
}

def serialize(value: Any) -> bytes:
    handler = _REGISTRY.get(type(value))
    if handler is None:
        raise TypeError(f"No serializer for {type(value)}")
    return handler.serialize(value)
# Adding float? Just: _REGISTRY[float] = FloatSerializer()

SMELL-O02 — Mutable Default Argument

Definition: def f(x=[]) or def f(x={}).

Why it stinks: Python creates the default ONCE when the function is defined. All calls share THE SAME object. Appending to it corrupts all future calls.

# ❌ WRONG — x is shared between all callers
def process(items: list = []):
    items.append("new")
    return items

process()  # ["new"]
process()  # ["new", "new"]  ← BUG!

# ✅ CORRECT
def process(items: list | None = None) -> list:
    if items is None:
        items = []
    items.append("new")
    return items

SMELL-O03 — Temporary Field

Definition: An instance variable that is set only inside a conditional in a business method, but never declared in __init__. When the condition is false, the attribute doesn't exist — leading to AttributeError at runtime.

Note: Lazy initialization in lifecycle methods (_connect, setup) is NOT this smell.

Refactoring: Introduce Null Object or declare with None default

# ❌ WRONG — self._buffer only exists after process() is called
class Processor:
    def __init__(self): pass  # _buffer not declared here!

    def process(self, data):
        if data:
            self._buffer = []  # Temporary field!
            self._buffer.extend(data)

    def flush(self):
        return self._buffer  # AttributeError if process() never called!

# ✅ CORRECT
class Processor:
    def __init__(self):
        self._buffer: list = []  # Always exists, safe default

    def process(self, data):
        if data:
            self._buffer.extend(data)

CATEGORY 3: CHANGE PREVENTERS

When a change in one place forces changes everywhere else.


SMELL-C01 — Divergent Change

Definition: One class that changes for many different reasons (SRP violation). Symptom: importing from 12+ different modules = too many masters.

Why it stinks: You touch the class for a cache change, a logging change, a validation change, a schema change. It's a change magnet.

Refactoring: Extract Class — split by reason to change.


SMELL-C02 — Shotgun Surgery (5 in codebase)

Definition: A single change requires modifications in many different files. Symptom: a non-infrastructure symbol imported by 20+ files.

Real instances:

KVStoreProtocol    imported by 37 files
DecisionContext    imported by 37 files
connection_manager_factory  imported by 25 files

Refactoring: Move Method / Consolidate

# If changing KVStoreProtocol forces changes in 37 files,
# either the protocol is too fine-grained (needs versioning),
# or the 37 files should go through a Facade instead of depending directly.

CATEGORY 4: DISPENSABLES

Pointless code whose absence would make things cleaner.


SMELL-D01 — Excessive Comments (3 in codebase)

Definition: Files where > 35% of non-blank lines are comments.

Why it stinks: A comment is an apology for unclear code. If you need 3 paragraphs to explain what a method does, the method name and structure are wrong. (Exception: legal headers, complex math with formula references.)

Refactoring: Rename to make code self-documenting

# ❌ BEFORE — needs a comment
# Check if the elapsed time exceeds our allowed processing window
if time.time() - self.start > self.max_seconds:
    ...

# ✅ AFTER — name carries the meaning
if self._processing_window_exceeded():
    ...

SMELL-D02 — Duplicate Code

Definition: The same code block appearing in multiple places.

Why it stinks: Fix a bug in one copy, forget the others. The codebase now has multiple versions of the "truth". Dijkstra: "if you have two things, you have two different things."

Refactoring: Extract Function

# ❌ WRONG — same normalization logic in 3 methods
def encode(self, states):
    normalized = [s if isinstance(s, dict) else s.dict() for s in states]
    ...
def encode_batch(self, states):
    normalized = [s if isinstance(s, dict) else s.dict() for s in states]
    ...

# ✅ CORRECT — one function, one truth
# normalize_states() already exists in ai/features/types.py

SMELL-D03 — Data Class (7 in codebase)

Definition: A class that only holds data (fields + getters), has no behavior.

Why it stinks: The behavior that operates on this data is scattered across the codebase. The class is just a dumb container; it offers no abstraction.

Refactoring: Move Behavior Here, or use @dataclass / Pydantic

# ❌ BEFORE — dumb container
class GraphQueryResult:
    def __init__(self):
        self.rows: list = []
        self.columns: list[str] = []
        self.execution_time: float = 0.0

    # No behavior — just stores data

# ✅ CORRECT OPTION A — use @dataclass if it's just data
@dataclass(frozen=True)
class GraphQueryResult:
    rows: list[dict]
    columns: list[str]
    execution_time: float

    def is_empty(self) -> bool:          # ← behavior belongs HERE
        return len(self.rows) == 0

    def to_dataframe(self): ...          # ← behavior belongs HERE

SMELL-D04 — Dead Code

Definition: Variables assigned but never read, methods never called.

Why it stinks: Readers waste time understanding code that does nothing. Dead code rots — it's never updated when surrounding code changes.

Refactoring: Delete it. Version control is your safety net.


SMELL-D05 — Lazy Class (46 in codebase)

Definition: A concrete class with fewer than 2 public methods and under 40 lines. It doesn't do enough to justify existing as a class.

Refactoring: Inline Class or replace with a function

# ❌ BEFORE — a class for a single function
class TimeoutCalculator:
    def calculate(self, base: float, multiplier: float) -> float:
        return base * multiplier

# ✅ AFTER — just a function
def calculate_timeout(base: float, multiplier: float) -> float:
    return base * multiplier

SMELL-D06 — print() in Production (10 in codebase)

Definition: print() calls in src/ (outside tests).

Why it stinks: Bypasses the structured logging pipeline. Invisible in production. No log level, no trace ID, no structured context.

Refactoring:

# ❌ WRONG
print(f"Processing batch of {len(items)} items")

# ✅ CORRECT
from loguru import logger
logger.debug("Processing batch", count=len(items))

CATEGORY 5: COUPLERS

Excessive coupling between classes.


SMELL-P01 — Inappropriate Intimacy (67 in codebase)

Definition: A class accessing the private attributes (_name) of another class.

Why it stinks: It couples two classes at their implementation level, not their interface level. Change the private attribute → the other class breaks.

Real instances:

causality/vector_clock.py  → Accessing incoming_vc._clock
database/manager.py        → Accessing s._run_loop

Refactoring: Move Method or expose via Protocol

# ❌ WRONG — VectorClock reads another VectorClock's internals
class VectorClock:
    def merge(self, other: "VectorClock") -> None:
        for key, ts in other._clock.items():  # intimate!
            ...

# ✅ CORRECT — expose what's needed via a public method
class VectorClock:
    def items(self) -> Iterable[tuple[str, int]]:
        return self._clock.items()

    def merge(self, other: "VectorClock") -> None:
        for key, ts in other.items():  # clean interface
            ...

SMELL-P02 — Message Chains (2 in codebase)

Definition: A long chain of calls: a.b().c().d().e() (5+ levels deep).

Why it stinks: You're navigating an object graph manually. If the middle changes, every chain breaks. Violates Law of Demeter: "only talk to your immediate neighbors."

Refactoring: Hide Delegate

# ❌ WRONG
result = self._engine._plan._offsets[feature_name][0]

# ✅ CORRECT — ask for what you need, not how to navigate to it
result = self._plan.get_offset(feature_name)

SMELL-P03 — Middle Man (1 in codebase)

Definition: A class where most methods just delegate to another object. It adds indirection without adding value.

Refactoring: Remove Middle Man or Inline Method

# ❌ WRONG — FeatureProxy just forwards everything
class FeatureProxy:
    def encode(self, state): return self._service.encode(state)
    def encode_batch(self, states): return self._service.encode_batch(states)
    def get_dimension(self, vid): return self._service.get_dimension(vid)
    # Why does this class exist?

# ✅ CORRECT — remove the proxy, use the service directly
# OR make the proxy add real value (caching, auth, rate limiting)

SMELL-P04 — Bare/Broad except (24 in codebase)

Definition: except: or except Exception: with no logging and no re-raise. Errors disappear silently.

Refactoring: Log or catch specific type

# ❌ WRONG — error disappears
try:
    result = await self._client.get(key)
except Exception:
    pass

# ✅ CORRECT
try:
    result = await self._client.get(key)
except ConnectionError as e:
    logger.warning("Cache unavailable", key=key, error=str(e))
    return None
except Exception:
    logger.exception("Unexpected cache error", key=key)
    raise

Smell → Refactoring Quick Reference

Code Smell Refactoring
B01 Long Method Extract Method
B02 Large Class / God Class Extract Class
B03 Long Parameter List Introduce Parameter Object
B04 Primitive Obsession Replace Primitive with Object
B05 Data Clumps Introduce Parameter Object
O01 Switch Statements (isinstance) Replace Conditional with Polymorphism
O02 Mutable Default Argument Use None + initialize in body
O03 Temporary Field Declare with None default in init
C01 Divergent Change Extract Class
C02 Shotgun Surgery Move Method / Facade
D01 Excessive Comments Rename + restructure
D02 Duplicate Code Extract Function
D03 Data Class Move Behavior / use @dataclass
D04 Dead Code Delete
D05 Lazy Class Inline Class / function
D06 print() in prod Replace with logger.*
P01 Inappropriate Intimacy Move Method / Protocol
P02 Message Chains Hide Delegate
P03 Middle Man Remove / Inline Method
P04 Bare except Catch specific type + log

Not statically detectable (require human review)

Smell Why undetectable How to detect manually
Feature Envy Needs type data to know which class's data a method uses Method that takes 3+ params from another class's fields
Refused Bequest Needs runtime usage data Subclass that overrides parent and calls pass or raise NotImplementedError
Alternative Classes with Different Interfaces Semantic similarity beyond syntax Two classes with similar fields but different method names for same concept
Parallel Inheritance Hierarchies Requires full class hierarchy map Every new subclass of A requires a new subclass of B
Speculative Generality Needs usage data Abstract class or method with exactly 1 concrete implementation
Incomplete Library Class Domain-specific judgment You're extending a stdlib/third-party class just to add one method

  • SOLID architecture rules: docs/guides/architecture/solid-rules.md
  • Design patterns: docs/guides/architecture/design-patterns.md
  • Violations backlog: docs/backlog/todo/solid-violations.md