Clean Code

Clean Code

What this skill does

This skill applies Robert C. Martin's Clean Code framework for writing and reviewing professional-quality code. It covers the full range of topics: naming, functions, comments, formatting, objects vs. data structures, error handling, boundaries, tests, and classes.

The master reference is the Smells & Heuristics catalog at the end — a coded, searchable index of every pattern covered in the book. When reviewing code, scan that catalog first.

The core insight: the ratio of time spent reading code to writing it is well over 10:1. Making code easier to read makes it easier to write. The biggest ROI is always readability.

The Boy Scout Rule: Leave the campground cleaner than you found it. Each time you touch code, leave it slightly better — one better name, one extracted function, one deleted comment. LeBlanc's Law warns: Later equals never.

Functions

Size

Functions should be small — the ideal is 4–5 lines. Functions should hardly ever be 20 lines. The test: can you read the function in its entirety without scrolling?

Do One Thing

A function does one thing if all its steps are one level of abstraction below the function's name. If you can meaningfully extract a section into a function whose name is not merely a restatement of its contents, the function is doing more than one thing.

One Level of Abstraction per Function

Mixing levels of abstraction in a function is always confusing. The Stepdown Rule: code reads top to bottom. Each function is followed by the next level of abstraction. Reading a module should feel like reading a narrative.

Switch Statements

Switch statements by nature do N things. When you must use one:

• Bury it in an abstract factory
• Use it only once — to create polymorphic objects
• Never repeat the same switch in multiple places

Arguments

• Niladic (zero) is ideal
• Monadic is fine
• Dyadic is tolerable with care
• Triadic and beyond: avoid with prejudice
• Flag arguments are always wrong — passing a boolean into a function declares that the function does more than one thing
• Output arguments are confusing: if a function must change state, have it change the state of the object it is called on

Command Query Separation

A function should either change the state of an object, or return information about it — never both. if (set("username", "bob")) mixes a command with a query; split them.

Prefer Exceptions to Error Codes

Returning error codes forces callers to deal with errors immediately and leads to deeply nested structures. Exceptions allow error handling to be separated from the happy path. Extract try/catch bodies into their own functions. Error handling is one thing — a function that handles errors should do nothing else.

DRY

Duplication may be the root of all evil in software. Every time you see it, it represents a missed opportunity for abstraction.

Naming

(This skill covers naming at a principles level; see the naming-guidelines skill for the complete ruleset with violations and corrections.)

• Use intention-revealing names. The name should tell you why it exists, what it does, and how it is used. If a name requires a comment, the name does not reveal intent.
• Avoid disinformation: accountList for a non-List container is misleading.
• Make meaningful distinctions: getActiveAccount() vs getActiveAccounts() vs getActiveAccountInfo() are indistinguishable to a caller.
• Use pronounceable names. If you can't say it, you can't discuss it.
• Use searchable names. Single-letter names and magic numbers appear in many places and cannot be grepped. The longer the scope, the longer the name.
• No encodings: Hungarian notation, member prefixes (m_), interface prefixes (I) — all are noise. Your IDE already knows the type.
• Classes: noun or noun phrase (Customer, WikiPage, Account)
• Methods: verb or verb phrase (postPayment, deletePage, save)
• One word per concept: pick fetch, retrieve, or get — not all three
• No puns: clarity is king; humour sacrifices it

Comments

Comments are a failure to express in code. The proper use of comments is to compensate for our failure to express ourselves in code. Every comment represents a failure — but some failures are unavoidable.

Good Comments

• Legal: copyright, license headers
• Explanation of intent: why a decision was made, not what the code does
• Clarification: when you cannot refactor the code itself (e.g., third-party library)
• Warning of consequences: why something is dangerous or surprising
• TODO: notes for future work (but review and remove them regularly)
• Amplification: marking something that might otherwise seem unimportant as critical
• Javadocs/docstrings in public APIs: when the API is public-facing

Bad Comments (delete these)

• Mumbling: comments written hastily that make no sense without author context
• Redundant: says what the code already says; takes longer to read than the code
• Misleading: not quite wrong, but wrong enough to lead the reader astray
• Mandated: "every function must have Javadoc" produces noise, not signal
• Journal comments: change history belongs in source control
• Noise: restating the obvious; adds no information
• Position markers: // --- Actions --- banners are clutter
• Closing brace comments: } // end while — extract instead
• Commented-out code: delete it. Source control remembers. Everyone leaves it because they think someone else put it there on purpose. It rots.
• HTML in comments: makes comments unreadable in source; let the IDE render it
• Nonlocal information: comments about something far away from where the comment lives
• Inobvious connection: the comment should describe the code next to it, not some other code elsewhere

Formatting

Code formatting is about communication. The developer's first professional obligation.

Vertical Formatting

• Newspaper metaphor: the top of a file is the headline — name alone tells you whether you're in the right module. As you move down, detail increases.
• Vertical openness: blank lines between concepts. Each blank line is a visual cue that a new and separate concept follows.
• Vertical density: lines that are tightly related belong together, with no blank lines between them.
• Vertical distance: concepts that are closely related should be close together in the file. Variable declarations near use; instance variables at the top of classes; dependent functions close (callers above callees); conceptually similar functions near each other.
• Target ~200 lines per file; 500 lines is a reasonable upper limit.

Horizontal Formatting

• Target ~80–120 characters per line
• Use horizontal whitespace to associate/disassociate concepts: a*a + b*b reads the multiplication as tighter than the addition
• Indentation reveals the hierarchy of scope — never collapse it for "short" blocks
• The team's formatting rules supersede individual preference; automate with a formatter

Objects and Data Structures

Hide Implementation

Objects hide data behind abstractions and expose functions that operate on that data. Adding getters and setters to every field is not encapsulation — it exposes implementation. Choose abstractions that let you manipulate the essence of data without knowing its implementation.

The Data/Object Anti-Symmetry

• OO code: easy to add new types without changing existing functions (polymorphism)
• Procedural code: easy to add new functions without changing existing data structures

Both are useful. The choice depends on what you expect to change. Don't try to do both — hybrids (half object, half data structure) are the worst of both worlds.

Law of Demeter

A method f of class C should only call methods of:

1. C itself
2. Objects created by f
3. Objects passed as arguments to f
4. Objects held in instance variables of C

Train wrecks: ctxt.getOptions().getScratchDir().getAbsolutePath() — each call gets an object returned by the previous call. Split into temporary variables, or restructure.

DTOs and Active Records: pure data structures with public fields are fine for database rows, socket messages, etc. Treat Active Records as data structures — don't add business logic to them; put that in separate objects.

Error Handling

• Use exceptions, not return codes: error handling obscures logic when mixed with the happy path. Exceptions allow them to be separated.
• Write the try/catch/finally first: this defines the transaction scope — the scope within which the caller can depend on consistent state.
• Use unchecked exceptions: checked exceptions violate the Open/Closed Principle. They cascade through every signature between the thrower and the catcher, creating coupling.
• Provide context: every exception should carry enough context to determine the source and location of an error (message, operation being attempted, type of failure).
• Define exception classes by caller needs: wrap third-party APIs in a single exception type. The caller usually cares about one question: did it fail?
• Special Case Pattern: return a special case object rather than throwing. The caller never has to deal with exceptional behavior.
• Don't return null: return empty collections, null objects, or special cases instead. Every null return is a future NullPointerException waiting to happen.
• Don't pass null: calling a method with null is even worse. Forbid it by convention.

Boundaries (Third-Party Code)

• Don't pass boundary interfaces around: Map (and similar generic containers) expose far more than you need. Wrap them in a type that exposes only what your code needs.
• Learning tests: before integrating a third-party library, write tests against its API to learn how it works. These tests also serve as regression tests when the library changes.
• Define the interface you wish you had: when integrating something unknown, write the code that calls the interface you want. Then write adapters that bridge the gap.
• Depend on something you control: code at the boundary of your system should live in few places and have clear separation. Depend on abstractions you own, not third-party interfaces you don't.

Unit Tests

Three Laws of TDD

1. You may not write production code until you have a failing unit test
2. You may not write more of a unit test than is sufficient to fail
3. You may not write more production code than is sufficient to make the failing test pass

Test Code Is Production Code

Dirty tests are worse than no tests. Tests enable change — without them, every change is a possible bug. If your tests are messy, your production code becomes rigid. Readability is even more important in tests than in production code.

Clean Tests: Build-Operate-Check

Each test follows a clear pattern: build the test data, operate on it, check the results. Evolve a domain-specific testing language from the helpers you find yourself extracting.

One Concept per Test

Minimize asserts per test. One concept per test. When a test fails, you should know immediately what broke.

F.I.R.S.T.

• Fast: slow tests don't get run
• Independent: tests should not depend on each other; they should run in any order
• Repeatable: in any environment (CI, local, offline)
• Self-validating: binary result — either they pass or they fail; no manual checking
• Timely: write tests just before the production code they test

Classes

Organization

Public static constants → private static variables → private instance variables → public functions → private utilities called by the public functions above them.

Small — Measured by Responsibilities

Classes should be small — but measured in responsibilities, not lines. If you can't describe a class in ~25 words without using "if," "and," "or," or "but," it has too many responsibilities.

Single Responsibility Principle (SRP)

A class should have one, and only one, reason to change. Many small, focused classes are easier to understand than a few large ones. A system with many small classes has no more moving parts than a system with a few large classes; it just has different organization.

Cohesion

Classes should have a small number of instance variables. Each method should use as many of those variables as possible. When cohesion drops — methods use only a subset of variables — it's a signal to split the class.

Open/Closed Principle (OCP)

Classes should be open for extension but closed for modification. Isolate behavior behind interfaces; add new behavior by adding new types, not by modifying existing code.

Dependency Inversion Principle (DIP)

Classes should depend on abstractions, not on concrete implementations. This makes classes testable (you can inject test doubles) and loosely coupled.

Smells and Heuristics — Quick Reference

This is the coded catalog from Chapter 17. When reviewing code, scan this list and note which codes apply. Each code can be cited in review comments.

Comments

Environment

Functions

General

Names

Tests

Quick Diagnosis Checklist

When reviewing any function or class, ask:

1. Can you name exactly one thing this function does?
2. Can you read the function without scrolling?
3. Does every name tell you why it exists, what it does, how it is used?
4. Are there any comments that exist because the code isn't clear enough? (Clarify the code instead)
5. Is there any commented-out code? (Delete it)
6. Does the function return null or an error code? (Return a special case or throw)
7. Are there flag arguments or output arguments? (Split the function or mutate this)
8. Does the function mix levels of abstraction? (Stepdown Rule)
9. Are there magic numbers or strings? (Name them)
10. Is there duplication? (Extract a shared abstraction)
11. Does the class have one reason to change?
12. Are all tests fast, independent, and self-validating?

Attribution

Based on Robert C. Martin's Clean Code: A Handbook of Agile Software Craftsmanship (2008). The Smells and Heuristics catalog (Chapter 17) consolidates patterns from Martin Fowler's Refactoring (1999), The Pragmatic Programmer (Hunt & Thomas, 2000), Design Patterns (Gamma et al., 1996), and Kent Beck's Smalltalk Best Practice Patterns (1997). The SOLID principles referenced here are from Martin's Agile Software Development: Principles, Patterns, and Practices (2002). Domain-driven design terms draw from Eric Evans' Domain-Driven Design (2003).