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 |
Related Documents
- 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