Code should be performant, changeable, readable and testable

Code1[1] None of this is original thought; it’s just a summary of other sources on code quality. should be performant, easy to change, easy to read, and easy to test. We will write tests once the code base is stable, but we should design for testability from the start. We will also follow project-specific standards for file organization, naming, and error handling. Prefer a functional style— separate data from code, avoid global / module state using pure functions and dependency injection. Use classes only when they provide clear value.

1. Performance & Architecture

Deep Modules Over Shallow Modules (Ousterhout)

Modules should provide deep functionality behind simple interfaces. Users should specify “what” and not worry about “how.”

✅ Good:

def fetch_user_orders(user_id: int, start_date: str, end_date: str) -> list[Order]:
    """Simple interface hides complex DB query implementation."""
    # ... joins across orders, line_items, and statuses tables,
    # date parsing, pagination, and result mapping
    return orders

❌ Bad:

def get_orders(db):
    """Too shallow - caller must handle all details."""
    return db.execute("SELECT * FROM orders")

# Caller now has to know cursor internals
cursor = get_orders(db)
for row in cursor:       # Caller iterates
    process(row)
cursor.close()           # Caller must remember to close

Minimize Abstraction Layers (Ousterhout)

Each layer adds overhead. Only create abstractions that provide significant value.

✅ Good:

# Direct: Request → Router → Service Function
@router.post("/orders")
def create_order_endpoint(order_data: OrderCreate):
    return process_order(order_data)

❌ Bad:

# Too many layers: Request → Router → Manager → Builder → Factory → Service
@router.post("/orders")
def create_order_endpoint(order_data: OrderCreate):
    manager = OrderManager()
    builder = manager.get_builder()
    factory = builder.create_factory()
    return factory.process(order_data)

Information Hiding Reduces Coupling (Ousterhout)

Well-hidden implementation details allow for performance optimizations without breaking clients.

✅ Good:

def get_active_users(filters: UserFilters) -> list[User]:
    """Public interface - implementation can change."""
    # Could hit SQL today, a cache tomorrow — callers don't care
    return _query_active_users(filters)

---

2. Changeability

Design for Change, Not Current Requirements (Ousterhout)

Build systems that can evolve without major rewrites.

✅ Good:

# Data-driven: Adding a new query param takes 1 line
param_configs = [
    ("status", "o.status"),
    ("region", "o.region"),
    ("tier", "u.tier"),
    # Add new param here
]

for param_key, column in param_configs:
    value = query_params.get(param_key)
    if value:
        conditions.append(f"{column} = :{param_key}")
        bind_vars[param_key] = value

❌ Bad:

# Hardcoded: Adding a new param requires modifying logic
status = query_params.get("status")
if status:
    conditions.append("o.status = :status")
    bind_vars["status"] = status

region = query_params.get("region")
if region:
    conditions.append("o.region = :region")
    bind_vars["region"] = region
# Must duplicate for each new param

Eliminate Duplication (Beck)

Change logic in one place rather than hunting down copies.

✅ Good:

def get_field_column_map():
    return [
        ("status", "status"),
        ("region", "region"),
        ("tier", "tier"),
    ]

for field_key, db_column in get_field_column_map():
    values = filters.get(field_key, [])
    if values:
        query = query.where(getattr(Order, db_column).in_(values))

❌ Bad:

statuses = filters.get("status", [])
if statuses:
    query = query.where(Order.status.in_(statuses))

regions = filters.get("region", [])
if regions:
    query = query.where(Order.region.in_(regions))
# Duplicated logic for each field

Use Dependency Injection (Hevery)

Ask for what you need rather than creating or looking for it.

✅ Good:

def process_order(order: Order, logger: Logger = get_logger(__name__)):
    """Logger is injected - easy to test and change."""
    logger.info("Processing order")
    return _submit(order)

❌ Bad:

def process_order(order: Order):
    """Hard dependency on global logger."""
    import logging
    logging.getLogger().info("Processing order")  # Hard to test
    return _submit(order)

✅ Good:

# Functional approach with config as a pure function

def get_config() -> Dict[str, Any]:
    """Provides the service configuration."""
    return {
        "max_retries": 3,
        "timeout_seconds": 30,
    }

def process_request(
    request: Request,
    get_config_func: Callable[[], Dict[str, Any]] = get_config,
) -> Response:
    """Config is passed explicitly, making the function testable and predictable."""
    config = get_config_func()
    return _handle(request, config)

❌ Bad:

# Mutable module-level state
CONFIG = {}

def process_request(request: Request):
    """Relies on mutable global state, making it hard to test and debug."""
    return _handle(request, CONFIG)

# Somewhere else in the code
CONFIG.update({"max_retries": 3, "timeout_seconds": 30})

Favor Composition Over Inheritance (Hevery)

Composition is more flexible when requirements shift.

✅ Good:

class OrderService:
    def __init__(self, repo, emailer):
        self.repo = repo
        self.emailer = emailer

❌ Bad:

class OrderService(OrderRepo, EmailSender, PaymentProcessor, AuditLogger):
    """Rigid inheritance hierarchy - hard to change."""
    pass

---

3. Readability

Choose Precise, Meaningful Names (Ousterhout/Beck)

Names should eliminate the need for additional explanation.

✅ Good:

def filter_users(users: list[User], criteria: Dict[str, Any]) -> list[User]:
    """Clear what it does, what it takes, what it returns."""
    pass

requested_status_filter = criteria.get("status")
active_user_records = [u for u in users if u.is_active]

❌ Bad:

def process(d, f):  # What does this do?
    pass

tmp = criteria.get("s")  # What is s?
x = [u for u in d if u.active]  # What are d and active?

Comments Explain Why, Not What When Code Is Not Obvious(Ousterhout)

Code should be self-documenting for mechanics; comments explain reasoning when the code unclear.

✅ Good:

# Default to ascending created_at if caller sends no sort param
# Why: clients expect chronological order for pagination to be stable
if not sort_field:
    sort_field = "created_at"

# Use 422 rather than 400 for validation failures
# Why: 422 signals the request was well-formed but semantically invalid,
#      which lets clients distinguish schema errors from logic errors
raise HTTPException(status_code=422, detail=errors)

❌ Bad:

# Check if sort_field is empty
if not sort_field:
    sort_field = "created_at"  # Set to created_at

# Raise HTTP exception
raise HTTPException(status_code=422, detail=errors)  # Raise with errors

Write Small, Focused Units (Beck)

Small classes, methods, and functions are easier to understand and modify.

✅ Good:

def build_error_response(errors: list[str]) -> dict:
    """One job: build a structured error payload."""
    return {"ok": False, "errors": errors}

def validate_order(order: OrderCreate) -> list[str]:
    """One job: validate order fields and return error messages."""
    return _check_required_fields(order) + _check_line_items(order)

❌ Bad:

def validate_and_process_and_notify_and_log_order(order, user, db, emailer, logger):
    """Does everything - 500 lines."""
    # Validate order fields
    # Persist to DB
    # Charge payment
    # Send confirmation email
    # Write audit log
    # Return response
    pass

Avoid Deep Nesting (Beck)

Flat code is easier to follow than deeply nested logic.

✅ Good:

def handle_request(request: Request, db: Session):
    # Early returns flatten the logic
    if not request.headers.get("Authorization"):
        return JSONResponse({"error": "missing auth"}, status_code=401)

    body = request.json()
    if not body:
        return JSONResponse({"error": "empty body"}, status_code=400)

    order = parse_order(body)
    if not order:
        return JSONResponse({"error": "invalid order"}, status_code=422)

    return process_order(order, db)

❌ Bad:

def handle_request(request: Request, db: Session):
    if request.headers.get("Authorization"):
        body = request.json()
        if body:
            order = parse_order(body)
            if order:
                return process_order(order, db)
            else:
                return JSONResponse({"error": "invalid order"}, status_code=422)
        else:
            return JSONResponse({"error": "empty body"}, status_code=400)
    else:
        return JSONResponse({"error": "missing auth"}, status_code=401)

Extract Explaining Variables (Beck)

Break complex expressions into named intermediate steps.

✅ Good:

is_bulk_order = len(order.line_items) > 50
requires_approval = order.total_amount > APPROVAL_THRESHOLD
recipient_emails = [u.email for u in order.notify_users if u.email_verified]

if requires_approval:
    notify_approvers(order, recipient_emails)

❌ Bad:

if order.total_amount > APPROVAL_THRESHOLD:
    notify_approvers(
        order,
        [u.email for u in order.notify_users if u.email_verified],
    )

---

4. Testability

Constructor Should Not Do Work (Hevery)

Constructors that perform operations are difficult to test.

✅ Good:

class DataLoader:
    def __init__(self, config: Dict):
        """Just store config - no work."""
        self.config = config

    def load(self) -> list[dict]:
        """Work happens in methods, not constructor."""
        return connect_to_db(self.config)

❌ Bad:

class DataLoader:
    def __init__(self, config: Dict):
        """Constructor does work - hard to test."""
        self.connection = connect_to_db(config)  # Network call in constructor!
        self.schema = self._fetch_schema(self.connection)

Test Behaviors, Not Implementation (Hevery)

Tests should verify what the code does, not how it does it.

✅ Good:

def test_filter_users_excludes_inactive():
    """Tests the behavior users care about."""
    users = [
        User(id=1, status="active"),
        User(id=2, status="inactive"),
        User(id=3, status="active"),
    ]

    result = filter_users(users, criteria={"status": "active"})

    assert all(u.status == "active" for u in result)

❌ Bad:

def test_filter_users_calls_datetime_parse():
    """Tests implementation details - brittle."""
    users = create_test_users()
    criteria = {"created_after": "2024-01-01"}

    with mock.patch('myapp.services.datetime.fromisoformat') as mock_dt:
        filter_users(users, criteria)
        assert mock_dt.called  # Who cares if it called this?

Tests Should Be Proximate to What They’re Testing

Keep tests close to the code they test for easy maintenance.

✅ Good File Structure:

order_service/
├── app/
│   ├── routers/
│   │   ├── orders.py
│   │   └── orders_test.py          # Tests next to code
│   └── services/
│       ├── order_processing.py
│       └── order_processing_test.py    # Tests next to code

❌ Bad File Structure:

order_service/
├── app/
│   ├── routers/
│   │   └── orders.py
│   └── services/
│       └── order_processing.py
└── tests/                             # Tests far from code
    ├── unit/
    │   └── test_everything.py         # Hard to find
    └── integration/

Make Tests Independent with One Assertion Each (Beck)

Each test should run successfully regardless of other tests and document expected behavior clearly.

✅ Good:

def test_filter_by_single_status():
    users = create_sample_users()
    result = filter_users(users, {"status": ["active"]})
    assert all(u.status == "active" for u in result)

def test_filter_by_multiple_statuses():
    users = create_sample_users()
    result = filter_users(users, {"status": ["active", "pending"]})
    assert all(u.status in {"active", "pending"} for u in result)

def test_filter_by_status_and_role():
    users = create_sample_users()
    result = filter_users(users, {"status": ["active"], "role": ["admin"]})
    assert all(u.status == "active" for u in result)
    assert all(u.role == "admin" for u in result)

❌ Bad:

def test_all_user_filters():
    """Tests too much - if it fails, what broke?"""
    users = create_sample_users()

    result1 = filter_users(users, {"status": ["active"]})
    assert len(result1) == 10

    result2 = filter_users(users, {"role": ["admin"]})
    assert len(result2) == 5

    result3 = filter_users(users, {"status": ["active"], "role": ["admin"]})
    assert len(result3) == 2
    # If this fails, which filter broke?

---

5. Project-Specific Standards

File Organization

order_service/
├── app/
│   ├── config/              # Configuration and settings
│   ├── routers/             # FastAPI routers (one file per resource)
│   │   ├── orders.py        # Route handlers + helpers + tests
│   │   └── users.py
│   ├── shared/              # Components used in 2+ modules
│   │   └── auth.py
│   └── services/            # Pure business logic functions
│       └── order_processing.py
├── migrations/              # Alembic migrations
├── docs/                    # Documentation
└── tests/                   # Integration/E2E tests only

Import Organization

"""Module docstring explaining purpose."""

# Standard library
import os
from typing import Optional, List, Dict, Any

# Third-party
from fastapi import APIRouter, Depends, HTTPException
from sqlalchemy.orm import Session
from pydantic import BaseModel

# Local
from app.config import get_logger
from app.services.order_processing import process_order

Type Hints

Use type hints for all function signatures:

def filter_orders(
    filters: Optional[Dict[str, Any]]
) -> list[Order]:
    """Apply filters and return matching orders."""
    pass

Logging

Inject logger as dependency, use appropriate levels:

def process_order(order: Order, logger: Logger = get_logger(__name__)):
    logger.debug(f"Processing order {order.id}")  # Detailed info
    logger.info("Order processing complete")        # Normal flow
    logger.warning("Payment retry required")        # Unexpected but handled
    logger.error("Processing failed", exc_info=True) # Errors with traceback

Error Handling

Use early returns and specific exceptions:

def create_order(data: Dict, db: Session) -> Order:
    # Early return for invalid input
    if not data.get("line_items"):
        logger.warning("No line items provided")
        raise HTTPException(status_code=422, detail="Order must have at least one line item")

    try:
        order = Order(**data)
        db.add(order)
        db.commit()
        return order
    except IntegrityError as e:
        logger.error(f"Duplicate order reference: {e}")
        raise HTTPException(status_code=409, detail="Order reference already exists")
    except SQLAlchemyError as e:
        logger.error(f"Order creation failed: {e}", exc_info=True)
        raise HTTPException(status_code=500, detail="Failed to persist order")

---

6. Miscellaneous

Exception Handling is Part of Normal Flow (Ousterhout)

Design error handling as carefully as happy path code.

def load_user(user_id: int) -> User:
    """
    Load a user from the database.

    Why: Errors are expected in data access - handle gracefully.
    """
    try:
        return db.query(User).filter(User.id == user_id).one()
    except NoResultFound:
        logger.warning(f"User not found: {user_id}")
        raise HTTPException(status_code=404, detail="User not found")
    except OperationalError as e:
        logger.error(f"DB connection error fetching user {user_id}: {e}")
        raise HTTPException(status_code=503, detail="Database unavailable")

Better to Be Wrong Than Vague (Ousterhout)

Specific but incorrect statements can be corrected; vague ones can’t.

✅ Good:

# Show recipient names when few (≤3), show count when many (>3).
# This balances detail with readability in notification payloads.
if len(recipients) <= 3:
    return {"notified": [r.email for r in recipients]}
else:
    return {"notified_count": len(recipients)}

❌ Bad:

# Format recipients nicely
if len(recipients) <= threshold:  # What threshold? Why?
    return format_recipients(recipients)  # How?

Red-Green-Refactor Cycle (Beck)

1. Red: Write a failing test
2. Green: Make it pass (even if ugly)
3. Refactor: Clean up while keeping tests green

# 1. RED - Write failing test
def test_filter_by_status():
    orders = create_orders()
    result = filter_orders(orders, {"status": ["pending"]})
    assert all(o.status == "pending" for o in result)

# 2. GREEN - Make it pass
def filter_orders(orders, filters):
    if "status" in filters:
        return [o for o in orders if o.status in filters["status"]]
    return orders

# 3. REFACTOR - Clean up
def filter_orders(
    filters: Optional[Dict[str, Any]],
    db: Session = Depends(get_db),
) -> list[Order]:
    """Apply filters using data-driven approach."""
    query = db.query(Order)
    for field, column in FIELD_MAP:
        values = (filters or {}).get(field, [])
        if values:
            query = query.filter(getattr(Order, column).in_(values))
    return query.all()

---

Quick Reference Checklist

Before committing code, ask yourself:

• Performant: Does this provide deep functionality behind a simple interface?
• Changeable: Can I add features without rewriting existing code?
• Readable: Would a new team member understand this in 6 months?
• Testable: Can I test this function without complex setup?
• DRY: Did I eliminate duplication using data-driven patterns?
• Names: Are variable and function names precise and meaningful?
• Comments: Do comments explain WHY, not WHAT?
• Nesting: Did I use early returns to flatten logic?
• Tests: Are tests proximate, independent, and behavioral?
• Types: Do all public functions have type hints?
• Errors: Is error handling part of normal flow?

---

Resources

• Ousterhout: A Philosophy of Software Design
• Beck: Tidy First?
• Hevery: Guide: Writing Testable Code

---

Remember: These are principles, not rules. Use judgment to apply them appropriately for each situation.