Incorrect Naming and Abstractions

Capsule overview

Of all the errors AI generates, incorrect naming and misapplied abstractions are the most damaging in the long run. A SQL injection is serious, but it's a specific bug you fix once. A function called get_user that actually also modifies state, validates permissions, and sends an email — that's a time bomb that causes cumulative bugs for months.

AI is particularly prone to this type of error because it optimizes for statistical patterns. It has seen millions of functions called process_data, handle_request, and get_user. It replicates those generic names without considering whether they're descriptive for your case. It applies the Factory pattern because it saw it in a lot of Java code, even though your Python application only needs an if/else.

In this capsule you're going to train your eye to detect these patterns. Each example follows the structure: (a) the code AI generates, (b) why it looks good at first glance, (c) the real problem, (d) the correct fix.

Pattern 1: Functions with Misleading Names

The problem

AI generates functions whose names promise one thing but do several. The name describes only a fraction of what the function actually does.

Code AI generates

from fastapi import FastAPI, HTTPException
from pydantic import BaseModel
from datetime import datetime
import smtplib
from email.mime.text import MIMEText

app = FastAPI()

class User(BaseModel):
    id: int
    username: str
    email: str
    is_active: bool = True
    last_login: datetime | None = None

users_db: dict[int, dict] = {}

async def get_user(user_id: int) -> User:
    """Gets a user by ID."""
    if user_id not in users_db:
        raise HTTPException(status_code=404, detail="User not found")

    user_data = users_db[user_id]

    user_data["last_login"] = datetime.now()
    users_db[user_id] = user_data

    if not user_data["is_active"]:
        send_reactivation_email(user_data["email"])
        user_data["is_active"] = True
        users_db[user_id] = user_data

    return User(**user_data)


def send_reactivation_email(email: str) -> None:
    msg = MIMEText("Your account has been reactivated.")
    msg["Subject"] = "Account reactivated"
    msg["From"] = "noreply@app.com"
    msg["To"] = email
    with smtplib.SMTP("localhost") as server:
        server.send_message(msg)

Why it looks good at first glance

• The function has a clear docstring: "Gets a user by ID"
• The name get_user is intuitive and follows conventions
• The code compiles, the types are correct
• The code structure is clean

The real problem

The get_user function does three things:

1. Gets the user (what the name promises)
2. Modifies last_login (a hidden side effect)
3. Reactivates inactive users and sends an email (a side effect with external side effects)

Why is this a time bomb?

Scenarios that are going to cause bugs:

1. A developer uses get_user() in a read-only endpoint
   → Without knowing it, they modify last_login on every query
   → The "real last login" analytics get corrupted

2. A unit test calls get_user() to verify data
   → The test modifies state as a side effect
   → The following tests fail intermittently

3. An admin dashboard queries users frequently
   → Each query sends a reactivation email to inactive users
   → The users receive 50 emails a day

4. Another developer reads the name and assumes it's idempotent
   → They cache the result of get_user()
   → The reactivation never runs for real users

The fix

from fastapi import FastAPI, HTTPException
from pydantic import BaseModel
from datetime import datetime

app = FastAPI()

class User(BaseModel):
    id: int
    username: str
    email: str
    is_active: bool = True
    last_login: datetime | None = None

users_db: dict[int, dict] = {}


async def get_user_by_id(user_id: int) -> User:
    """Gets a user by ID. Read-only operation."""
    if user_id not in users_db:
        raise HTTPException(status_code=404, detail="User not found")
    return User(**users_db[user_id])


async def record_user_login(user_id: int) -> None:
    """Records the timestamp of the current login."""
    if user_id not in users_db:
        raise HTTPException(status_code=404, detail="User not found")
    users_db[user_id]["last_login"] = datetime.now()


async def send_reactivation_notification(email: str) -> None:
    """Sends a reactivation notification to the user."""
    msg = MIMEText("Your account has been reactivated.")
    msg["Subject"] = "Account reactivated"
    msg["From"] = "noreply@app.com"
    msg["To"] = email
    with smtplib.SMTP("localhost") as server:
        server.send_message(msg)


async def reactivate_user(user_id: int) -> User:
    """Reactivates an inactive user and sends a notification."""
    if user_id not in users_db:
        raise HTTPException(status_code=404, detail="User not found")

    user_data = users_db[user_id]
    if not user_data["is_active"]:
        user_data["is_active"] = True
        users_db[user_id] = user_data
        await send_reactivation_notification(user_data["email"])

    return User(**user_data)

Why the fix is better:

• Each function does exactly what its name says
• get_user_by_id is read-only — safe to cache, test, and use in any context
• record_user_login is explicit about the mutation
• reactivate_user is explicit about the side effect (email)
• The developer who reads the names knows exactly what to expect

Warning sign

When you see a function in AI code with a name like get_X, fetch_X, or load_X, verify that it only reads. If it modifies state, the name is misleading.

Pattern 2: Generic Names That Hide Intent

The problem

AI generates functions with names so generic that they convey no specific meaning. process_data, handle_request, do_operation — these names could apply to any function.

Code AI generates

from fastapi import FastAPI, HTTPException
from pydantic import BaseModel
from datetime import datetime

app = FastAPI()

class OrderItem(BaseModel):
    product_id: int
    quantity: int
    unit_price: float

class Order(BaseModel):
    id: int
    items: list[OrderItem]
    created_at: datetime
    status: str = "pending"

orders_db: dict[int, dict] = {}


def process_data(data: dict) -> dict:
    """Processes the received data."""
    total = 0
    processed_items = []

    for item in data.get("items", []):
        subtotal = item["quantity"] * item["unit_price"]
        if item["quantity"] > 10:
            subtotal *= 0.9
        processed_items.append({
            **item,
            "subtotal": round(subtotal, 2)
        })
        total += subtotal

    if total > 500:
        total *= 0.95

    data["items"] = processed_items
    data["total"] = round(total, 2)
    data["processed"] = True

    return data


def handle_request(request_data: dict) -> dict:
    """Handles the request."""
    validated = validate_input(request_data)
    result = process_data(validated)
    return format_output(result)


def validate_input(data: dict) -> dict:
    """Validates the input."""
    if "items" not in data:
        raise ValueError("Items required")
    return data


def format_output(data: dict) -> dict:
    """Formats the output."""
    return {
        "order": data,
        "status": "success",
        "timestamp": datetime.now().isoformat()
    }

Why it looks good at first glance

• The functions are short and follow the validate → process → format pattern
• There's apparent separation of concerns
• The code has docstrings
• The structure is clean

The real problem

Read only the function names:

process_data()     → What data? What type of processing?
handle_request()   → What request? From which endpoint?
validate_input()   → What does it validate specifically?
format_output()    → What format? For whom?

Without reading the implementation, you know nothing. Compare with descriptive names:

calculate_order_totals_with_discounts()   → You know exactly what it does
process_new_order_submission()            → You know the context
validate_order_has_items()                → You know what it validates
format_order_api_response()              → You know the format and destination

Also, process_data uses dict as the type instead of typed models — another frequent AI pattern where type safety is lost.

The fix

from fastapi import FastAPI, HTTPException
from pydantic import BaseModel
from datetime import datetime

app = FastAPI()

class OrderItem(BaseModel):
    product_id: int
    quantity: int
    unit_price: float
    subtotal: float = 0.0

class OrderRequest(BaseModel):
    items: list[OrderItem]

class OrderSummary(BaseModel):
    items: list[OrderItem]
    subtotal_before_discount: float
    discount_applied: float
    total: float

BULK_DISCOUNT_THRESHOLD = 10
BULK_DISCOUNT_RATE = 0.10
ORDER_DISCOUNT_THRESHOLD = 500.0
ORDER_DISCOUNT_RATE = 0.05


def calculate_order_totals_with_discounts(order: OrderRequest) -> OrderSummary:
    """Calculates per-item subtotals (volume discount for >10 units)
    and the order total (5% discount if it exceeds $500)."""
    items_with_subtotals = []
    subtotal_before_discount = 0.0

    for item in order.items:
        item_subtotal = item.quantity * item.unit_price
        if item.quantity > BULK_DISCOUNT_THRESHOLD:
            item_subtotal *= (1 - BULK_DISCOUNT_RATE)
        items_with_subtotals.append(item.model_copy(update={"subtotal": round(item_subtotal, 2)}))
        subtotal_before_discount += item_subtotal

    discount_applied = 0.0
    total = subtotal_before_discount
    if total > ORDER_DISCOUNT_THRESHOLD:
        discount_applied = total * ORDER_DISCOUNT_RATE
        total -= discount_applied

    return OrderSummary(
        items=items_with_subtotals,
        subtotal_before_discount=round(subtotal_before_discount, 2),
        discount_applied=round(discount_applied, 2),
        total=round(total, 2),
    )

Why the fix is better:

• The function name says exactly what it calculates
• Pydantic models replace generic dicts
• The business constants (thresholds, rates) are explicit, not magic numbers
• A new developer understands the purpose by reading only the name and signature

Warning sign

Names that start with process_, handle_, manage_, do_, or operation_ are almost always too generic. If you can't tell what a function does by reading only its name, the name is incorrect.

Pattern 3: Premature Abstractions — Unnecessary Factory

The problem

AI applies design patterns by statistical frequency, not by necessity. It has seen the Factory pattern in thousands of repositories and applies it even when a simple if/else would solve the problem in 5 lines.

Code AI generates

from abc import ABC, abstractmethod
from pydantic import BaseModel


class NotificationBase(BaseModel):
    recipient: str
    message: str


class Notification(ABC):
    @abstractmethod
    def send(self, notification: NotificationBase) -> bool:
        pass

    @abstractmethod
    def validate(self, notification: NotificationBase) -> bool:
        pass


class EmailNotification(Notification):
    def send(self, notification: NotificationBase) -> bool:
        print(f"Sending email to {notification.recipient}: {notification.message}")
        return True

    def validate(self, notification: NotificationBase) -> bool:
        return "@" in notification.recipient


class SMSNotification(Notification):
    def send(self, notification: NotificationBase) -> bool:
        print(f"Sending SMS to {notification.recipient}: {notification.message}")
        return True

    def validate(self, notification: NotificationBase) -> bool:
        return notification.recipient.startswith("+")


class PushNotification(Notification):
    def send(self, notification: NotificationBase) -> bool:
        print(f"Sending push to {notification.recipient}: {notification.message}")
        return True

    def validate(self, notification: NotificationBase) -> bool:
        return len(notification.recipient) > 0


class NotificationFactory:
    _registry: dict[str, type[Notification]] = {
        "email": EmailNotification,
        "sms": SMSNotification,
        "push": PushNotification,
    }

    @classmethod
    def create(cls, notification_type: str) -> Notification:
        if notification_type not in cls._registry:
            raise ValueError(f"Unknown notification type: {notification_type}")
        return cls._registry[notification_type]()

    @classmethod
    def register(cls, notification_type: str, notification_class: type[Notification]) -> None:
        cls._registry[notification_type] = notification_class


def send_notification(notification_type: str, recipient: str, message: str) -> bool:
    factory = NotificationFactory()
    notifier = factory.create(notification_type)
    notification = NotificationBase(recipient=recipient, message=message)

    if not notifier.validate(notification):
        raise ValueError(f"Invalid recipient for {notification_type}")

    return notifier.send(notification)

Why it looks good at first glance

• It follows a recognized design pattern (Factory)
• It has an abstract class, concrete implementations, and a factory
• It's "extensible" — you can add new notification types
• The code is well structured with class separation

The real problem

Ask yourself: how many times are you going to add a new notification type? In most applications, notification types are defined once and rarely change. This abstraction:

• Adds 70 lines of code to do what 20 lines would solve
• Creates 6 classes/files where 1 function suffices
• Introduces indirection that makes debugging harder
• The "extensibility" is never used — YAGNI (You Ain't Gonna Need It)
• The dynamic register() registration is an open door to runtime bugs

Cost of premature abstraction:

Reading:     You have to navigate 6 classes to understand what send_notification does
Debugging:   If email fails, is the bug in Factory, EmailNotification, or Notification?
Testing:     You need tests for each class + the factory + the integration
Onboarding:  A new developer takes 3x longer to understand this code
Changes:     Adding a field to NotificationBase requires touching all the subclasses

The fix

from pydantic import BaseModel, field_validator
from enum import Enum


class NotificationType(str, Enum):
    EMAIL = "email"
    SMS = "sms"
    PUSH = "push"


class NotificationRequest(BaseModel):
    notification_type: NotificationType
    recipient: str
    message: str

    @field_validator("recipient")
    @classmethod
    def validate_recipient(cls, v: str, info) -> str:
        notification_type = info.data.get("notification_type")
        if notification_type == NotificationType.EMAIL and "@" not in v:
            raise ValueError("Email recipient must contain @")
        if notification_type == NotificationType.SMS and not v.startswith("+"):
            raise ValueError("SMS recipient must start with +")
        if not v:
            raise ValueError("Recipient cannot be empty")
        return v


def send_notification(request: NotificationRequest) -> bool:
    """Sends a notification through the specified channel."""
    if request.notification_type == NotificationType.EMAIL:
        return _send_email(request.recipient, request.message)
    elif request.notification_type == NotificationType.SMS:
        return _send_sms(request.recipient, request.message)
    elif request.notification_type == NotificationType.PUSH:
        return _send_push(request.recipient, request.message)
    raise ValueError(f"Unsupported type: {request.notification_type}")


def _send_email(recipient: str, message: str) -> bool:
    print(f"Sending email to {recipient}: {message}")
    return True


def _send_sms(recipient: str, message: str) -> bool:
    print(f"Sending SMS to {recipient}: {message}")
    return True


def _send_push(recipient: str, message: str) -> bool:
    print(f"Sending push to {recipient}: {message}")
    return True

Why the fix is better:

• ~40 lines vs ~70 lines — easier to read and maintain
• A single entry point (send_notification) — easy to debug
• Pydantic handles the validation — you don't need abstract classes
• The Enum guarantees type safety — you can't pass an invalid type
• If you need the Factory pattern in the future, you refactor then (not before)

When you SHOULD use Factory

Factory makes sense when:

• You have 10+ implementations that change frequently
• The implementations are loaded from plugins or external configuration
• Different teams add implementations independently
• Creating the object requires complex variable logic

If none of these apply, an if/else is the correct abstraction.

Pattern 4: Unnecessary Deep Inheritance

The problem

AI generates inheritance hierarchies 3-4 levels deep where composition or simple functions would suffice. This comes from its training on Java/C# code where deep inheritance was more common.

Code AI generates

from datetime import datetime
from pydantic import BaseModel


class BaseEntity(BaseModel):
    id: int | None = None
    created_at: datetime = datetime.now()
    updated_at: datetime = datetime.now()

    def save(self) -> None:
        self.updated_at = datetime.now()


class BaseUserEntity(BaseEntity):
    username: str
    email: str

    def get_display_name(self) -> str:
        return self.username


class BaseActiveUserEntity(BaseUserEntity):
    is_active: bool = True
    last_login: datetime | None = None

    def deactivate(self) -> None:
        self.is_active = False

    def record_login(self) -> None:
        self.last_login = datetime.now()


class AdminUser(BaseActiveUserEntity):
    permissions: list[str] = []
    admin_level: int = 1

    def has_permission(self, permission: str) -> bool:
        return permission in self.permissions

    def grant_permission(self, permission: str) -> None:
        if permission not in self.permissions:
            self.permissions.append(permission)


class SuperAdminUser(AdminUser):
    can_delete_users: bool = True
    can_modify_system: bool = True

    def has_permission(self, permission: str) -> bool:
        return True

Why it looks good at first glance

• Each level of the hierarchy adds functionality
• The names follow a logical progression
• SuperAdmin "is an" AdminUser, AdminUser "is an" ActiveUser...
• The code is DRY — it doesn't repeat fields

The real problem

Inheritance hierarchy:
SuperAdminUser → AdminUser → BaseActiveUserEntity → BaseUserEntity → BaseEntity → BaseModel

5 levels of inheritance for a user model.

The concrete problems:

1. Rigidity: What if you need an active user that is NOT an admin but has permissions? It doesn't fit in the hierarchy.

2. Fragility: A change in BaseEntity.save() affects all the child classes. A bug at level 2 propagates to levels 3, 4, and 5.

3. Diamond problem: If in the future you need a ModeratorUser that inherits from AdminUser but with different BaseActiveUserEntity rules, the inheritance breaks.

4. datetime.now() in the default: Every instance shares the same timestamp (evaluated when the class is loaded, not when the instance is created).

5. Testing: To test SuperAdminUser, you need to understand 5 levels of inheritance.

The fix

from datetime import datetime
from pydantic import BaseModel, Field
from enum import Enum


class UserRole(str, Enum):
    USER = "user"
    ADMIN = "admin"
    SUPER_ADMIN = "super_admin"


class User(BaseModel):
    id: int | None = None
    username: str
    email: str
    role: UserRole = UserRole.USER
    is_active: bool = True
    permissions: list[str] = Field(default_factory=list)
    last_login: datetime | None = None
    created_at: datetime = Field(default_factory=datetime.now)
    updated_at: datetime = Field(default_factory=datetime.now)


def has_permission(user: User, permission: str) -> bool:
    """SuperAdmin has all permissions. Others check their list."""
    if user.role == UserRole.SUPER_ADMIN:
        return True
    return permission in user.permissions


def grant_permission(user: User, permission: str) -> User:
    """Returns a copy of the user with the permission added."""
    if permission not in user.permissions:
        updated_permissions = [*user.permissions, permission]
        return user.model_copy(update={"permissions": updated_permissions})
    return user


def record_login(user: User) -> User:
    """Returns a copy of the user with last_login updated."""
    return user.model_copy(update={"last_login": datetime.now()})


def deactivate_user(user: User) -> User:
    """Returns a copy of the deactivated user."""
    return user.model_copy(update={"is_active": False})

Why the fix is better:

• A single User model with a role field — no hierarchy
• Pure functions that return copies — no hidden mutation
• Field(default_factory=datetime.now) resolves the shared-timestamp bug
• Adding a new role only requires extending the Enum
• Testing is trivial: you create a User with the fields you need

Warning sign

When you see inheritance more than 2 levels deep in AI code, ask yourself: "Could I solve this with composition (a field that defines the type) instead of inheritance?" The answer is almost always yes.

Pattern 5: Misapplied Design Pattern — Unnecessary Singleton

The problem

AI applies the Singleton pattern to classes that don't need it. Singleton makes sense for truly global resources (connection pools, system configuration). AI uses it for any class that "should have a single instance."

Code AI generates

from threading import Lock


class DatabaseConfig:
    _instance = None
    _lock = Lock()

    def __new__(cls):
        if cls._instance is None:
            with cls._lock:
                if cls._instance is None:
                    cls._instance = super().__new__(cls)
                    cls._instance._initialized = False
        return cls._instance

    def __init__(self):
        if self._initialized:
            return
        self.host = "localhost"
        self.port = 5432
        self.database = "myapp"
        self.username = "admin"
        self.password = "secret123"
        self._initialized = True

    def get_connection_string(self) -> str:
        return f"postgresql://{self.username}:{self.password}@{self.host}:{self.port}/{self.database}"


class Logger:
    _instance = None
    _lock = Lock()

    def __new__(cls):
        if cls._instance is None:
            with cls._lock:
                if cls._instance is None:
                    cls._instance = super().__new__(cls)
                    cls._instance._initialized = False
        return cls._instance

    def __init__(self):
        if self._initialized:
            return
        self.logs: list[str] = []
        self._initialized = True

    def log(self, message: str) -> None:
        self.logs.append(message)

    def get_logs(self) -> list[str]:
        return self.logs.copy()


config = DatabaseConfig()
logger = Logger()

Why it looks good at first glance

• Singleton is a recognized pattern
• The double-checked locking is the "correct" implementation
• The intent (a single instance) seems reasonable
• The code is thread-safe

The real problem

1. DatabaseConfig as a Singleton: Configuration should be loaded from environment variables, not hardcoded. The Singleton makes it impossible to have different configurations for dev/staging/production without modifying the class.

2. Logger as a Singleton: Python already has a logging module that handles this. Reinventing logging with a list is losing features (levels, formatters, handlers).

3. Testing: Singletons make tests depend on each other. If a test modifies the Singleton, it affects all the following tests. You need manual teardown.

4. Hardcoded password: "secret123" directly in the code. A serious security problem that the Singleton pattern hides visually.

5. Global coupling: All the code depends on the global instance. Changing the configuration requires modifying the class.

The fix

from pydantic_settings import BaseSettings
from functools import lru_cache
import logging


class DatabaseConfig(BaseSettings):
    host: str = "localhost"
    port: int = 5432
    database: str = "myapp"
    username: str = "admin"
    password: str

    model_config = {"env_prefix": "DB_"}

    @property
    def connection_string(self) -> str:
        return f"postgresql://{self.username}:{self.password}@{self.host}:{self.port}/{self.database}"


@lru_cache
def get_database_config() -> DatabaseConfig:
    """Functional singleton: caches the instance but allows override in tests."""
    return DatabaseConfig()


def get_logger(name: str) -> logging.Logger:
    """Uses Python's standard logging system."""
    logger = logging.getLogger(name)
    if not logger.handlers:
        handler = logging.StreamHandler()
        handler.setFormatter(
            logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s")
        )
        logger.addHandler(handler)
        logger.setLevel(logging.INFO)
    return logger

Why the fix is better:

• BaseSettings loads config from environment variables automatically
• password is required — it has no hardcoded default
• @lru_cache acts as a functional singleton: it caches but can be cleared with get_database_config.cache_clear() in tests
• Uses Python's standard logging instead of reinventing the wheel
• Testing: get_database_config.cache_clear() clears the cache between tests

Warning sign

When you see _instance = None and a __new__ override in AI code, ask yourself: "Does this resource really need to be global and unique?" In Python, @lru_cache on a factory function or a module-level variable is usually enough.

Summary of Warning Signs

Quick checklist for naming and abstractions:

☐ get_/fetch_/load_ functions that modify state
  → Verify that they only read

☐ Generic names: process_, handle_, manage_, do_
  → Replace with names that describe the specific action

☐ Factory pattern with < 5 implementations
  → Consider if/else or match/case

☐ Inheritance more than 2 levels deep
  → Consider composition with enums/fields

☐ Singleton with a __new__ override
  → Consider @lru_cache or a module-level variable

☐ Abstract classes with a single implementation
  → Remove the abstraction, use the implementation directly

☐ Magic numbers in business logic
  → Extract to constants with descriptive names

Connection to the Project

The capstone project in module 8 contains 3-4 naming and abstraction errors. Specifically, look for:

• ✅ At least one function with a misleading name that has side effects
• ✅ At least one premature abstraction (a pattern where it's not needed)
• ✅ Generic names that make it hard to understand what the code does
• ✅ Business constants as magic numbers

What you practice in this capsule is exactly what you'll apply in the project.

Troubleshooting

"When IS it correct to use generic names?"

In utility functions that are genuinely generic. A map(), filter(), or sort() are generic because they operate on any data. But a function that calculates order discounts isn't generic — it has a specific domain.

"How do I know if an abstraction is premature?"

The rule of three: don't create an abstraction until you have at least 3 concrete implementations. If you only have 1-2, it's premature.

"Does AI always generate these errors?"

Not always, but frequently. If your prompt is specific about naming and structure, AI generates better code. Vague prompts → vague names.

"Do I refactor all the project's naming or only the new code?"

Start with the new code. Refactoring existing naming is valuable but introduces risk. Apply the Boy Scout rule: leave the code better than you found it, one name at a time.

"What do I do if the team uses generic names by convention?"

Respect the team's convention but suggest improvements gradually. A process_order in a codebase where everything is called process_X is consistent, even if not ideal.

Exercises

Exercise 1: Identify misleading naming

Analyze this function. What does it actually do vs what the name promises?

from datetime import datetime

users_db: dict[int, dict] = {
    1: {"id": 1, "username": "alice", "email": "alice@example.com", "login_count": 5},
    2: {"id": 2, "username": "bob", "email": "bob@example.com", "login_count": 0},
}

def check_user(user_id: int) -> bool:
    """Checks whether the user exists."""
    if user_id not in users_db:
        return False

    user = users_db[user_id]
    user["login_count"] += 1
    user["last_checked"] = datetime.now().isoformat()

    if user["login_count"] > 100:
        user["status"] = "veteran"

    return True

from datetime import datetime

def user_exists(user_id: int) -> bool:
    """Checks whether the user exists. Read-only."""
    return user_id in users_db

def increment_login_count(user_id: int) -> int:
    """Increments and returns the new login count."""
    if user_id not in users_db:
        raise KeyError(f"User {user_id} not found")
    users_db[user_id]["login_count"] += 1
    return users_db[user_id]["login_count"]

def update_veteran_status(user_id: int, threshold: int = 100) -> bool:
    """Marks the user as veteran if it exceeds the threshold. Returns True if updated."""
    if user_id not in users_db:
        raise KeyError(f"User {user_id} not found")
    user = users_db[user_id]
    if user.get("login_count", 0) > threshold and user.get("status") != "veteran":
        user["status"] = "veteran"
        return True
    return False

Exercise 2: Simplify a premature abstraction

This code uses the Strategy pattern to calculate taxes. Simplify it without losing functionality.

from abc import ABC, abstractmethod


class TaxStrategy(ABC):
    @abstractmethod
    def calculate(self, amount: float) -> float:
        pass


class USATaxStrategy(TaxStrategy):
    def calculate(self, amount: float) -> float:
        return amount * 0.08


class EUTaxStrategy(TaxStrategy):
    def calculate(self, amount: float) -> float:
        return amount * 0.21


class TaxCalculator:
    def __init__(self, strategy: TaxStrategy):
        self.strategy = strategy

    def get_tax(self, amount: float) -> float:
        return self.strategy.calculate(amount)


calculator = TaxCalculator(USATaxStrategy())
tax = calculator.get_tax(100.0)

from enum import Enum

class TaxRegion(str, Enum):
    USA = "usa"
    EU = "eu"

TAX_RATES: dict[TaxRegion, float] = {
    TaxRegion.USA: 0.08,
    TaxRegion.EU: 0.21,
}

def calculate_tax(amount: float, region: TaxRegion) -> float:
    """Calculates the tax according to the region."""
    rate = TAX_RATES.get(region)
    if rate is None:
        raise ValueError(f"No tax rate defined for region: {region}")
    return amount * rate

tax = calculate_tax(100.0, TaxRegion.USA)

Exercise 3: Refactor inheritance to composition

Convert this 3-level hierarchy into a flat model with an enum.

from pydantic import BaseModel


class BaseVehicle(BaseModel):
    make: str
    model: str
    year: int

    def describe(self) -> str:
        return f"{self.year} {self.make} {self.model}"


class MotorVehicle(BaseVehicle):
    engine_size: float
    fuel_type: str = "gasoline"

    def fuel_cost_per_km(self) -> float:
        return self.engine_size * 0.05


class ElectricVehicle(BaseVehicle):
    battery_capacity: float
    range_km: int

    def fuel_cost_per_km(self) -> float:
        return self.battery_capacity * 0.001

from pydantic import BaseModel, model_validator
from enum import Enum


class PowertrainType(str, Enum):
    GASOLINE = "gasoline"
    DIESEL = "diesel"
    ELECTRIC = "electric"
    HYBRID = "hybrid"

COST_PER_KM = {
    PowertrainType.GASOLINE: lambda engine_size: engine_size * 0.05,
    PowertrainType.DIESEL: lambda engine_size: engine_size * 0.04,
    PowertrainType.ELECTRIC: lambda battery_cap: battery_cap * 0.001,
    PowertrainType.HYBRID: lambda engine_size: engine_size * 0.03,
}

class Vehicle(BaseModel):
    make: str
    model: str
    year: int
    powertrain: PowertrainType
    engine_size: float | None = None
    battery_capacity: float | None = None
    range_km: int | None = None

    @model_validator(mode="after")
    def validate_powertrain_fields(self):
        if self.powertrain == PowertrainType.ELECTRIC:
            if self.battery_capacity is None:
                raise ValueError("Electric vehicles require battery_capacity")
        else:
            if self.engine_size is None:
                raise ValueError(f"{self.powertrain.value} vehicles require engine_size")
        return self

    def describe(self) -> str:
        return f"{self.year} {self.make} {self.model} ({self.powertrain.value})"

    def fuel_cost_per_km(self) -> float:
        calculator = COST_PER_KM[self.powertrain]
        if self.powertrain == PowertrainType.ELECTRIC:
            return calculator(self.battery_capacity)
        return calculator(self.engine_size)

Exercise 4: Rename generic functions

These functions have generic names. Rename them based on what they actually do.

users_db: dict[int, dict] = {
    1: {"id": 1, "username": "alice", "banned": False},
    2: {"id": 2, "username": "bob", "banned": False},
}

def process(items: list[dict]) -> list[dict]:
    return [item for item in items if item.get("status") == "active"]

def handle(data: dict) -> dict:
    data["total"] = sum(item["price"] * item["qty"] for item in data["items"])
    data["tax"] = data["total"] * 0.16
    data["grand_total"] = data["total"] + data["tax"]
    return data

def transform(records: list[dict]) -> dict[str, list[dict]]:
    result: dict[str, list[dict]] = {}
    for record in records:
        category = record.get("category", "uncategorized")
        result.setdefault(category, []).append(record)
    return result

def execute(user_id: int, action: str) -> bool:
    if action == "ban":
        users_db[user_id]["banned"] = True
    elif action == "unban":
        users_db[user_id]["banned"] = False
    return True

def filter_active_items(items: list[dict]) -> list[dict]:
    """Returns only the items with status 'active'."""
    return [item for item in items if item.get("status") == "active"]


def calculate_order_totals_with_tax(order_data: dict, tax_rate: float = 0.16) -> dict:
    """Calculates the subtotal, tax, and grand total of an order."""
    order_data["subtotal"] = sum(
        item["price"] * item["qty"] for item in order_data["items"]
    )
    order_data["tax"] = order_data["subtotal"] * tax_rate
    order_data["grand_total"] = order_data["subtotal"] + order_data["tax"]
    return order_data


def group_records_by_category(records: list[dict]) -> dict[str, list[dict]]:
    """Groups records by their 'category' field."""
    result: dict[str, list[dict]] = {}
    for record in records:
        category = record.get("category", "uncategorized")
        result.setdefault(category, []).append(record)
    return result


def set_user_ban_status(user_id: int, *, banned: bool) -> None:
    """Sets a user's ban status."""
    if user_id not in users_db:
        raise KeyError(f"User {user_id} not found")
    users_db[user_id]["banned"] = banned

Summary

• Misleading naming is the most subtle error: get_X functions that modify state are time bombs
• Generic names (process_, handle_) don't communicate intent — they force you to read the implementation
• Premature abstractions (Factory, Strategy) add complexity with no benefit when there are few implementations
• Deep inheritance (3+ levels) creates rigidity — composition with enums is more flexible in Python
• Misapplied Singleton complicates testing and hides global dependencies
• AI generates these errors because it replicates statistical patterns without evaluating whether they apply to your context
• The general rule: a function's name should be enough to understand what it does without reading its implementation

Additional resources

1. Clean Code — Meaningful Names - Chapter 2 of Clean Code on effective naming
2. Refactoring Guru — Code Smells - A catalog of code smells with suggested refactorings
3. YAGNI — Martin Fowler - "You Aren't Gonna Need It" — why premature abstractions cost more than they save
4. Composition over Inheritance — Wikipedia - A fundamental principle of object-oriented design
5. Python Design Patterns - Idiomatic design patterns in Python (not translated Java)
6. FastAPI Dependency Injection - How FastAPI solves the singleton problem with dependency injection

Next capsule: Unhandled Edge Cases — the pattern that causes crashes in production.

Debugging & Code Review with Claude Code — Module 5, Capsule 02 Claude Code Agentic Development Path — Guide #6 of 11