# Code Smells and Heuristics

Comprehensive catalog of code smells organized by category, with identification criteria and targeted refactorings. Based on Robert C. Martin's *Clean Code*, Chapter 17.


## Table of Contents
1. [What Is a Code Smell?](#what-is-a-code-smell)
2. [Comment Smells](#comment-smells)
3. [Environment Smells](#environment-smells)
4. [Function Smells](#function-smells)
5. [General Smells](#general-smells)
6. [Naming Smells](#naming-smells)
7. [Test Smells](#test-smells)
8. [Smell Detection Quick Reference](#smell-detection-quick-reference)

---

## What Is a Code Smell?

A code smell is a surface indication that usually corresponds to a deeper problem in the system. Smells are not bugs -- the code works. But they suggest fragility, rigidity, or unnecessary complexity that will cause problems as the codebase evolves. Learn to recognize smells quickly and you'll know where to focus refactoring effort.

---

## Comment Smells

Comments that indicate problems in the code:

### C1: Inappropriate Information

Comments that hold information better kept in other systems.

| Information type | Where it belongs | Not in comments |
|-----------------|-----------------|-----------------|
| Change history | Git log | Not `// Changed by John on 2024-01-15` |
| Author attribution | `git blame` | Not `// Author: john@company.com` |
| Issue tracking | Jira/GitHub Issues | Not `// Fixes bug #1234` (use commit message) |
| Build instructions | README or Makefile | Not `// Run with -Xmx512m flag` |

**Comments should only contain technical notes about the code itself.**

### C2: Obsolete Comments

Comments that have grown old and are no longer accurate. Code evolves; comments don't always follow.

```java
// BAD: This comment is now a lie
// Returns the total price including 5% tax
public double getTotal() {
    return subtotal * 1.08; // Tax rate changed to 8%, comment not updated
}
```

**Fix:** Delete the comment. If the information is important, express it in the code:

```java
private static final double TAX_RATE = 0.08;

public double getTotal() {
    return subtotal * (1 + TAX_RATE);
}
```

### C3: Redundant Comments

Comments that describe something that adequately describes itself.

```java
i++; // increment i
```

If the code is clear, the comment is noise. If the code is not clear, fix the code.

### C4: Poorly Written Comments

If you're going to write a comment, take the time to write it well. Don't ramble. Don't state the obvious. Use correct grammar and punctuation. Be brief and precise.

### C5: Commented-Out Code

```python
# result = old_algorithm(data)
# if result > threshold:
#     notify_admin(result)
result = new_algorithm(data)
```

Commented-out code rots. Others are afraid to delete it ("maybe someone needs it?"). It accumulates. **Delete it.** Version control is your safety net.

---

## Environment Smells

### E1: Build Requires More Than One Step

You should be able to build the entire system with a single command.

```bash
# GOOD: One command
make build
# or
npm run build
# or
./gradlew build
```

If building requires checkout, install, configure, set env vars, download dependencies separately, the build is fragile and developers will avoid running it.

### E2: Tests Require More Than One Step

You should be able to run all the tests with a single command.

```bash
# GOOD: One command
make test
# or
npm test
# or
pytest
```

If running tests requires starting databases, setting up fixtures manually, or running scripts in a specific order, developers won't run them.

---

## Function Smells

### F1: Too Many Arguments

More than three arguments is a smell. Arguments are hard to understand, hard to remember, and hard to test (every combination of arguments is a test case).

| Arguments | Assessment | Action |
|-----------|------------|--------|
| 0 | Ideal | Keep as is |
| 1 | Good | Common and clear |
| 2 | Acceptable | Ensure natural pairing |
| 3 | Questionable | Can any be grouped into an object? |
| 4+ | Refactor | Introduce parameter object or builder |

**Refactoring:**
```python
# BAD: Too many arguments
def create_user(first_name, last_name, email, phone, address, city, state, zip_code):
    ...

# GOOD: Parameter object
def create_user(personal_info: PersonalInfo, address: Address):
    ...
```

### F2: Output Arguments

Arguments that are modified by the function are confusing. Is the argument input or output?

```java
// BAD: Is report being read or modified?
appendFooter(report);

// GOOD: Method on the object being modified
report.appendFooter();
```

In general, output arguments should be avoided. If a function must change the state of something, have it change the state of the object it is called on.

### F3: Flag Arguments

Boolean arguments loudly declare that the function does two things.

```python
# BAD: Flag argument
def create_account(user_data, is_admin):
    if is_admin:
        # 15 lines of admin setup
    else:
        # 15 lines of regular setup

# GOOD: Separate functions
def create_admin_account(user_data):
    ...

def create_user_account(user_data):
    ...
```

### F4: Dead Functions

Functions that are never called should be deleted. Don't keep them around "just in case." Your version control system remembers them if you ever need them back.

**How to find dead functions:**
- IDE "Find Usages" reports zero callers
- Static analysis tools flag unreachable code
- Code coverage reports show 0% coverage
- Search for the function name across the codebase

---

## General Smells

### G1: Multiple Languages in One Source File

A single source file should contain one language. Mixing HTML, JavaScript, CSS, SQL, and server-side code in one file creates confusion.

| Acceptable | Problematic |
|------------|-------------|
| JavaScript in a `.js` file | SQL strings embedded in Java |
| CSS in a `.css` file | HTML templates inline in Python |
| SQL in a `.sql` migration file | CSS-in-JS with complex logic |

**Minimize** the extent and number of extra languages in source files.

### G2: Obvious Behavior Is Not Implemented

When the obvious behavior of a function is not implemented, readers lose trust in the author.

```python
# SURPRISING: dayOfWeek("Monday") should obviously return Day.MONDAY
def day_of_week(name):
    days = {"Monday": 1, "Tuesday": 2, ...}  # Missing "MONDAY", "monday"
    return days[name]  # Crashes on case variants
```

Follow the Principle of Least Surprise. Users and callers should not be surprised by what a function does.

### G3: Incorrect Behavior at the Boundaries

Don't rely on your intuition for boundary cases. **Write tests for every boundary condition.** Things that commonly fail at boundaries:

| Boundary | Common failure |
|----------|---------------|
| Empty input | NullPointerException, IndexOutOfBounds |
| Single element | Off-by-one in loops |
| Maximum capacity | Buffer overflow, performance cliff |
| Negative values | Unexpected results in calculations |
| Unicode | Encoding issues, wrong string length |
| Concurrent access | Race conditions, deadlocks |

### G4: Overridden Safeties

Turning off warnings, suppressing exceptions, or disabling tests is dangerous.

```python
# BAD: Ignoring the warning doesn't fix the problem
@SuppressWarnings("unchecked")  # Why is this unchecked?
warnings.filterwarnings("ignore")  # What are we hiding?
@pytest.mark.skip("Flaky")  # Fix it instead of skipping
```

Turning off compiler warnings or ignoring failing tests is like ignoring a check-engine light.

### G5: Duplication (DRY Violations)

Duplication is the single most important smell. Every instance represents a missed opportunity for abstraction.

| Duplication type | How to find it | Refactoring |
|-----------------|----------------|-------------|
| **Exact clones** | Copy-paste detection tools | Extract shared function |
| **Structural clones** | Same algorithm, different data | Template Method or Strategy pattern |
| **Conditional chains** | Repeated `if/else` or `switch` | Polymorphism |
| **Cross-module** | Same logic in multiple modules | Extract shared library or module |

**The Rule of Three:** First instance: just write it. Second instance: note the duplication. Third instance: refactor.

### G6: Code at Wrong Level of Abstraction

Functions and classes should operate at a single level of abstraction. Mixing high-level business logic with low-level implementation details creates confusion.

```python
# BAD: Mixed abstraction levels
def process_order(order):
    # High-level business logic
    validate_order(order)

    # Suddenly low-level database details
    connection = psycopg2.connect(host="db.example.com", port=5432)
    cursor = connection.cursor()
    cursor.execute("INSERT INTO orders ...")
    connection.commit()

    # Back to high-level
    send_confirmation(order)

# GOOD: Consistent abstraction level
def process_order(order):
    validate_order(order)
    order_repository.save(order)
    send_confirmation(order)
```

### G7: Feature Envy

A method that uses more features of another class than its own class has "feature envy." It wants to be somewhere else.

```python
# BAD: This method envies the Order class
class ReportGenerator:
    def calculate_order_summary(self, order):
        subtotal = sum(item.price * item.quantity for item in order.items)
        tax = subtotal * order.tax_rate
        shipping = order.weight * order.shipping_rate
        return subtotal + tax + shipping

# GOOD: Move the method to where the data lives
class Order:
    def calculate_total(self):
        subtotal = sum(item.price * item.quantity for item in self.items)
        tax = subtotal * self.tax_rate
        shipping = self.weight * self.shipping_rate
        return subtotal + tax + shipping
```

### G8: Selector Arguments

Arguments used to select behavior (not just booleans, but enums and strings too).

```python
# BAD: Selector argument
def calculate(operation, a, b):
    if operation == "add": return a + b
    if operation == "subtract": return a - b
    if operation == "multiply": return a * b

# GOOD: Separate functions
def add(a, b): return a + b
def subtract(a, b): return a - b
def multiply(a, b): return a * b
```

### G9: Obscured Intent

Code that is designed to be compact at the expense of clarity.

```python
# BAD: What does this do?
def m(a):return sum(1 for c in a if c.s=='A'and c.d<dt.now()-td(30))

# GOOD: Clear intent
def count_recently_active_customers(customers):
    thirty_days_ago = datetime.now() - timedelta(days=30)
    return sum(
        1 for customer in customers
        if customer.status == 'ACTIVE'
        and customer.last_active_date < thirty_days_ago
    )
```

### G10: Magic Numbers

Raw numeric literals scattered through the code.

```python
# BAD: What do these numbers mean?
if len(password) < 8:
    raise ValueError("Too short")
time.sleep(86400)
price = amount * 0.08

# GOOD: Named constants explain intent
MIN_PASSWORD_LENGTH = 8
SECONDS_PER_DAY = 86400
SALES_TAX_RATE = 0.08

if len(password) < MIN_PASSWORD_LENGTH:
    raise ValueError(f"Password must be at least {MIN_PASSWORD_LENGTH} characters")
time.sleep(SECONDS_PER_DAY)
price = amount * SALES_TAX_RATE
```

### G11: Dead Code

Code that is never executed: unreachable conditions, unused variables, functions with no callers, impossible `catch` blocks.

**Types of dead code:**
- Conditions that can never be true
- `catch` blocks for exceptions that are never thrown
- Variables that are assigned but never read
- Functions that are never called
- Entire modules with no imports

**Fix:** Delete it. Every line of dead code is a line someone has to read, wonder about, and maintain. Version control is the safety net.

---

## Naming Smells

### N1: Choosing Descriptive Names

Names should be descriptive. Don't settle for the first name that comes to mind. Take time to choose a name that is as descriptive and unambiguous as possible.

### N2: Choosing Names at the Appropriate Level of Abstraction

Don't choose names that communicate implementation. Choose names that reflect the level of abstraction of the class or function you are working in.

```python
# BAD: Implementation-level name
class FTPFileDownloader:  # What if we switch to HTTP?

# GOOD: Abstraction-level name
class FileDownloader:  # The how is an implementation detail
```

### N3: Using Standard Nomenclature Where Possible

Use names from well-known patterns and conventions:
- `Factory`, `Strategy`, `Visitor`, `Iterator` for design patterns
- `Repository`, `Service`, `Controller` for architectural layers
- Domain terms from the business (Ubiquitous Language from DDD)

### N4: Unambiguous Names

Choose names that make the function or variable's workings unambiguous.

```python
# BAD: Ambiguous
def rename(old, new):  # Rename what? A file? A user? A variable?

# GOOD: Unambiguous
def rename_file(old_path, new_path)
```

### N5: Use Long Names for Long Scopes

The length of a name should be proportional to the size of the scope that contains it.

| Scope | Name length | Example |
|-------|-------------|---------|
| 1-line lambda | 1 char | `x` in `items.map(x => x.id)` |
| 5-line method | Short | `i`, `sum`, `item` |
| Class field | Medium | `retryCount`, `lastUpdate` |
| Module constant | Long | `MAX_CONNECTION_POOL_SIZE` |
| Global/public API | Very long | `DEFAULT_SESSION_TIMEOUT_MINUTES` |

### N6: Avoid Encodings

Don't use Hungarian notation, member prefixes, or interface prefixes.

| Encoding | Example | Better |
|----------|---------|--------|
| Hungarian | `strName`, `iCount` | `name`, `count` |
| Member prefix | `m_name`, `_name` | `name` (context is the class) |
| Interface prefix | `IUserService` | `UserService` (implementations get suffix: `UserServiceImpl`) |
| Type suffix | `nameString` | `name` |

---

## Test Smells

### T1: Insufficient Tests

A test suite should test everything that could possibly break. Test every condition, every boundary, every edge case.

### T2: Using a Coverage Tool

Code coverage tools report which lines are not tested. Use them as a guide, not a goal. 100% line coverage does not mean 100% correctness, but untested lines definitely contain potential bugs.

### T3: Don't Skip Trivial Tests

Trivial tests are easy to write and their documentary value is higher than the cost of writing them.

### T4: An Ignored Test Is a Question About an Ambiguity

If requirements are unclear, write the test with `@skip` and a note about what's uncertain. A skipped test is a question waiting to be answered.

### T5: Test Boundary Conditions

Boundaries are where bugs cluster. Test all edges: empty input, one element, max capacity, off-by-one, overflow.

### T6: Exhaustively Test Near Bugs

When you find a bug in a function, don't just fix it. Test the function exhaustively. Bugs tend to congregate -- if there's one, there are likely others nearby.

### T7: Patterns of Failure Are Revealing

If tests fail in a pattern (all tests with dates fail, all tests with unicode fail), the pattern reveals the nature of the bug. Use this diagnostic information.

### T8: Test Coverage Patterns Can Be Revealing

Look at which code is not covered. If a complex conditional has untested branches, those branches likely contain bugs.

### T9: Tests Should Be Fast

Slow tests don't get run. A test suite that takes 30 minutes will be run once a day at best. A suite that takes 10 seconds will be run after every change.

---

## Smell Detection Quick Reference

| Category | Key Smells | First Action |
|----------|-----------|--------------|
| **Comments** | Obsolete, redundant, commented-out code | Delete the comment; improve the code |
| **Environment** | Multi-step build or test | Script to single command |
| **Functions** | Too many args, flag args, dead functions | Extract object, split function, delete |
| **General** | Duplication, wrong abstraction level, feature envy | Extract, move, consolidate |
| **Names** | Ambiguous, wrong level, encoded | Rename to reveal intent |
| **Tests** | Insufficient, slow, skipped, no boundaries | Add tests, mock dependencies, fix or delete |

**Remember:** Not every smell requires immediate action. Use professional judgment. A smell in frequently-changed code demands attention. A smell in stable code that never changes may not be worth the risk of refactoring.

# Comments and Formatting

Comprehensive guide to comment discipline and code formatting. Based on Robert C. Martin's *Clean Code*, Chapters 4 and 5.


## Table of Contents
1. [The Truth About Comments](#the-truth-about-comments)
2. [Good Comments](#good-comments)
3. [Bad Comments](#bad-comments)
4. [Formatting](#formatting)
5. [When Comments Are Truly Necessary](#when-comments-are-truly-necessary)

---

## The Truth About Comments

**Don't comment bad code -- rewrite it.** Comments are, at best, a necessary evil. The proper use of comments is to compensate for our failure to express ourselves in code. Every time you write a comment, you should grimace and feel the failure of your ability of expression.

Comments lie. Not always, and not intentionally, but too often. Code changes and evolves; comments don't always follow. The older a comment is and the farther it is from the code it describes, the more likely it is to be wrong.

---

## Good Comments

Not all comments are bad. Some are necessary and valuable. Here are the types worth writing:

### Legal Comments

Copyright and license headers mandated by corporate or legal standards.

```java
// Copyright (c) 2024 Acme Corp. All rights reserved.
// Licensed under the Apache License, Version 2.0
```

Keep them short. Reference a standard license file rather than embedding the full text.

### Informative Comments

Provide information that cannot be expressed in the code itself.

```java
// Format: kk:mm:ss EEE, MMM dd, yyyy
Pattern timeMatcher = Pattern.compile("\\d*:\\d*:\\d* \\w*, \\w* \\d*, \\d*");
```

Even here, a named constant or custom type could eliminate the need: `TIMESTAMP_PATTERN`.

### Explanation of Intent

Explain *why* a decision was made, not *what* the code does.

```python
# We sort by creation date descending because the business requirement
# specifies that the most recently created items appear first in the
# dashboard, even though alphabetical would be more intuitive.
items.sort(key=lambda x: x.created_at, reverse=True)
```

This is valuable because the *what* is visible in the code, but the *why* would otherwise be lost.

### Warning of Consequences

Alert other developers about consequences that are not obvious.

```java
// Don't run this test in CI -- it takes 45 minutes and requires
// a live connection to the production payment gateway
@Ignore("Long-running integration test requiring production access")
public void testLivePaymentGateway() { ... }
```

### TODO Comments

Mark work that needs to be done but cannot be done right now.

```python
# TODO(#1234): Replace with proper caching once Redis is provisioned
def get_user_preferences(user_id):
    return db.query(f"SELECT * FROM preferences WHERE user_id = {user_id}")
```

**Rules for TODOs:**
- Include a ticket number or issue reference
- Scan and resolve them regularly (they are not permanent)
- Never use TODO as an excuse to leave broken code
- IDE/linter plugins can track and report outstanding TODOs

### Amplification

Emphasize the importance of something that might otherwise seem inconsequential.

```java
String listItemContent = match.group(3).trim();
// The trim is critically important. It removes trailing whitespace
// that would cause the item to be recognized as another list.
new ListItemWidget(this, listItemContent, this.level + 1);
```

---

## Bad Comments

Most comments fall into these categories and should be eliminated:

### Mumbling

Comments written because "you should comment" rather than because they add value.

```java
// The processor
private Processor processor;

// Default constructor
public MyClass() { }
```

These say nothing. Delete them.

### Redundant Comments

Comments that take longer to read than the code they describe.

```java
// Returns the day of the month
public int getDayOfMonth() {
    return dayOfMonth;
}

// Check if the employee is eligible for benefits
if (employee.isEligibleForBenefits()) { ... }
```

The code already says exactly this. The comment is noise.

### Misleading Comments

Comments that are subtly incorrect -- the most dangerous kind.

```java
// Returns true if the user is active
public boolean isActive() {
    return lastLoginDate != null && !isDeleted && subscriptionEndDate.isAfter(now());
}
```

The comment says "active" means logged in before. The code checks three conditions. When the code changes and the comment doesn't, a future developer will be misled.

### Mandated Comments

Rules that require every function or variable to have a comment produce noise.

```java
/**
 * The name.
 * @param name The name.
 */
public void setName(String name) {
    this.name = name;
}
```

This adds no information. It's clutter that developers learn to ignore, which means they'll also ignore the few comments that actually matter.

### Journal Comments

Changelog entries at the top of files.

```java
// 2024-01-15 - Added validation for email format
// 2024-01-20 - Fixed bug in email regex
// 2024-02-01 - Added support for international emails
```

This is what version control is for. `git log` and `git blame` provide this information with more accuracy and context.

### Commented-Out Code

```python
# user_cache = {}
# def get_cached_user(user_id):
#     if user_id not in user_cache:
#         user_cache[user_id] = db.get_user(user_id)
#     return user_cache[user_id]

def get_user(user_id):
    return db.get_user(user_id)
```

Other developers are afraid to delete commented-out code because they think it must be there for a reason. It accumulates like barnacles. **Delete it.** Version control has perfect memory.

### Noise Comments

Comments that restate the obvious in a different form.

```java
/** Default constructor */
protected AnnualDateRule() { }

/** The day of the month */
private int dayOfMonth;

/** Returns the day of the month
 * @return the day of the month */
public int getDayOfMonth() { return dayOfMonth; }
```

Every one of these is noise. They provide no information, train developers to ignore comments, and create a false sense of documentation thoroughness.

### Position Markers and Banners

```java
// ========== PRIVATE METHODS ==========
// --- Validation ---
// /////// Constructor ///////
```

If your file is so large that you need position markers, the file is too large. Extract classes instead of adding banners.

### Closing Brace Comments

```java
if (condition) {
    while (running) {
        for (item : items) {
            // ... lots of code ...
        } // for
    } // while
} // if
```

If you need comments to track closing braces, your function is too long. Extract methods until the structure is obvious.

### Attribution Comments

```java
// Added by: john.smith@company.com
```

Version control tracks authorship more reliably. Use `git blame`.

---

## Formatting

### Why Formatting Matters

Code formatting is about communication, and communication is the professional developer's first order of business. The formatting of your code communicates important information long after the original developer has moved on.

### Vertical Formatting

#### The Newspaper Metaphor

Source files should be organized like a newspaper article:
- **Name** should be simple but explanatory (the headline)
- **Top** should provide high-level concepts and algorithms (the synopsis)
- **Bottom** should contain the lowest-level functions and details (the body)

#### Vertical Openness Between Concepts

Each group of related lines represents a complete thought. Separate thoughts with blank lines.

```python
# GOOD: Blank lines separate concepts
import os
import sys

from myapp.models import User
from myapp.services import EmailService


class UserRegistration:

    def __init__(self, email_service):
        self.email_service = email_service

    def register(self, name, email):
        user = User.create(name=name, email=email)
        self.email_service.send_welcome(user)
        return user
```

```python
# BAD: Everything runs together
import os
import sys
from myapp.models import User
from myapp.services import EmailService
class UserRegistration:
    def __init__(self, email_service):
        self.email_service = email_service
    def register(self, name, email):
        user = User.create(name=name, email=email)
        self.email_service.send_welcome(user)
        return user
```

#### Vertical Density

Lines that are tightly related should appear close together vertically. Don't insert blank lines between closely related lines.

```java
// BAD: Useless comments break vertical density
public class ReporterConfig {

    /**
     * The class name of the reporter listener
     */
    private String className;

    /**
     * The properties of the reporter listener
     */
    private List<Property> properties = new ArrayList<>();
}

// GOOD: Dense, related declarations together
public class ReporterConfig {
    private String className;
    private List<Property> properties = new ArrayList<>();
}
```

#### Vertical Distance

Closely related concepts should be kept vertically close to each other. Don't force the reader to hop around the file.

**Rules:**
- **Local variables:** Declare at the top of the function or as close to first usage as practical
- **Instance variables:** Declare at the top of the class (everyone needs to know about them)
- **Dependent functions:** The caller should be above the callee, and they should be close
- **Conceptual affinity:** Functions that do similar things or operate on the same data should be near each other

#### Vertical Ordering

Function call dependencies should point downward: a function that is called should be below the function that calls it. This creates a nice flow from high-level to low-level, like reading a newspaper.

### Horizontal Formatting

#### Line Length

**Keep lines short.** The old 80-character limit is a reasonable guideline. Modern screens can show more, but readability drops beyond 100-120 characters. Scrolling horizontally breaks the reader's flow.

#### Horizontal Openness and Density

Use whitespace to associate strongly related things and disassociate weakly related things.

```java
// Spaces around assignment (weak association between sides)
int lineCount = countLines();

// No space between function name and parenthesis (strong association)
lineCount = countLines();

// Spaces around binary operators by precedence
return b*b - 4*a*c;  // Multiplication is higher precedence, tighter
return (-b + determinant) / (2*a);
```

#### Indentation

Indentation makes the scope hierarchy visible. Each level of nesting gets one indentation level. **Never break this rule, even for short `if` statements or tiny loops.**

```java
// BAD: Collapsed scopes hide structure
if (condition) return true;

// GOOD: Indentation preserved
if (condition) {
    return true;
}
```

### Team Rules

**A team should agree on a single formatting style and everyone should use it.** Individual style preferences must yield to the team standard.

The best way to enforce team rules:

| Approach | Tool examples | Benefit |
|----------|---------------|---------|
| **Automated formatter** | Prettier, Black, gofmt, rustfmt | Eliminates all style debates |
| **Linter with auto-fix** | ESLint, Pylint, RuboCop | Catches style and quality issues |
| **Pre-commit hooks** | Husky, pre-commit, lefthook | Prevents style violations from entering repo |
| **CI enforcement** | Format check in pipeline | Catches anything hooks miss |
| **EditorConfig** | `.editorconfig` file | Consistent settings across editors |

**The best formatting rule:** Use an automated formatter and never think about formatting again. Time spent debating tabs versus spaces is time not spent writing clean code.

---

## When Comments Are Truly Necessary

Despite the general advice to minimize comments, certain situations genuinely require them:

| Situation | Why code alone isn't enough | Example |
|-----------|---------------------------|---------|
| **Regulatory requirement** | Law/compliance requires documentation | HIPAA, SOX, GDPR compliance notes |
| **Non-obvious performance choice** | Algorithm choice isn't self-evident | "Using radix sort here because n > 10M and keys are bounded" |
| **External system quirk** | Workaround for third-party bug | "API returns 200 for errors; we check response body instead" |
| **Concurrency rationale** | Threading decisions need explanation | "Double-checked locking required here because..." |
| **Domain formula** | Mathematical formula from spec | "Amortization formula from IRS Publication 936" |
| **Public API contract** | Users cannot read implementation | Javadoc for library interfaces |

The key: comments should explain *why*, never *what*. If you find yourself explaining *what* the code does, the code needs to be clearer, not the comment.

# Error Handling

Comprehensive guide to writing clean error handling that keeps business logic readable. Based on Robert C. Martin's *Clean Code*, Chapter 7.


## Table of Contents
1. [The Core Problem](#the-core-problem)
2. [Use Exceptions, Not Return Codes](#use-exceptions-not-return-codes)
3. [Write Your Try-Catch-Finally Statement First](#write-your-try-catch-finally-statement-first)
4. [Use Unchecked Exceptions](#use-unchecked-exceptions)
5. [Provide Context with Exceptions](#provide-context-with-exceptions)
6. [Define Exception Classes in Terms of the Caller's Needs](#define-exception-classes-in-terms-of-the-callers-needs)
7. [Don't Return Null](#dont-return-null)
8. [Don't Pass Null](#dont-pass-null)
9. [Error Handling Patterns Summary](#error-handling-patterns-summary)
10. [Common Error Handling Anti-Patterns](#common-error-handling-anti-patterns)

---

## The Core Problem

Error handling is important, but if it obscures logic, it's wrong. Code that mixes business logic with error handling is hard to read, test, and maintain. The goal is to write code where the happy path reads cleanly and error handling is a separate, well-organized concern.

---

## Use Exceptions, Not Return Codes

Return codes force the caller to check immediately after the call, cluttering the calling code with error-checking logic.

### Before: Return Codes

```java
// BAD: Error checking clutters the logic
public class DeviceController {
    public void sendShutDown() {
        DeviceHandle handle = getHandle(DEV1);
        if (handle != DeviceHandle.INVALID) {
            DeviceRecord record = retrieveDeviceRecord(handle);
            if (record.getStatus() != DEVICE_SUSPENDED) {
                pauseDevice(handle);
                clearDeviceWorkQueue(handle);
                closeDevice(handle);
            } else {
                logger.log("Device suspended. Unable to shut down.");
            }
        } else {
            logger.log("Invalid handle for: " + DEV1.toString());
        }
    }
}
```

### After: Exceptions

```java
// GOOD: Business logic is clean; errors handled separately
public class DeviceController {
    public void sendShutDown() {
        try {
            tryToShutDown();
        } catch (DeviceShutDownError e) {
            logger.log(e);
        }
    }

    private void tryToShutDown() throws DeviceShutDownError {
        DeviceHandle handle = getHandle(DEV1);
        DeviceRecord record = retrieveDeviceRecord(handle);
        pauseDevice(handle);
        clearDeviceWorkQueue(handle);
        closeDevice(handle);
    }
}
```

The business logic (shut down sequence) is now visible without wading through error checks.

---

## Write Your Try-Catch-Finally Statement First

Try-catch blocks define a scope within your program. The code in the `try` block can abort at any point and resume in the `catch`. This makes try blocks like transactions: the `catch` must leave the program in a consistent state.

**Practice:** When writing code that could throw exceptions, start with the try-catch-finally. This helps you define what the caller can expect, regardless of what goes wrong.

```python
# Start with the structure
def load_configuration(path):
    try:
        content = read_file(path)
        config = parse_yaml(content)
        validate_config(config)
        return config
    except FileNotFoundError:
        return default_configuration()
    except ParseError as e:
        raise ConfigurationError(f"Invalid config at {path}: {e}")
    finally:
        log_config_load_attempt(path)
```

---

## Use Unchecked Exceptions

Checked exceptions (Java's `throws` clause) violate the Open/Closed Principle. If you throw a checked exception from a low-level function, every function in the call chain between the throw and the catch must declare that exception. A single change at a low level forces signature changes all the way up.

| Aspect | Checked exceptions | Unchecked exceptions |
|--------|-------------------|---------------------|
| **Coupling** | Every caller must declare or catch | Only relevant callers catch |
| **Encapsulation** | Low-level details leak to high-level | Abstraction layers maintained |
| **Refactoring** | Adding new exception type cascades changes | New exceptions don't affect existing callers |
| **When appropriate** | Critical library APIs where caller MUST handle | Application code, most library code |

**In practice:** Use unchecked exceptions for application code. The cost of checked exceptions in dependency management outweighs their documentary benefit.

---

## Provide Context with Exceptions

Each exception should provide enough context to determine the source and location of the error.

### What to Include

| Context element | Why | Example |
|----------------|-----|---------|
| **Operation that failed** | Identifies what was attempted | "Failed to save invoice" |
| **Input that caused failure** | Enables reproduction | "Invoice #1234 for customer 'Acme'" |
| **Constraint that was violated** | Explains why it failed | "Total amount exceeds maximum of $1,000,000" |
| **Suggested recovery** | Helps caller respond | "Retry after 30 seconds" or "Check network connection" |

### Implementation Pattern

```python
# BAD: No context
raise ValueError("Invalid input")

# BAD: Raw technical error
raise Exception(str(e))

# GOOD: Contextual error message
raise InvoiceValidationError(
    f"Cannot create invoice for customer '{customer.name}': "
    f"requested amount ${amount:.2f} exceeds credit limit "
    f"of ${customer.credit_limit:.2f}"
)
```

### Custom Exception Classes

```python
class OrderError(Exception):
    """Base exception for order processing."""
    pass

class InsufficientInventoryError(OrderError):
    def __init__(self, product, requested, available):
        self.product = product
        self.requested = requested
        self.available = available
        super().__init__(
            f"Cannot fulfill order: {product.name} has "
            f"{available} units available, {requested} requested"
        )

class PaymentDeclinedError(OrderError):
    def __init__(self, order, reason):
        self.order = order
        self.reason = reason
        super().__init__(
            f"Payment declined for order #{order.id}: {reason}"
        )
```

---

## Define Exception Classes in Terms of the Caller's Needs

When wrapping a third-party API, define exception classes based on how the caller will handle them, not based on the types of errors the API throws.

### Before: Mirroring Third-Party Exceptions

```java
// BAD: Caller must handle every possible vendor exception
try {
    port.open();
} catch (DeviceResponseException e) {
    reportPortError(e);
    logger.log("Device response exception", e);
} catch (ATM1212UnlockedException e) {
    reportPortError(e);
    logger.log("Unlock exception", e);
} catch (GMXError e) {
    reportPortError(e);
    logger.log("Device response exception");
}
```

### After: Wrapping by Caller's Needs

```java
// GOOD: Single exception class wraps all vendor exceptions
public class LocalPort {
    private ACMEPort innerPort;

    public void open() {
        try {
            innerPort.open();
        } catch (DeviceResponseException e) {
            throw new PortDeviceFailure(e);
        } catch (ATM1212UnlockedException e) {
            throw new PortDeviceFailure(e);
        } catch (GMXError e) {
            throw new PortDeviceFailure(e);
        }
    }
}

// Caller only handles one type
try {
    port.open();
} catch (PortDeviceFailure e) {
    reportError(e);
    logger.log(e.getMessage(), e);
}
```

**Benefits of wrapping:**
- Minimizes dependencies on the third-party API
- Makes it easy to swap vendors
- Simplifies testing with mocks
- Keeps caller code clean

---

## Don't Return Null

Returning null from a method is an invitation for NullPointerExceptions. Every null return forces every caller to add a null check, and a single missed check crashes the application.

### Alternatives to Returning Null

| Instead of null | Return this | When |
|----------------|-------------|------|
| Null collection | Empty collection | Method returns a list, set, or map |
| Null string | Empty string `""` | Method returns text |
| Null object | Special case object | Object has default behavior |
| Null optional value | `Optional.empty()` | Value may legitimately be absent |
| Null on error | Throw exception | Absence indicates a problem |

### The Special Case Pattern

Instead of checking for null to handle a special case, create an object that handles the special case.

```python
# BAD: Null checks everywhere
def get_expenses(employee):
    expenses = db.find_expenses(employee)
    if expenses is None:
        return 0
    total = 0
    for expense in expenses:
        if expense is not None:
            total += expense.amount if expense.amount is not None else 0
    return total

# GOOD: No null checks needed
def get_expenses(employee):
    expenses = db.find_expenses(employee)  # Returns empty list, never None
    return sum(expense.amount for expense in expenses)
```

### Null Object Pattern

```python
class RealUser:
    def __init__(self, name, permissions):
        self.name = name
        self.permissions = permissions

    def has_permission(self, action):
        return action in self.permissions

class GuestUser:
    """Null Object -- behaves like a user with no permissions."""
    name = "Guest"
    permissions = frozenset()

    def has_permission(self, action):
        return False

def find_user(user_id):
    user = db.find(user_id)
    return user if user else GuestUser()

# Caller never needs to check for null
user = find_user(request.user_id)
if user.has_permission("edit"):
    allow_edit()
```

---

## Don't Pass Null

Returning null is bad. Passing null is worse. When you pass null as an argument, you are creating a requirement for the callee to check for null, and if they don't, you get a runtime error.

```java
// BAD: What should this do with null?
public double calculateMetric(Point p1, Point p2) {
    return Math.sqrt(
        Math.pow(p2.x - p1.x, 2) + Math.pow(p2.y - p1.y, 2)
    );
}

// If someone calls calculateMetric(null, new Point(1, 2)) -- NullPointerException

// GOOD: Fail fast with clear message
public double calculateMetric(Point p1, Point p2) {
    Objects.requireNonNull(p1, "p1 must not be null");
    Objects.requireNonNull(p2, "p2 must not be null");
    return Math.sqrt(
        Math.pow(p2.x - p1.x, 2) + Math.pow(p2.y - p1.y, 2)
    );
}
```

**The best policy:** Forbid passing null by default. Use static analysis tools (`@NonNull`, `@Nullable` annotations) and code review to enforce this.

---

## Error Handling Patterns Summary

| Pattern | When to use | Benefit |
|---------|-------------|---------|
| **Try-catch-finally first** | Any code that could fail | Defines transaction boundary upfront |
| **Wrap third-party APIs** | Calling external libraries | Isolates vendor dependencies |
| **Special Case pattern** | Default behavior for missing data | Eliminates null/error checks in callers |
| **Null Object pattern** | Polymorphic default behavior | No null checks in calling code |
| **Guard clauses** | Input validation | Fail fast with clear messages |
| **Custom exception hierarchy** | Domain-specific errors | Caller handles by intent, not implementation |
| **Exception with context** | All thrown exceptions | Enables diagnosis without debugging |
| **Empty over null** | Collections, strings, optionals | Eliminates NullPointerException risk |

---

## Common Error Handling Anti-Patterns

| Anti-pattern | Problem | Fix |
|-------------|---------|-----|
| **Catch-and-ignore** | `catch (Exception e) { }` -- swallows all errors silently | At minimum log; usually re-throw or handle specifically |
| **Catch-and-log-and-rethrow** | Duplicates logging at every level | Catch at one level, let others propagate |
| **Returning error codes** | Forces immediate checking, clutters happy path | Use exceptions |
| **Returning -1 or sentinel values** | Magic values that callers forget to check | Use Optional or throw |
| **Exception for control flow** | Using try-catch instead of if-else for expected conditions | Use exceptions only for exceptional situations |
| **Overly broad catch** | `catch (Exception e)` catches bugs too | Catch specific exception types |
| **Nested try-catch** | Multiple try blocks in one function | Extract each try block into its own function |
| **Throws declaration cascade** | Checked exceptions forcing changes up the call stack | Use unchecked exceptions for application errors |

# Functions and Methods

Comprehensive guide to writing small, focused functions that do one thing well. Based on Robert C. Martin's *Clean Code*, Chapters 3 and 4.


## Table of Contents
1. [The First Rule of Functions](#the-first-rule-of-functions)
2. [Do One Thing](#do-one-thing)
3. [Function Arguments](#function-arguments)
4. [Flag Arguments](#flag-arguments)
5. [Command-Query Separation](#command-query-separation)
6. [Side Effects](#side-effects)
7. [Extract Till You Drop](#extract-till-you-drop)
8. [Structured Programming](#structured-programming)
9. [DRY: Don't Repeat Yourself](#dry-dont-repeat-yourself)
10. [Function Organization Within a Class](#function-organization-within-a-class)
11. [Common Function Anti-Patterns](#common-function-anti-patterns)

---

## The First Rule of Functions

**Functions should be small.** The second rule of functions is that they should be smaller than that.

A well-written function:
- Fits on one screen (ideally 4-10 lines)
- Has a name that describes exactly what it does
- Takes few arguments (zero is best, three is the maximum)
- Has no side effects
- Operates at a single level of abstraction

---

## Do One Thing

**Functions should do one thing. They should do it well. They should do it only.**

### How to Know if a Function Does One Thing

If you can extract another function from it with a name that is not merely a restatement of its implementation, the function does more than one thing.

```python
# BAD: Does three things
def process_payment(order):
    # 1. Validate
    if not order.items:
        raise ValueError("Empty order")
    if order.total <= 0:
        raise ValueError("Invalid total")

    # 2. Charge
    payment_result = gateway.charge(order.customer.card, order.total)
    if not payment_result.success:
        raise PaymentError(payment_result.error)

    # 3. Notify
    email_service.send_receipt(order.customer.email, order)
    analytics.track("payment_completed", order.id)

# GOOD: Does one thing, delegates details
def process_payment(order):
    validate_order(order)
    charge_customer(order)
    send_notifications(order)
```

### The Step-Down Rule

Code should read like a top-down narrative. Every function should be followed by the next level of abstraction.

```
To process a payment:
    We validate the order.
    We charge the customer.
    We send notifications.

To validate the order:
    We check it has items.
    We check the total is positive.

To charge the customer:
    We call the payment gateway.
    We handle any payment failure.

To send notifications:
    We email the receipt.
    We track the analytics event.
```

This reads like a newspaper: the headline (top-level function) tells you the story, and each successive paragraph (sub-function) provides more detail.

---

## Function Arguments

The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification.

### Argument Count Guide

| Count | Name | When acceptable | Example |
|-------|------|-----------------|---------|
| **0** | Niladic | Simple operations | `getCurrentTime()` |
| **1** | Monadic | Asking a question or transforming input | `isValid(email)`, `parse(json)` |
| **2** | Dyadic | Natural pairings | `Point(x, y)`, `assertEquals(expected, actual)` |
| **3** | Triadic | Rarely; consider object | `Color(r, g, b)` |
| **4+** | Polyadic | Almost never | Wrap in object: `new Config(...)` |

### Common Monadic Forms

Three common reasons to pass a single argument:

1. **Asking a question:** `boolean fileExists(path)` -- returns true/false about the argument
2. **Transforming it:** `InputStream fileOpen(path)` -- transforms the argument and returns the result
3. **Event:** `void passwordAttemptFailedNTimes(attempts)` -- uses the argument to alter system state (make this clear from the name)

### Why Many Arguments Are Problematic

```java
// BAD: What does each argument mean? What order?
createReport(title, startDate, endDate, format, includeCharts, sendEmail, recipients);

// GOOD: Parameter object groups related arguments
ReportConfig config = new ReportConfig.Builder()
    .title("Q4 Revenue")
    .dateRange(startDate, endDate)
    .format(PDF)
    .includeCharts(true)
    .build();
createReport(config);
```

Each argument increases the difficulty of understanding, testing, and calling the function. Arguments also create ordering dependencies that the reader must memorize.

---

## Flag Arguments

**Flag arguments are ugly.** Passing a boolean into a function loudly declares that the function does two things -- one thing if the flag is true, another if false.

```python
# BAD: Flag argument
def render(document, is_for_print):
    if is_for_print:
        # 20 lines of print rendering
        ...
    else:
        # 20 lines of screen rendering
        ...

# GOOD: Two clearly named functions
def render_for_print(document):
    ...

def render_for_screen(document):
    ...
```

If a function must behave differently based on a condition, split it into two named functions. If the behaviors share logic, extract the shared part into a private helper.

---

## Command-Query Separation

Functions should either do something (command) or answer something (query), but not both.

```java
// BAD: Does this set the attribute or check if it exists?
if (set("username", "unclebob")) { ... }

// GOOD: Separate query from command
if (attributeExists("username")) {
    setAttribute("username", "unclebob");
}
```

**Commands** change the state of an object. They should return void.
**Queries** return information about an object. They should not change state.

When a function both changes state and returns a value, the reader cannot tell from the call site what is happening.

---

## Side Effects

A side effect is when a function promises to do one thing but also does other hidden things.

### Common Hidden Side Effects

| Declared purpose | Hidden side effect | Danger |
|-----------------|-------------------|--------|
| `checkPassword(user, password)` | Initializes a session | Calling "check" unexpectedly logs user in |
| `getUser(id)` | Creates user if not found | "Get" implies read-only; caller doesn't expect writes |
| `toString()` | Modifies internal state | Debugging with print statements changes behavior |
| `validate(input)` | Sends analytics event | Validation during testing triggers real events |

### How to Fix Side Effects

1. **Make the side effect explicit in the name:** `checkPasswordAndInitSession()`
2. **Separate the concerns:** `checkPassword()` + `initSession()` as two calls
3. **Prefer option 2** -- separation makes testing easier and keeps functions honest

---

## Extract Till You Drop

If you can extract a named function from a block of code, you should. The extracted function's name adds documentation value, even if the function is only called from one place.

### When to Extract

| Signal | Action |
|--------|--------|
| A block inside an `if`, `else`, `for`, or `while` | Extract to named function |
| A comment explaining what the next lines do | Replace comment with named function |
| A function longer than 10 lines | Look for extraction opportunities |
| Nested indentation deeper than 2 levels | Extract inner blocks |
| Code that you'd need to re-read to understand | Name it so you don't have to |

### Extraction Example

```typescript
// BEFORE: Deeply nested, hard to follow
function processOrders(orders: Order[]) {
  for (const order of orders) {
    if (order.status === 'pending') {
      let total = 0;
      for (const item of order.items) {
        if (item.inStock) {
          total += item.price * item.quantity;
          if (item.quantity > 10) {
            total *= 0.9; // bulk discount
          }
        }
      }
      if (total > 0) {
        order.total = total;
        order.status = 'processed';
        db.save(order);
        emailService.sendConfirmation(order);
      }
    }
  }
}

// AFTER: Each function does one thing
function processOrders(orders: Order[]) {
  orders
    .filter(isPending)
    .forEach(processOrder);
}

function processOrder(order: Order) {
  const total = calculateOrderTotal(order);
  if (total > 0) {
    finalizeOrder(order, total);
  }
}

function calculateOrderTotal(order: Order): number {
  return order.items
    .filter(item => item.inStock)
    .reduce((sum, item) => sum + calculateItemPrice(item), 0);
}

function calculateItemPrice(item: Item): number {
  const basePrice = item.price * item.quantity;
  return item.quantity > BULK_DISCOUNT_THRESHOLD
    ? basePrice * BULK_DISCOUNT_RATE
    : basePrice;
}

function finalizeOrder(order: Order, total: number) {
  order.total = total;
  order.status = 'processed';
  db.save(order);
  emailService.sendConfirmation(order);
}
```

---

## Structured Programming

Dijkstra's rule states that every function should have one entry and one exit: one `return` statement, no `break` or `continue` in loops, and never `goto`.

**In practice, for small functions, multiple returns and early exits improve clarity:**

```python
# Guard clauses improve readability in small functions
def calculate_discount(customer):
    if customer is None:
        return 0
    if not customer.is_active:
        return 0
    if customer.total_purchases < MINIMUM_FOR_DISCOUNT:
        return 0

    return customer.total_purchases * DISCOUNT_RATE
```

Guard clauses handle the error cases at the top of the function, leaving the happy path unindented and clear. This pattern is preferable to deeply nested `if-else` chains.

---

## DRY: Don't Repeat Yourself

Duplication is the root of all evil in software. Every piece of knowledge should have a single, unambiguous, authoritative representation in the system.

### Types of Duplication

| Type | Example | Fix |
|------|---------|-----|
| **Exact duplication** | Same code block in three places | Extract to shared function |
| **Structural duplication** | Same algorithm with different data | Template Method or Strategy pattern |
| **Conceptual duplication** | Same business rule expressed differently | Consolidate into single source of truth |
| **Data duplication** | Same value computed in multiple places | Compute once, pass result |

### The Rule of Three

The first time you write something, just write it. The second time you see duplication, note it. The third time, refactor. This avoids premature abstraction while still catching genuine duplication.

---

## Function Organization Within a Class

### The Newspaper Metaphor

Organize functions the way a newspaper organizes articles:

1. **Headline at the top:** Public methods (the API) appear first
2. **Synopsis next:** High-level private methods called by the public methods
3. **Details last:** Low-level helper functions at the bottom

The reader can stop reading at any depth once they have enough understanding.

### Vertical Distance

**Dependent functions should be close.** If one function calls another, they should be vertically close in the source file, and the caller should be above the callee.

```java
// GOOD: Caller above callee, close together
public void processOrder(Order order) {
    validateOrder(order);
    chargeCustomer(order);
    shipOrder(order);
}

private void validateOrder(Order order) {
    // validation logic
}

private void chargeCustomer(Order order) {
    // payment logic
}

private void shipOrder(Order order) {
    // shipping logic
}
```

**Variables should be declared as close to their usage as possible.** Local variables at the top of the function. Loop variables inside the loop statement. Instance variables at the top of the class (everyone needs to know they exist).

---

## Common Function Anti-Patterns

| Anti-pattern | Problem | Refactoring |
|-------------|---------|-------------|
| **Output arguments** | `appendFooter(report)` -- is `report` input or output? | Make it a method: `report.appendFooter()` |
| **Selector arguments** | `calculate(MONTHLY)` enum switches behavior | Split: `calculateMonthly()`, `calculateAnnual()` |
| **Dead functions** | Never called, just sitting there | Delete them. Version control remembers. |
| **Switch statements** | Long switches violate SRP and OCP | Replace with polymorphism or strategy pattern |
| **Temporal coupling** | Functions must be called in a specific order | Make ordering explicit through return values or builder |
| **Leaky abstraction** | Function exposes implementation details | Hide internals, return domain objects |

# Naming Conventions

Comprehensive guide to choosing names that reveal intent, avoid disinformation, and make code read like well-written prose. Based on Robert C. Martin's *Clean Code*, Chapter 2.

## The Golden Rule

**A name should tell you why it exists, what it does, and how it is used.** If a name requires a comment to explain it, the name does not reveal its intent.

---

## Intention-Revealing Names

The name of a variable, function, or class should answer three questions:
1. Why does it exist?
2. What does it do?
3. How is it used?

### Before and After

| Before | After | Why it's better |
|--------|-------|-----------------|
| `d` | `elapsedTimeInDays` | Reveals what the number represents |
| `list` | `flaggedAccounts` | Describes *which* list and *why* it exists |
| `hp` | `isHighPriority` | Self-documents its boolean nature and meaning |
| `temp` | `unprocessedBatchItems` | Communicates purpose, not temporary status |
| `x`, `y` | `latitude`, `longitude` | Domain language replaces abstract math symbols |
| `val` | `discountPercentage` | Specifies what value and its unit |
| `data` | `customerOrderHistory` | Names the specific data, not the generic concept |
| `info` | `shippingTrackingDetails` | Adds specificity to a meaningless suffix |

### Code Example

```python
# BAD: What does this do?
def get_them(the_list):
    result = []
    for x in the_list:
        if x[0] == 4:
            result.append(x)
    return result

# GOOD: Intention is revealed
def get_flagged_cells(game_board):
    flagged_cells = []
    for cell in game_board:
        if cell.is_flagged():
            flagged_cells.append(cell)
    return flagged_cells
```

---

## Avoiding Disinformation

Disinformation means using names that imply something different from the actual meaning.

**Rules:**
- Don't use `accountList` unless it is actually a `List` type -- use `accounts` or `accountGroup`
- Don't use names that vary in small ways: `XYZControllerForEfficientHandlingOfStrings` vs `XYZControllerForEfficientStorageOfStrings`
- Don't use lowercase `L` or uppercase `O` as variable names (look like `1` and `0`)
- Don't use names of other well-known entities: `hp`, `aix`, `sco` are Unix platform names

### Common Disinformation Patterns

| Disinformation | Problem | Fix |
|----------------|---------|-----|
| `accountList` when it's a `Set` | Implies ordered, duplicates allowed | Use `accounts` |
| `name1`, `name2` | Implies ordering or relationship that doesn't exist | Use meaningful distinction: `firstName`, `lastName` |
| `strName` | Hungarian notation; type may change | Just use `name` |
| `theAccount` vs `account` | No meaningful distinction | Pick one, use it consistently |

---

## Meaningful Distinctions

If names must be different, they should mean something different.

**Noise words are meaningless distinctions:**
- `Product` vs `ProductInfo` vs `ProductData` -- what's the difference?
- `getAccount()` vs `getAccountInfo()` vs `getAccountData()` -- indistinguishable
- `name` vs `nameString` -- is `name` a floating-point number?

**The test:** If you cannot tell what two similarly named things do without reading their implementations, the names fail.

### How to Make Meaningful Distinctions

| Instead of | Use | Rationale |
|------------|-----|-----------|
| `copyChars(a1, a2)` | `copyChars(source, destination)` | Arguments reveal their role |
| `moneyAmount` vs `money` | Just `money` | Amount is implied by the context |
| `theMessage` vs `message` | Just `message` | The article adds no information |
| `accountData` vs `account` | Just `account` | Data is inherent -- what else would it be? |

---

## Pronounceable Names

If you cannot pronounce a name, you cannot discuss it without sounding foolish.

| Unpronounceable | Pronounceable |
|-----------------|---------------|
| `genymdhms` | `generationTimestamp` |
| `modymdhms` | `modificationTimestamp` |
| `pszqint` | `formattedQuantity` |
| `cstmrAddr` | `customerAddress` |

**Why it matters:** Programming is a social activity. You discuss code in reviews, pairing, design meetings, and stand-ups. Names you cannot say out loud create communication barriers.

---

## Searchable Names

Single-letter names and numeric constants are impossible to search for in a large codebase.

**Rule of thumb:** The length of a name should correspond to the size of its scope.

| Scope | Name length | Example |
|-------|-------------|---------|
| Single-line lambda | 1 character acceptable | `items.map(x => x.id)` |
| Loop body (3-5 lines) | 1-2 characters acceptable | `for i in range(len(items))` |
| Method scope | Descriptive word(s) | `retryCount`, `matchingUser` |
| Class scope | Fully descriptive phrase | `maximumQueueCapacity` |
| Module/global scope | Complete, searchable phrase | `DEFAULT_CONNECTION_TIMEOUT_MS` |

### Constants Must Be Named

```javascript
// BAD: What does 5 mean? What does 7 mean?
if (retries > 5) { ... }
setTimeout(fn, 7 * 86400000);

// GOOD: Self-documenting
const MAX_RETRY_ATTEMPTS = 5;
const CACHE_EXPIRY_DAYS = 7;
const MS_PER_DAY = 86400000;

if (retries > MAX_RETRY_ATTEMPTS) { ... }
setTimeout(fn, CACHE_EXPIRY_DAYS * MS_PER_DAY);
```

---

## Class Names vs. Method Names

### Class Names: Nouns and Noun Phrases

Classes represent things. Their names should be nouns or noun phrases.

| Good | Bad | Why |
|------|-----|-----|
| `Customer` | `CustomerManager` | Manager is vague -- what does it manage? |
| `WikiPage` | `WebPage` | More specific to the domain |
| `AddressParser` | `ParseAddress` | Noun phrase, not a verb |
| `InvoiceRepository` | `InvoiceProcessor` | Processor is vague -- what processing? |
| `Account` | `AccountData` | Data suffix is noise |

**Avoid:** `Manager`, `Processor`, `Data`, `Info` -- these are weasel words that indicate the class has no clear responsibility.

### Method Names: Verbs and Verb Phrases

Methods represent actions. Their names should be verbs or verb phrases.

| Good | Bad | Why |
|------|-----|-----|
| `save()` | `doSave()` | `do` prefix is noise |
| `calculateTax()` | `tax()` | Verb clarifies it computes, not stores |
| `isValid()` | `checkValid()` | Boolean accessor follows `is`/`has`/`can` convention |
| `fromJson(str)` | `parse(str)` | Static factory name describes transformation |
| `deleteExpiredSessions()` | `cleanup()` | Specific about what gets cleaned |

### Accessor, Mutator, and Predicate Conventions

```java
// Accessors: get + property
String getName()
int getAge()

// Mutators: set + property
void setName(String name)
void setAge(int age)

// Predicates: is/has/can + condition
boolean isEmpty()
boolean hasPermission(Role role)
boolean canExecute()
```

---

## Naming Conventions by Language

### Python
- `snake_case` for functions and variables: `calculate_total`, `user_count`
- `PascalCase` for classes: `UserAccount`, `OrderProcessor`
- `UPPER_SNAKE_CASE` for constants: `MAX_RETRIES`, `DEFAULT_TIMEOUT`
- `_leading_underscore` for private: `_internal_cache`
- `__dunder__` for magic methods: `__init__`, `__str__`

### JavaScript/TypeScript
- `camelCase` for functions and variables: `calculateTotal`, `userCount`
- `PascalCase` for classes and components: `UserAccount`, `OrderList`
- `UPPER_SNAKE_CASE` for constants: `MAX_RETRIES`, `API_BASE_URL`
- `#privateField` for private class fields (ES2022+)

### Java
- `camelCase` for methods and variables: `calculateTotal()`, `userCount`
- `PascalCase` for classes: `UserAccount`, `OrderService`
- `UPPER_SNAKE_CASE` for constants: `MAX_RETRIES`
- Packages: `com.company.project.module`

### Go
- `PascalCase` for exported (public): `CalculateTotal`, `UserCount`
- `camelCase` for unexported (private): `calculateTotal`, `userCount`
- Short names acceptable for small scopes: `r` for reader, `w` for writer
- Acronyms stay uppercase: `HTTPClient`, `XMLParser`, `userID`

### Rust
- `snake_case` for functions and variables: `calculate_total`, `user_count`
- `PascalCase` for types and traits: `UserAccount`, `Serialize`
- `UPPER_SNAKE_CASE` for constants and statics: `MAX_RETRIES`
- Lifetime names: short lowercase `'a`, `'b` or descriptive `'input`, `'output`

---

## The Pick-One-Word-Per-Concept Rule

Using different words for the same abstract concept is confusing.

| Inconsistent | Consistent | Rule |
|-------------|------------|------|
| `fetch` / `retrieve` / `get` | Pick `get` everywhere | One word for the read operation |
| `controller` / `manager` / `driver` | Pick `controller` everywhere | One word for the coordination role |
| `create` / `make` / `build` / `new` | Pick `create` for API, `build` for complex assembly | Differentiate only if semantics differ |
| `remove` / `delete` / `destroy` | `delete` for permanent, `remove` for detach | Different words when semantics differ |

---

## Solution Domain vs. Problem Domain Names

**Use solution domain names** when the concept is a well-known computer science term:
- `Queue`, `Stack`, `HashMap`, `Factory`, `Visitor`, `Iterator`
- Fellow programmers will recognize these instantly

**Use problem domain names** when the concept belongs to the business:
- `Invoice`, `Shipment`, `PolicyHolder`, `ClaimAdjuster`
- Domain experts and future maintainers will understand these

**The priority:** Problem domain names first, solution domain names second. Code that reads in business terms is easier for the whole team -- developers, testers, product managers -- to discuss and verify.

---

## Common Anti-Patterns

### Names to Avoid

| Anti-pattern | Example | Problem |
|-------------|---------|---------|
| **Single letter** | `a`, `b`, `t` | Meaningless outside tiny loops |
| **Abbreviation** | `acct`, `mgr`, `btn`, `usr` | Saves milliseconds, costs hours |
| **Generic suffix** | `DataObject`, `InfoManager` | Adds nothing; signals unclear responsibility |
| **Type encoding** | `sName`, `iCount`, `bFlag` | IDE shows types; encoding is obsolete and misleading |
| **Numbered series** | `arg1`, `arg2`, `arg3` | Provides no information about each argument's role |
| **Mental mapping** | `r` means URL (in the author's head) | Forces readers to maintain a mental translation table |
| **Puns** | `add` meaning both "concatenate" and "insert" | Same word, different semantics creates confusion |

---

## Renaming Checklist

When renaming a variable, function, or class, follow this checklist to ensure the change improves clarity:

1. **Does the new name reveal intent?** The reader should understand purpose without reading the implementation.
2. **Is it pronounceable?** Can you say it in a code review without spelling it out?
3. **Is it searchable?** Can you find all occurrences with a simple text search?
4. **Does it avoid disinformation?** The name should not imply something the code does not do.
5. **Is it consistent with the codebase?** Use the same word for the same concept everywhere.
6. **Is the length proportional to scope?** Short names for tiny scopes, long names for large scopes.
7. **Does it use domain language?** Prefer business terminology over generic programming terms.

**Remember:** Renaming is one of the most powerful refactorings. Modern IDEs make it safe and fast. Never hesitate to rename something that is unclear -- your future self and your teammates will thank you.

# Testing Principles

Comprehensive guide to writing clean, maintainable tests that serve as executable documentation. Based on Robert C. Martin's *Clean Code*, Chapter 9.


## Table of Contents
1. [Why Tests Matter](#why-tests-matter)
2. [The Three Laws of TDD](#the-three-laws-of-tdd)
3. [Clean Tests](#clean-tests)
4. [One Concept Per Test](#one-concept-per-test)
5. [F.I.R.S.T. Principles](#first-principles)
6. [Test Naming](#test-naming)
7. [Test Patterns and Practices](#test-patterns-and-practices)
8. [Tests as Documentation](#tests-as-documentation)

---

## Why Tests Matter

Test code is just as important as production code. It is not a second-class citizen. It requires thought, design, and care. Dirty tests are equivalent to, if not worse than, having no tests. Tests that are hard to read, fragile, or slow become a liability that developers avoid and eventually delete.

**The fundamental equation:** Clean tests = confidence to refactor = clean production code. Without tests, every change is a potential bug. With dirty tests, every change requires fighting through incomprehensible test code. With clean tests, refactoring is fearless.

---

## The Three Laws of TDD

Test-Driven Development follows three simple rules:

| Law | Rule | What it means |
|-----|------|---------------|
| **First** | You may not write production code until you have written a failing unit test | Tests drive the design, not the other way around |
| **Second** | You may not write more of a unit test than is sufficient to fail (compilation failures count) | Write the minimum test that fails |
| **Third** | You may not write more production code than is sufficient to pass the currently failing test | Write the minimum code that passes |

### The Red-Green-Refactor Cycle

1. **Red:** Write a failing test (it should fail for the right reason)
2. **Green:** Write the simplest code that makes the test pass (even if ugly)
3. **Refactor:** Clean up both production code and test code while keeping all tests green

This cycle runs in seconds to minutes, not hours. Each cycle produces one small, tested increment.

### Benefits of TDD

| Benefit | Why |
|---------|-----|
| **Nearly 100% coverage** | Every line of production code was written to pass a test |
| **Tests as documentation** | Tests show exactly how the code is intended to be used |
| **Fearless refactoring** | You know immediately if a change breaks something |
| **Better design** | Hard-to-test code is hard to use; TDD pushes toward clean design |
| **Debugging reduction** | When a test fails, the bug is in the last few lines you wrote |

---

## Clean Tests

### What Makes a Test Clean?

**Readability.** The same thing that makes production code clean makes test code clean: readability. What makes tests readable? The same thing that makes all code readable: clarity, simplicity, and density of expression. In a test, you want to say a lot with as few expressions as possible.

### The Build-Operate-Check Pattern

Every clean test has three distinct phases:

```python
def test_should_apply_bulk_discount_when_quantity_exceeds_threshold():
    # BUILD: Create the test data
    order = an_order()
        .with_item(product="Widget", quantity=25, unit_price=10.00)
        .build()

    # OPERATE: Execute the behavior under test
    invoice = billing_service.generate_invoice(order)

    # CHECK: Verify the expected outcome
    assert invoice.total == 225.00  # 250 - 10% bulk discount
    assert invoice.discount_applied == "BULK_10"
```

Also known as **Arrange-Act-Assert** or **Given-When-Then**.

### Domain-Specific Testing Language

Build utility functions and helpers that read like a domain-specific language for your tests.

```python
# BAD: Raw setup code obscures the test's intent
def test_expired_subscription():
    user = User(
        id=uuid4(),
        name="Alice",
        email="alice@example.com",
        created_at=datetime(2024, 1, 1),
        subscription=Subscription(
            plan="pro",
            status="active",
            expires_at=datetime(2024, 1, 15),
        ),
    )
    user.subscription.check_expiry(current_date=datetime(2024, 2, 1))
    assert user.subscription.status == "expired"

# GOOD: Helpers create a readable narrative
def test_expired_subscription():
    user = a_user().with_pro_subscription(expires_on="2024-01-15").build()

    user.check_subscription_on("2024-02-01")

    assert_that(user).has_expired_subscription()
```

The test reads like a specification: "Given a user with a pro subscription expiring on Jan 15, when we check the subscription on Feb 1, then the subscription should be expired."

---

## One Concept Per Test

Each test function should test one concept. This does not necessarily mean one assert per test -- it means one logical assertion, one behavioral expectation.

### One Concept, Multiple Asserts (Acceptable)

```python
def test_should_create_valid_invoice_from_order():
    order = an_order().with_two_items().build()

    invoice = billing_service.generate_invoice(order)

    assert invoice.customer == order.customer
    assert invoice.line_items_count == 2
    assert invoice.total == order.calculated_total
    assert invoice.status == "pending"
```

All four asserts verify one concept: "generating an invoice from an order produces a valid invoice."

### Multiple Concepts (Split Into Separate Tests)

```python
# BAD: Two concepts in one test
def test_invoice_generation():
    order = an_order().build()
    invoice = billing_service.generate_invoice(order)
    assert invoice.total == order.calculated_total  # Concept 1: correct total

    invoice.mark_as_paid()
    assert invoice.status == "paid"  # Concept 2: payment status transition

# GOOD: Each test covers one concept
def test_should_calculate_correct_invoice_total():
    order = an_order().with_total(150.00).build()
    invoice = billing_service.generate_invoice(order)
    assert invoice.total == 150.00

def test_should_transition_to_paid_when_marked_as_paid():
    invoice = an_invoice().with_status("pending").build()
    invoice.mark_as_paid()
    assert invoice.status == "paid"
```

---

## F.I.R.S.T. Principles

Clean tests follow five principles that form the acronym F.I.R.S.T.:

### Fast

Tests should be fast. When tests run slowly, you won't run them frequently. When you don't run them frequently, you won't find problems early. When you don't find problems early, you won't fix them easily.

| Guideline | Target | How |
|-----------|--------|-----|
| Unit test suite | Under 10 seconds | Mock all external dependencies |
| Individual test | Under 100ms | No I/O, no network, no database |
| Integration tests | Separate suite | Run separately, not on every save |

### Independent

Tests should not depend on each other. One test should not set up conditions for the next. You should be able to run each test independently and in any order.

```python
# BAD: Test B depends on Test A's side effects
def test_a_create_user():
    global test_user
    test_user = UserService.create("Alice")

def test_b_update_user():
    UserService.update(test_user.id, name="Bob")  # Fails if A doesn't run first

# GOOD: Each test is self-contained
def test_create_user():
    user = UserService.create("Alice")
    assert user.name == "Alice"

def test_update_user():
    user = UserService.create("Alice")  # Own setup
    updated = UserService.update(user.id, name="Bob")
    assert updated.name == "Bob"
```

### Repeatable

Tests should produce the same result every time, in any environment -- development machine, CI server, production-like staging. Tests that depend on network availability, current time, or random data are flaky.

| Flaky dependency | Fix |
|-----------------|-----|
| Current time | Inject a clock; mock `datetime.now()` |
| Random data | Use seeded random or fixed test data |
| Network calls | Mock HTTP clients |
| Database state | Use transactions that roll back, or in-memory DB |
| File system | Use temp directories; clean up in teardown |
| Environment variables | Set explicitly in test setup |

### Self-Validating

Tests should have a boolean output: pass or fail. No manual interpretation required.

```python
# BAD: Requires human to check output
def test_report_generation():
    report = generate_report()
    print(report)  # Developer must read and visually verify

# GOOD: Automated assertion
def test_report_generation():
    report = generate_report()
    assert report.title == "Q4 Revenue Report"
    assert report.total_revenue == 142_500.00
    assert len(report.line_items) == 12
```

### Timely

Tests should be written just before the production code that makes them pass (TDD). Tests written after the fact are harder to write because the production code may not be designed for testability. You may decide that some production code is "too hard to test" -- which really means it's too coupled.

---

## Test Naming

Test names should describe the scenario being tested and the expected behavior.

### Naming Patterns

| Pattern | Example | When to use |
|---------|---------|-------------|
| `should_[expected]_when_[condition]` | `should_reject_login_when_password_expired` | Most common; clear cause-effect |
| `[method]_[scenario]_[expected]` | `withdraw_insufficient_funds_throws_exception` | When testing a specific method |
| `given_[state]_when_[action]_then_[result]` | `given_empty_cart_when_checkout_then_error` | BDD-style |
| `test_[behavior_description]` | `test_expired_tokens_are_rejected` | Simple, readable |

### Bad Test Names

| Bad name | Problem | Better name |
|----------|---------|-------------|
| `test1` | Meaningless | `test_empty_input_returns_empty_list` |
| `testProcess` | What about process? | `test_process_skips_inactive_users` |
| `testCalculate` | Too vague | `test_calculate_applies_weekend_surcharge` |
| `testBug1234` | Won't make sense in 6 months | `test_duplicate_orders_are_rejected` |

---

## Test Patterns and Practices

### Parameterized Tests

When testing the same behavior with different inputs, use parameterized tests instead of copy-pasting.

```python
@pytest.mark.parametrize("input_email,expected_valid", [
    ("user@example.com", True),
    ("user@sub.example.com", True),
    ("user@example", False),
    ("@example.com", False),
    ("user@.com", False),
    ("", False),
])
def test_email_validation(input_email, expected_valid):
    assert validate_email(input_email) == expected_valid
```

### Test Fixtures and Builders

Use the Builder pattern for test data to make tests readable and maintainable.

```python
class UserBuilder:
    def __init__(self):
        self._name = "Default User"
        self._email = "default@example.com"
        self._role = "viewer"
        self._active = True

    def with_name(self, name): self._name = name; return self
    def with_role(self, role): self._role = role; return self
    def inactive(self): self._active = False; return self
    def build(self):
        return User(
            name=self._name, email=self._email,
            role=self._role, active=self._active,
        )

def a_user():
    return UserBuilder()

# Usage in tests
admin = a_user().with_name("Alice").with_role("admin").build()
inactive_user = a_user().inactive().build()
```

### Testing Error Paths

Every error path in production code should have a corresponding test.

```python
def test_should_raise_on_negative_amount():
    account = an_account().with_balance(100).build()
    with pytest.raises(ValueError, match="Amount must be positive"):
        account.withdraw(-50)

def test_should_raise_on_insufficient_funds():
    account = an_account().with_balance(100).build()
    with pytest.raises(InsufficientFundsError):
        account.withdraw(150)
```

### Boundary Condition Tests

Test the edges, not just the middle.

| Boundary | Tests needed |
|----------|-------------|
| Empty input | `[]`, `""`, `None`, `{}` |
| Single element | List with one item, string with one char |
| Maximum values | `MAX_INT`, full capacity, max length |
| Off-by-one | `n-1`, `n`, `n+1` for any threshold |
| Transition points | Just below and just above limits |
| Overflow/underflow | Values that exceed type boundaries |

---

## Tests as Documentation

Clean tests serve as the most reliable documentation of how the system behaves. Unlike comments or wiki pages, tests are always up to date -- if they weren't, they'd be failing.

| Documentation type | Tests provide |
|-------------------|---------------|
| **API usage** | Test setup shows how to call the API |
| **Expected behavior** | Assertions describe what should happen |
| **Edge cases** | Boundary tests document special cases |
| **Error behavior** | Error path tests document failure modes |
| **Business rules** | Test names describe domain rules |

When a new developer asks "how does this work?", point them to the tests. Clean tests answer the question better than any comment or README.