
Clean Code
- 681 installs
- 186 repo stars
- Updated July 24, 2026
- pproenca/dot-skills
clean-code is a Code Review & Quality skill that applies Robert C. Martin Clean Code principles with incorrect-vs-correct examples for developers who want to eliminate readability and maintainability issues across any co
About
clean-code is a rule-based agent skill from pproenca/dot-skills that encodes Clean Code principles as structured guidance with impact ratings, anti-pattern examples, corrected alternatives, and explicit exception cases. Each rule follows a consistent template—title, impact level, incorrect code block, correct code block, and when-not-to-use notes—so an agent can audit or rewrite code against named maintainability standards rather than vague style preferences. Developers reach for clean-code when refactoring legacy modules, standardizing team conventions, or preparing diffs for human review where naming, function size, and clarity violations are common. The readme excerpt references Clean Code chapter citations and multi-language bad/good snippets, making the skill suitable for Java and other stacks without tying to one framework.
- 48 rules across 10 categories including naming, functions, comments, formatting, error handling, objects, classes, and t
- Each rule provides incorrect vs correct code examples with explicit 'When NOT to use' exceptions
- Polyglot guidance that works with Java, TypeScript, Python, and other languages
- Modern updates that correct outdated 2008 advice for contemporary agent-driven development
- Hard-gated review workflow that produces annotated code with severity levels before commit
Clean Code by the numbers
- 681 all-time installs (skills.sh)
- Ranked #193 of 1,356 Code Review & Quality skills by installs in the Skillselion catalog
- Security screen: MEDIUM risk (skills.sh audit)
- Data as of Jul 31, 2026 (Skillselion catalog sync)
npx skills add https://github.com/pproenca/dot-skills --skill clean-codeAdd your badge
Show developers this skill is listed on Skillselion. Paste this into your README.
| Installs | 681 |
|---|---|
| repo stars | ★ 186 |
| Security audit | 3 / 3 scanners passed |
| Last updated | July 24, 2026 |
| Repository | pproenca/dot-skills ↗ |
How do you apply Clean Code rules automatically?
Automatically apply Clean Code principles and eliminate common readability and maintainability issues across any codebase.
Who is it for?
Developers refactoring messy modules or aligning a mixed-language codebase with Robert C. Martin readability standards before pull request review.
Skip if: Teams that only need framework-specific lint autofixes from ESLint or Prettier without principle-level explanations and exception guidance.
When should I use this skill?
The user asks to clean up code, improve readability, apply Clean Code, or fix maintainability smells with before-and-after examples.
What you get
Refactored code snippets, rule-by-rule fix notes, and documented exception cases per Clean Code chapter references.
- Refactored code snippets
- Rule-by-rule fix guidance
Files
Robert C. Martin (Uncle Bob) Clean Code Best Practices
Comprehensive software craftsmanship guide based on Robert C. Martin's "Clean Code: A Handbook of Agile Software Craftsmanship", updated with modern corrections where the original 2008 advice has been superseded. Contains 48 rules across 10 categories, prioritized by impact to guide code reviews, refactoring decisions, and new development. Examples are primarily in Java but principles are language-agnostic.
When to Apply
Reference these guidelines when:
- Writing new functions, classes, or modules
- Naming variables, functions, classes, or files
- Reviewing code for maintainability issues
- Refactoring existing code to improve clarity
- Writing or improving unit tests
- Wrapping third-party dependencies
Rule Categories by Priority
| Priority | Category | Impact | Prefix |
|---|---|---|---|
| 1 | Meaningful Names | CRITICAL | name- |
| 2 | Functions | CRITICAL | func- |
| 3 | Comments | HIGH | cmt- |
| 4 | Formatting | HIGH | fmt- |
| 5 | Error Handling | HIGH | err- |
| 6 | Objects and Data Structures | MEDIUM-HIGH | obj- |
| 7 | Boundaries | MEDIUM-HIGH | bound- |
| 8 | Classes and Systems | MEDIUM-HIGH | class- |
| 9 | Unit Tests | MEDIUM | test- |
| 10 | Emergence and Simple Design | MEDIUM | emerge- |
Quick Reference
1. Meaningful Names (CRITICAL)
- `name-intention-revealing` - Use names that reveal intent
- `name-avoid-disinformation` - Avoid misleading names
- `name-meaningful-distinctions` - Make meaningful distinctions
- `name-pronounceable` - Use pronounceable names
- `name-searchable` - Use searchable names
- `name-avoid-encodings` - Avoid encodings in names
- `name-class-noun` - Use noun phrases for class names
- `name-method-verb` - Use verb phrases for method names
2. Functions (CRITICAL)
- `func-small` - Keep functions small
- `func-one-thing` - Functions should do one thing
- `func-abstraction-level` - Maintain one level of abstraction
- `func-minimize-arguments` - Minimize function arguments
- `func-no-side-effects` - Avoid side effects
- `func-command-query-separation` - Separate commands from queries
- `func-dry` - Do not repeat yourself
3. Comments (HIGH)
- `cmt-express-in-code` - Express yourself in code, not comments
- `cmt-explain-intent` - Use comments to explain intent
- `cmt-avoid-redundant` - Avoid redundant comments
- `cmt-avoid-commented-out-code` - Delete commented-out code
- `cmt-warning-consequences` - Use warning comments for consequences
4. Formatting (HIGH)
- `fmt-vertical-formatting` - Use vertical formatting for readability
- `fmt-horizontal-alignment` - Avoid horizontal alignment
- `fmt-team-rules` - Follow team formatting rules
- `fmt-indentation` - Respect indentation rules
5. Error Handling (HIGH)
- `err-use-exceptions` - Separate error handling from happy path
- `err-write-try-catch-first` - Write try-catch-finally first
- `err-provide-context` - Provide context with exceptions
- `err-define-by-caller-needs` - Define exceptions by caller needs
- `err-avoid-null` - Avoid returning and passing null
6. Objects and Data Structures (MEDIUM-HIGH)
- `obj-data-abstraction` - Hide data behind abstractions
- `obj-data-object-asymmetry` - Understand data/object anti-symmetry
- `obj-law-of-demeter` - Follow the Law of Demeter
- `obj-avoid-hybrids` - Avoid hybrid data-object structures
- `obj-dto` - Use DTOs for data transfer
7. Boundaries (MEDIUM-HIGH)
- `bound-wrap-third-party` - Wrap third-party APIs
- `bound-learning-tests` - Write learning tests for third-party code
8. Classes and Systems (MEDIUM-HIGH)
- `class-small` - Keep classes small
- `class-cohesion` - Maintain class cohesion
- `class-organize-for-change` - Organize classes for change
- `class-isolate-from-change` - Isolate classes from change
- `class-separate-concerns` - Separate construction from use
9. Unit Tests (MEDIUM)
- `test-first-law` - Follow the three laws of TDD
- `test-keep-clean` - Keep tests clean
- `test-one-assert` - One concept per test
- `test-first-principles` - Follow FIRST principles
- `test-build-operate-check` - Use Build-Operate-Check pattern
10. Emergence and Simple Design (MEDIUM)
- `emerge-simple-design` - Follow the four rules of simple design
- `emerge-expressiveness` - Maximize expressiveness
How to Use
Read individual reference files for detailed explanations and code examples:
- Section definitions - Category structure and impact levels
- Rule template - Template for adding new rules
Reference Files
| File | Description |
|---|---|
| references/_sections.md | Category definitions and ordering |
| assets/templates/_template.md | Template for new rules |
| metadata.json | Version and reference information |
Rule Title Here
Brief explanation of the rule and why it matters. Focus on the principle and its impact on code maintainability and readability.
Incorrect (description of what's wrong):
// Bad code example here showing the anti-pattern
// Comment explaining the problemCorrect (description of what's right):
// Good code example here showing the solution
// Comment explaining the benefitWhen NOT to use this pattern:
- Exception case 1
- Exception case 2
Reference: Clean Code, Chapter N: Topic
{
"version": "1.0.7",
"organization": "Robert C. Martin (Uncle Bob)",
"technology": "Software Craftsmanship (examples in Java, principles are language-agnostic)",
"date": "January 2026",
"abstract": "Comprehensive software craftsmanship guide based on Robert C. Martin's 'Clean Code: A Handbook of Agile Software Craftsmanship', updated with modern corrections where the original 2008 advice has been superseded. Contains 48 rules across 10 categories covering naming, functions, comments, formatting, error handling, objects, boundaries, classes, testing, and simple design. Each rule includes 'When NOT to use' caveats, polyglot guidance where applicable, and real-world examples comparing incorrect vs. correct implementations.",
"references": [
"https://www.oreilly.com/library/view/clean-code-a/9780136083238/",
"https://blog.cleancoder.com/",
"https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882"
]
}
Sections
This file defines all sections, their ordering, impact levels, and descriptions. The section ID (in parentheses) is the filename prefix used to group rules.
---
1. Meaningful Names (name)
Impact: CRITICAL Description: Names are the primary documentation. Bad names cascade confusion throughout the codebase, forcing every reader to decipher intent.
2. Functions (func)
Impact: CRITICAL Description: Functions are the verbs of code. Small, focused functions enable understanding, testing, and reuse while reducing complexity.
3. Comments (cmt)
Impact: HIGH Description: Comments should explain intent, not obvious mechanics. Wrong or stale comments actively mislead readers and cost more than no comments.
4. Formatting (fmt)
Impact: HIGH Description: Consistent formatting reduces cognitive load. Code should read top-to-bottom like a newspaper article.
5. Error Handling (err)
Impact: HIGH Description: Clean error handling separates happy path from exceptional cases. Use the error mechanism idiomatic to your language (exceptions, Result types, or explicit error returns).
6. Objects and Data Structures (obj)
Impact: MEDIUM-HIGH Description: Objects hide data and expose behavior; data structures expose data and have no behavior. Mixing these creates hybrid messes.
7. Boundaries (bound)
Impact: MEDIUM-HIGH Description: Third-party code and external APIs are boundaries. Wrap them to isolate change, ease testing, and maintain control over your codebase's interfaces.
8. Classes and Systems (class)
Impact: MEDIUM-HIGH Description: Classes should be small and have a single responsibility. Systems should separate construction from use. SRP and DIP are among the highest-leverage design principles.
9. Unit Tests (test)
Impact: MEDIUM Description: Tests are first-class citizens that enable safe refactoring. Test code deserves the same care as production code.
10. Emergence and Simple Design (emerge)
Impact: MEDIUM Description: Good design emerges from following four rules in order: pass tests, reveal intent, eliminate duplication, minimize elements. Resist the urge to over-engineer.
Write Learning Tests for Third-Party Code
When integrating a third-party library, write small tests that verify your understanding of how the API works. These tests serve double duty: they help you learn the API, and they act as a safety net when upgrading the library.
Incorrect (learning by trial and error in production code):
// Experimenting directly in application code
public void importCustomerRecord(String json) {
// Does Gson handle null fields? Let's find out...
Gson gson = new Gson();
CustomerRecord record = gson.fromJson(json, CustomerRecord.class);
// Does this throw or return null for missing fields?
// Who knows — we'll find out in production
}Correct (learning tests verify assumptions):
@Test
public void gsonShouldDeserializeNullFieldsAsNull() {
Gson gson = new Gson();
Data data = gson.fromJson("{}", Data.class);
assertNull(data.getName());
}
@Test
public void gsonShouldHandleMalformedJson() {
Gson gson = new Gson();
assertThrows(JsonSyntaxException.class,
() -> gson.fromJson("{invalid", Data.class));
}
@Test
public void gsonShouldIgnoreUnknownFields() {
Gson gson = new Gson();
Data data = gson.fromJson("{\"name\":\"test\",\"unknown\":true}", Data.class);
assertEquals("test", data.getName());
}Benefits:
- Free education on the API's behavior
- When you upgrade the library, these tests break first — before your production code does
- Documents exactly which behaviors your code relies on
- Takes minutes to write but saves hours of debugging
Reference: Clean Code, Chapter 8: Boundaries
Wrap Third-Party APIs
When you use a third-party API, wrap it in a class you control. This minimizes your dependency on the third party, makes testing easier, and gives you a single place to change when the API evolves.
Incorrect (direct dependency scattered across codebase):
// Every caller depends on ACMEPort directly
public class SensorReader {
public void readSensors() {
try {
ACMEPort port = new ACMEPort(12);
port.open();
// ...
} catch (ACMEDeviceException e) {
logger.log("Device exception", e);
} catch (ACMEConfigException e) {
logger.log("Config exception", e);
} catch (ACMECommunicationException e) {
logger.log("Communication exception", e);
}
}
}
// If ACME changes their API, every file that uses it must changeCorrect (wrapped behind your own interface):
public class LocalPort {
private ACMEPort innerPort;
public LocalPort(int portNumber) {
innerPort = new ACMEPort(portNumber);
}
public void open() {
try {
innerPort.open();
} catch (ACMEDeviceException | ACMEConfigException | ACMECommunicationException e) {
throw new PortException(e);
}
}
}
// Callers depend only on your wrapper
public class SensorReader {
public void readSensors() {
try {
LocalPort port = new LocalPort(12);
port.open();
// ...
} catch (PortException e) {
logger.log("Port error", e);
}
}
}Benefits:
- One place to change when the third-party API changes
- Easier to mock in tests (mock
LocalPort, not the vendor library) - Freedom to switch vendors without rewriting the entire codebase
- Consistent exception hierarchy under your control
When NOT to wrap:
- Stable, widely-used standard library APIs (e.g.,
java.util.List,String) do not need wrapping. - If the wrapper would be a 1:1 passthrough with no value added, it's premature. Wait until you have a concrete reason (testing, migration, or simplification).
Reference: Clean Code, Chapter 8: Boundaries
Maintain Class Cohesion
Classes should have a small number of instance variables. Each method should manipulate one or more of those variables. The more variables a method manipulates, the more cohesive the method is to its class.
Incorrect (low cohesion - methods use different variables):
public class Utility {
private Database db;
private EmailService email;
private Logger logger;
private Cache cache;
public void saveUser(User user) {
db.save(user); // Only uses db
logger.log("Saved user"); // Only uses logger
}
public void sendNotification(String message) {
email.send(message); // Only uses email
}
public Object getCached(String key) {
return cache.get(key); // Only uses cache
}
}
// Each method uses different instance variables - low cohesionCorrect (high cohesion - methods use shared variables):
public class Stack {
private int topOfStack = 0;
private List<Integer> elements = new LinkedList<>();
public int size() {
return topOfStack; // Uses topOfStack
}
public void push(int element) {
topOfStack++; // Uses topOfStack
elements.add(element); // Uses elements
}
public int pop() throws EmptyStackException {
if (topOfStack == 0)
throw new EmptyStackException();
int element = elements.get(--topOfStack); // Uses both
elements.remove(topOfStack); // Uses both
return element;
}
}
// Every method uses the same instance variables - high cohesionWhen cohesion breaks down: If a subset of methods only uses a subset of variables, extract those methods and variables into a separate class.
Reference: Clean Code, Chapter 10: Classes
Isolate Classes from Change
Depend on abstractions, not concretions. Classes should depend on interfaces rather than concrete implementations. This enables testing and reduces the impact of change.
Incorrect (depends on concrete implementation):
public class Portfolio {
private NYSEStockExchange exchange;
public Portfolio(NYSEStockExchange exchange) {
this.exchange = exchange;
}
public Money value() {
Money total = Money.ZERO;
for (String symbol : holdings.keySet()) {
// Cannot test without real stock exchange connection
total = total.plus(exchange.currentPrice(symbol)
.times(holdings.get(symbol)));
}
return total;
}
}Correct (depends on abstraction):
public interface StockExchange {
Money currentPrice(String symbol);
}
public class Portfolio {
private StockExchange exchange;
public Portfolio(StockExchange exchange) {
this.exchange = exchange;
}
public Money value() {
Money total = Money.ZERO;
for (String symbol : holdings.keySet()) {
total = total.plus(exchange.currentPrice(symbol)
.times(holdings.get(symbol)));
}
return total;
}
}
// Production
Portfolio portfolio = new Portfolio(new NYSEStockExchange());
// Testing
@Test
public void portfolioValueShouldSumAllHoldings() {
StockExchange mockExchange = mock(StockExchange.class);
when(mockExchange.currentPrice("AAPL")).thenReturn(Money.dollars(150));
Portfolio portfolio = new Portfolio(mockExchange);
portfolio.add("AAPL", 10);
assertEquals(Money.dollars(1500), portfolio.value());
}Dependency Inversion Principle (DIP): High-level modules should not depend on low-level modules. Both should depend on abstractions.
Reference: Clean Code, Chapter 10: Classes
Organize Classes for Change
Classes should be organized so that change is minimally invasive. Following the Open-Closed Principle (OCP): classes should be open for extension but closed for modification.
Incorrect (must modify class for each new SQL type):
public class Sql {
public Sql(String table, Column[] columns) { /* ... */ }
public String create() { /* ... */ }
public String insert(Object[] fields) { /* ... */ }
public String selectAll() { /* ... */ }
public String findByKey(String key) { /* ... */ }
// Must modify this class to add update, delete, etc.
// Each change risks breaking existing functionality
}Correct (extend without modifying):
public abstract class Sql {
protected String table;
protected Column[] columns;
public Sql(String table, Column[] columns) {
this.table = table;
this.columns = columns;
}
public abstract String generate();
}
public class CreateSql extends Sql {
public CreateSql(String table, Column[] columns) {
super(table, columns);
}
public String generate() { /* CREATE TABLE ... */ }
}
public class InsertSql extends Sql {
private Object[] fields;
public InsertSql(String table, Column[] columns, Object[] fields) {
super(table, columns);
this.fields = fields;
}
public String generate() { /* INSERT INTO ... */ }
}
// Adding UpdateSql doesn't require modifying existing classes
public class UpdateSql extends Sql { /* ... */ }Benefits:
- Adding new SQL types = adding new classes
- Existing classes remain untouched
- Each class does one thing
Reference: Clean Code, Chapter 10: Classes
Separate Construction from Use
The startup process of object construction is a separate concern from runtime logic. Applications should separate the main function (which builds objects) from the rest of the system (which uses them).
Incorrect (construction mixed with use):
public class OrderProcessor {
public void processOrder(Order order) {
// Construction buried in business logic
EmailService emailService = new SmtpEmailService(
"smtp.example.com", 587, "user", "password");
InventoryService inventory = new InventoryServiceImpl(
new PostgresConnection("localhost", 5432));
PaymentGateway gateway = new StripeGateway(API_KEY);
// Business logic
inventory.reserve(order.getItems());
gateway.charge(order.getPayment());
emailService.sendConfirmation(order);
}
}Correct (construction separated from use):
// Main builds the object graph
public class Application {
public static void main(String[] args) {
EmailService emailService = new SmtpEmailService(
Config.get("smtp.host"), Config.getInt("smtp.port"));
InventoryService inventory = new InventoryServiceImpl(
new PostgresConnection(Config.get("db.url")));
PaymentGateway gateway = new StripeGateway(Config.get("stripe.key"));
OrderProcessor processor = new OrderProcessor(
emailService, inventory, gateway);
processor.start();
}
}
// Business class receives dependencies
public class OrderProcessor {
private final EmailService emailService;
private final InventoryService inventory;
private final PaymentGateway gateway;
public OrderProcessor(EmailService email, InventoryService inv,
PaymentGateway pay) {
this.emailService = email;
this.inventory = inv;
this.gateway = pay;
}
public void processOrder(Order order) {
inventory.reserve(order.getItems());
gateway.charge(order.getPayment());
emailService.sendConfirmation(order);
}
}Alternative: Use Dependency Injection frameworks (Spring, Guice) to manage construction.
Reference: Clean Code, Chapter 11: Systems
Keep Classes Small
Classes should be small. With functions, we measure size by counting lines. With classes, we count responsibilities. A class should have only one reason to change.
Incorrect (large class with multiple responsibilities):
public class SuperDashboard extends JFrame implements MetaDataUser {
public String getCustomizerLanguagePath() { /* ... */ }
public void setSystemConfigPath(String path) { /* ... */ }
public String getSystemConfigDocument() { /* ... */ }
public void setSystemConfigDocument(String doc) { /* ... */ }
public boolean getGuruState() { /* ... */ }
public boolean getNoviceState() { /* ... */ }
public boolean getOpenSourceState() { /* ... */ }
public void showObject(MetaObject object) { /* ... */ }
public void showProgress(String s) { /* ... */ }
public boolean isMetadataDirty() { /* ... */ }
public void setIsMetadataDirty(boolean dirty) { /* ... */ }
public Component getLastFocusedComponent() { /* ... */ }
// 70+ more methods...
}Correct (small focused classes):
public class Version {
public int getMajorVersionNumber() { /* ... */ }
public int getMinorVersionNumber() { /* ... */ }
public int getBuildNumber() { /* ... */ }
}
public class Dashboard extends JFrame {
private final Version version;
private final UserPreferences preferences;
private final ProgressIndicator progress;
public void show() { /* ... */ }
}
public class UserPreferences {
public Language getLanguage() { /* ... */ }
public boolean isNovice() { /* ... */ }
public boolean isGuru() { /* ... */ }
}
public class ProgressIndicator {
public void showProgress(String message) { /* ... */ }
public void hideProgress() { /* ... */ }
}Single Responsibility Principle (SRP): A class should have one, and only one, reason to change.
Reference: Clean Code, Chapter 10: Classes
Delete Commented-Out Code
Commented-out code is an abomination. Others who see it will not have the courage to delete it. They think it must be there for a reason. It rots and becomes increasingly irrelevant.
Incorrect (commented-out code persisting):
public void processOrder(Order order) {
validateOrder(order);
// calculateLegacyDiscount(order);
// if (order.getCustomer().isPremium()) {
// applyPremiumDiscount(order);
// }
calculateDiscount(order);
// sendNotification(order);
// logOrderDetails(order);
saveOrder(order);
notifyWarehouse(order);
}Correct (clean code, trust version control):
public void processOrder(Order order) {
validateOrder(order);
calculateDiscount(order);
saveOrder(order);
notifyWarehouse(order);
}Why delete it?
- Version control remembers everything
- Commented code creates confusion: Is it needed? Why is it there?
- It clutters the file and makes scanning harder
- It becomes stale and misleading over time
If you really need it: Create a git branch, or add a reference comment pointing to the commit where it was removed.
Reference: Clean Code, Chapter 4: Comments
Avoid Redundant Comments
A comment that merely restates what the code does is worse than no comment. It adds clutter, becomes stale, and trains readers to ignore comments entirely.
Incorrect (redundant comments):
/**
* The name of the customer
*/
private String customerName;
/**
* Returns the name
* @return the name
*/
public String getName() {
return name;
}
// Check if the account is closed
if (account.isClosed()) {
// Throw an exception
throw new AccountClosedException();
}
/**
* Default constructor
*/
public Account() {
}Correct (comment only when necessary):
private String customerName;
public String getName() {
return name;
}
if (account.isClosed()) {
throw new AccountClosedException();
}
public Account() {
}
// Comments that add value:
// RFC 2822 date format required by legacy email parser
private static final String DATE_FORMAT = "EEE, dd MMM yyyy HH:mm:ss Z";
// Thread-safe: ConcurrentHashMap handles synchronization
private Map<String, Session> activeSessions = new ConcurrentHashMap<>();Rule of thumb: If the comment just repeats the code in English, delete it.
Reference: Clean Code, Chapter 4: Comments
Use Comments to Explain Intent
Good comments explain WHY, not WHAT. The code already tells you what it does. Comments should explain the reasoning behind a decision that cannot be expressed in code.
Incorrect (narrating the code):
// Increment counter by 1
counter++;
// Loop through all employees
for (Employee employee : employees) {
// Check if employee is active
if (employee.isActive()) {
// Add employee to list
activeEmployees.add(employee);
}
}
// Set the name
this.name = name;Correct (explaining intent):
// We use insertion sort here because the list is almost always nearly sorted,
// and insertion sort is O(n) for nearly sorted data vs O(n log n) for quicksort
insertionSort(nearlyOrderedList);
// Format matches the external API's expected date format (ISO 8601 without timezone)
// See: https://api.vendor.com/docs/date-format
String formattedDate = date.format(DateTimeFormatter.ISO_LOCAL_DATE);
// Bias toward newer sessions - users expect recent data first
// Business requirement from Product (ticket PROD-1234)
sessions.sort(Comparator.comparing(Session::getCreatedAt).reversed());Good comment situations:
- Legal comments (copyright, license)
- Explanation of intent or rationale
- Clarification of obscure API behavior
- Warning of consequences
- TODO comments (temporary)
Reference: Clean Code, Chapter 4: Comments
Express Yourself in Code, Not Comments
In many cases, creating a function that says the same thing as the comment you wanted to write is better. Code can be refactored to be self-explanatory.
Incorrect (comment explains obscure code):
// Check to see if the employee is eligible for full benefits
if ((employee.flags & HOURLY_FLAG) && (employee.age > 65)) {
// ...
}
// Add 30 days to the current date
Date newDate = new Date(currentDate.getTime() + (30L * 24 * 60 * 60 * 1000));
// Returns true if the string contains only digits
boolean result = str.matches("[0-9]+");Correct (code expresses intent):
if (employee.isEligibleForFullBenefits()) {
// ...
}
// In Employee class:
public boolean isEligibleForFullBenefits() {
return isHourlyWorker() && age > RETIREMENT_AGE;
}
Date newDate = currentDate.plusDays(30);
boolean result = StringUtils.isNumeric(str);
// Or define your own:
boolean result = containsOnlyDigits(str);Benefits:
- Function names are searchable
- Functions can be tested
- Code documents itself
- No risk of comment becoming stale
When comments are still valuable:
- Why, not what: Comments explaining why a decision was made (business rules, regulatory requirements, algorithm choices) cannot be replaced by code.
- Warnings: Thread-safety, performance implications, or non-obvious side effects deserve explicit comments.
- Legal/license comments: Required by policy and cannot be expressed as code.
- Public API documentation: Javadoc/docstrings for public APIs are expected and useful.
Reference: Clean Code, Chapter 4: Comments
Use Warning Comments for Consequences
Comments that warn other programmers about certain consequences are valuable. They prevent others from making mistakes that have non-obvious impacts.
Incorrect (no warning about hidden dangers):
public static SimpleDateFormat makeStandardDateFormat() {
return new SimpleDateFormat("yyyy-MM-dd");
}
public void runExpensiveOperation() {
// Process all historical data
processAllRecords();
}
@Test
public void testWithRealDatabase() {
database.connect();
// ...
}Correct (warnings prevent mistakes):
// WARNING: DateTimeFormatter is thread-safe, but this legacy method returns
// SimpleDateFormat which is NOT thread-safe. Do not cache or share the result.
// Consider migrating callers to use DateTimeFormatter.ISO_LOCAL_DATE directly.
public static SimpleDateFormat makeStandardDateFormat() {
return new SimpleDateFormat("yyyy-MM-dd");
}
// Preferred (Java 8+): thread-safe, no warning needed
private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd");
// WARNING: This operation takes ~30 minutes on production data
// and consumes significant memory. Run only during maintenance windows.
public void runExpensiveOperation() {
processAllRecords();
}
// Don't run this test unless you have several hours to kill.
// Requires real database connection and processes 10M+ records.
@Ignore("Long-running integration test")
@Test
public void testWithRealDatabase() {
database.connect();
// ...
}Good warnings include:
- Thread-safety concerns
- Performance implications
- External dependencies or side effects
- Security considerations
Reference: Clean Code, Chapter 4: Comments
Maximize Expressiveness
The majority of the cost of a software project is in long-term maintenance. Write code for the reader, not the writer. The clearer the author can make the code, the less time others will spend understanding it.
Incorrect (clever but opaque):
// Bit manipulation for no reason
public boolean isWeekend(int day) {
return ((day & 0x6) == day) && (day > 0);
}
// Terse variable names in complex logic
public int calc(int[] a, int n) {
int r = 0;
for (int i = 0; i < n; i++)
r = Math.max(r, a[i] - (i > 0 ? a[i-1] : 0));
return r;
}Correct (clear and expressive):
public boolean isWeekend(DayOfWeek day) {
return day == DayOfWeek.SATURDAY || day == DayOfWeek.SUNDAY;
}
public int findMaxDailyGain(int[] dailyPrices, int numberOfDays) {
int maxGain = 0;
for (int day = 1; day < numberOfDays; day++) {
int dailyChange = dailyPrices[day] - dailyPrices[day - 1];
maxGain = Math.max(maxGain, dailyChange);
}
return maxGain;
}Techniques for expressiveness:
- Choose good names (the single most effective technique)
- Keep functions and classes small and focused
- Use standard patterns and nomenclature — when you use the Strategy pattern, name the class with "Strategy" so readers recognize the intent
- Write well-crafted unit tests — tests are documentation by example
Reference: Clean Code, Chapter 12: Emergence
Follow the Four Rules of Simple Design
Kent Beck's four rules of Simple Design, in order of priority:
1. Passes all tests — correctness is non-negotiable 2. Reveals intent — code is clear and expressive 3. No duplication — every concept has a single representation 4. Fewest elements — remove anything that doesn't serve the first three
Incorrect (over-engineered for imaginary requirements):
// Single implementation with unnecessary abstraction layers
public interface PaymentProcessor {}
public interface PaymentProcessorFactory {}
public abstract class AbstractPaymentProcessor implements PaymentProcessor {}
public class PaymentProcessorFactoryImpl implements PaymentProcessorFactory {
public PaymentProcessor create() {
return new DefaultPaymentProcessor();
}
}
public class DefaultPaymentProcessor extends AbstractPaymentProcessor {
public void process(Payment payment) {
gateway.charge(payment.getAmount());
}
}Correct (simplest design that works):
public class PaymentProcessor {
private final PaymentGateway gateway;
public PaymentProcessor(PaymentGateway gateway) {
this.gateway = gateway;
}
public void process(Payment payment) {
gateway.charge(payment.getAmount());
}
}Rule 4 is the most overlooked: After making code correct, expressive, and DRY, actively look for things to remove. Every class, method, and variable should justify its existence. If you have one implementation of an interface, question whether you need the interface. If a design pattern adds indirection without a current concrete benefit, remove it.
When more structure is justified:
- When you have 2+ implementations today (not hypothetically)
- When a framework requires it (e.g., dependency injection interfaces)
- When tests need seams for mocking external dependencies
Reference: Clean Code, Chapter 12: Emergence
Avoid Returning and Passing Null
Returning null creates work for callers and invites errors. One missing null check leads to NullPointerException. Throw exceptions, return empty collections, or use the Special Case / Null Object pattern.
Incorrect (returning null):
public List<Employee> getEmployees() {
if (thereAreNoEmployees) {
return null; // Caller must check for null
}
return employees;
}
// Every caller must do this
List<Employee> employees = getEmployees();
if (employees != null) { // Easy to forget
for (Employee e : employees) {
pay(e);
}
}
// Passing null
public double calculatePay(Employee employee, Money bonus) {
// What if bonus is null?
}
calculatePay(employee, null); // What does null mean?Correct (avoid null entirely):
// Return empty collection instead of null
public List<Employee> getEmployees() {
if (thereAreNoEmployees) {
return Collections.emptyList();
}
return employees;
}
// Clean caller code
for (Employee e : getEmployees()) {
pay(e);
}
// Use Optional for truly optional values
public Optional<Employee> findById(Long id) {
return Optional.ofNullable(employeeMap.get(id));
}
// Use Special Case object
public interface BillingPlan {
Money getRate();
}
public class NullBillingPlan implements BillingPlan {
public Money getRate() {
return Money.ZERO;
}
}
// Explicit parameters instead of null
public double calculatePay(Employee employee, Money bonus) { /* ... */ }
public double calculatePayWithoutBonus(Employee employee) { /* ... */ }When null/nullable is acceptable:
- Language-level nullable types with compiler enforcement (Kotlin
?, TypeScriptstrictNullChecks, RustOption<T>) make null safe because the compiler forces handling. The problem is untracked nullability, not the concept itself. - Interop with APIs or frameworks that require null (e.g., JDBC, some serialization libraries).
Reference: Clean Code, Chapter 7: Error Handling
Define Exceptions by Caller Needs
Define exception classes based on how they are caught, not on their source or type. Often a single exception class is sufficient for a particular area of code. Wrap third-party API exceptions.
Incorrect (catching many exception types):
// Caller must handle many different exceptions
try {
port.open();
} catch (DeviceResponseException e) {
reportPortError(e);
logger.log("Device response problem", e);
} catch (ATM1212UnlockedException e) {
reportPortError(e);
logger.log("Unlock exception", e);
} catch (GMXError e) {
reportPortError(e);
logger.log("Device response exception", e);
} finally {
// ...
}Correct (unified exception handling):
// Wrapper class translates exceptions
public class LocalPort {
private ACMEPort innerPort;
public void open() throws PortDeviceFailure {
try {
innerPort.open();
} catch (DeviceResponseException e) {
throw new PortDeviceFailure(e);
} catch (ATM1212UnlockedException e) {
throw new PortDeviceFailure(e);
} catch (GMXError e) {
throw new PortDeviceFailure(e);
}
}
}
// Clean caller code
try {
port.open();
} catch (PortDeviceFailure e) {
reportPortError(e);
logger.log(e.getMessage(), e);
}Benefits of wrapping:
- Minimizes dependencies on third-party APIs
- Easier to mock for testing
- Easier to switch implementations later
- Single exception type per area simplifies catching
Reference: Clean Code, Chapter 7: Error Handling
Provide Context with Exceptions
Each exception should provide enough context to determine the source and location of an error. Create informative error messages that describe the operation that failed and the type of failure.
Incorrect (no context):
throw new Exception("Error occurred");
throw new RuntimeException("Failed");
throw new DataAccessException("Query failed");
// Stack trace alone doesn't tell you which order, which customer
catch (SQLException e) {
throw new RuntimeException(e);
}Correct (rich context):
throw new OrderProcessingException(
String.format("Failed to process order %s for customer %s: %s",
orderId, customerId, "Insufficient inventory for item SKU-12345"));
throw new ConfigurationException(
"Cannot load configuration from " + configPath +
". Expected format: YAML. Check file permissions and syntax.");
catch (SQLException e) {
throw new DataAccessException(
String.format("Failed to insert order %s into table %s. " +
"Constraint violated: %s",
order.getId(), "orders", e.getMessage()),
e); // Preserve original exception
}Include in exception messages:
- What operation was being attempted
- What data was involved (IDs, names, quantities)
- Why it failed (the specific error condition)
- Preserve the original exception when wrapping
Reference: Clean Code, Chapter 7: Error Handling
Separate Error Handling from Happy Path
Error handling is important, but when it obscures logic, it's wrong. The core insight is separation: keep the happy path visible and error handling consolidated, regardless of which mechanism your language uses.
Incorrect (error handling obscures business logic):
if (deletePage(page) == E_OK) {
if (registry.deleteReference(page.name) == E_OK) {
if (configKeys.deleteKey(page.name.makeKey()) == E_OK) {
logger.log("page deleted");
} else {
logger.log("configKey not deleted");
}
} else {
logger.log("deleteReference from registry failed");
}
} else {
logger.log("delete failed");
return E_ERROR;
}Correct (Java/C#/Python — exceptions separate concerns):
public void delete(Page page) {
try {
deletePageAndAllReferences(page);
} catch (PageDeletionException e) {
logger.log(e.getMessage());
}
}
private void deletePageAndAllReferences(Page page) throws PageDeletionException {
deletePage(page);
registry.deleteReference(page.name);
configKeys.deleteKey(page.name.makeKey());
}Correct (Go — explicit error returns with early return):
func (s *Service) Delete(page Page) error {
if err := s.deletePage(page); err != nil {
return fmt.Errorf("deleting page: %w", err)
}
if err := s.registry.DeleteReference(page.Name); err != nil {
return fmt.Errorf("deleting reference: %w", err)
}
if err := s.configKeys.DeleteKey(page.Name); err != nil {
return fmt.Errorf("deleting config key: %w", err)
}
return nil
}Correct (Rust — Result type with ? operator):
fn delete(&self, page: &Page) -> Result<(), DeletionError> {
self.delete_page(page)?;
self.registry.delete_reference(&page.name)?;
self.config_keys.delete_key(&page.name)?;
Ok(())
}The principle is language-agnostic: separate error-handling logic from business logic. The mechanism differs:
- Java/C#/Python: try-catch with specific exception types
- Go: explicit error returns with early return pattern
- Rust:
Result<T, E>with?operator - TypeScript/Kotlin:
Resulttypes or exceptions depending on the domain
When NOT to use exceptions:
- In Go, Rust, or other languages designed around explicit error values — use their idiomatic patterns instead
- For expected business outcomes (e.g., "user not found") — consider returning
Optionalor a domain-specific result type rather than throwing - In performance-critical hot paths where exception overhead matters
Reference: Clean Code, Chapter 3: Functions and Chapter 7: Error Handling
Write Try-Catch-Finally First
When writing code that could throw exceptions, start with the try-catch-finally statement. This helps define what the user of the code should expect, regardless of what goes wrong in the try block.
Incorrect (error handling added as afterthought):
// Written without considering error cases
public List<RecordedGrip> retrieveSection(String sectionName) {
FileInputStream stream = new FileInputStream(sectionName);
BufferedReader reader = new BufferedReader(new InputStreamReader(stream));
String line;
List<RecordedGrip> grips = new ArrayList<>();
while ((line = reader.readLine()) != null) {
grips.add(parseGrip(line));
}
return grips;
// What if file doesn't exist? What if parsing fails?
}Correct (start with try-catch structure):
// TDD approach: start with test for exception case
@Test(expected = StorageException.class)
public void retrieveSectionShouldThrowOnInvalidFileName() {
sectionStore.retrieveSection("invalid-file");
}
// Then write code starting with try-catch
public List<RecordedGrip> retrieveSection(String sectionName) throws StorageException {
try (var stream = new FileInputStream(sectionName)) {
return readGrips(stream);
} catch (IOException e) {
throw new StorageException("Error reading section: " + sectionName, e);
}
}Benefits:
- Forces you to consider error cases first
- Establishes clear transaction boundaries
- The finally block ensures cleanup happens
Reference: Clean Code, Chapter 7: Error Handling
Avoid Horizontal Alignment
Horizontal alignment of variable declarations or assignments looks nice but creates maintenance problems. When one line changes length, all aligned lines need reformatting.
Incorrect (horizontal alignment):
private Socket socket;
private InputStream input;
private OutputStream output;
private Request request;
private Response response;
private FitNesseContext context;
private long requestParsingTimeLimit;
private long requestProgress;
public void setResponse(Response response) { this.response = response; }
public void setSocket(Socket socket) { this.socket = socket; }
public void setContext(FitNesseContext context) { this.context = context; }Correct (natural formatting):
private Socket socket;
private InputStream input;
private OutputStream output;
private Request request;
private Response response;
private FitNesseContext context;
private long requestParsingTimeLimit;
private long requestProgress;
public void setResponse(Response response) {
this.response = response;
}
public void setSocket(Socket socket) {
this.socket = socket;
}
public void setContext(FitNesseContext context) {
this.context = context;
}Why avoid alignment?
- Adding a longer variable name requires reformatting all lines
- Eyes are drawn to the wrong thing (the alignment, not the names)
- Most formatters don't preserve it anyway
Reference: Clean Code, Chapter 5: Formatting
Respect Indentation Rules
Indentation makes the scope hierarchy visible. Each level of indentation represents a nested scope. Never collapse short statements onto one line to save space.
Incorrect (collapsed structure):
public class CommentWidget extends TextWidget {
public CommentWidget(ParentWidget parent, String text) { super(parent, text); }
public String render() throws Exception { return ""; }
}
public void process() { for (int i = 0; i < 10; i++) { if (valid(i)) { execute(i); } } }
if (condition) return false; else return true;Correct (proper indentation):
public class CommentWidget extends TextWidget {
public CommentWidget(ParentWidget parent, String text) {
super(parent, text);
}
public String render() throws Exception {
return "";
}
}
public void process() {
for (int i = 0; i < 10; i++) {
if (valid(i)) {
execute(i);
}
}
}
if (condition) {
return false;
} else {
return true;
}Why maintain indentation?
- Visual scanning relies on indentation
- Structure is immediately apparent
- Debugging is easier when scope is visible
- Merges are cleaner with consistent formatting
Reference: Clean Code, Chapter 5: Formatting
Follow Team Formatting Rules
A team of developers should agree upon a single formatting style. Every member should use that style. The goal is to make the software have a consistent style, as if written by one person.
Incorrect (inconsistent styles mixed):
// Developer A's style
public void processOrder(Order order){
if(order.isValid()){
order.process();
}
}
// Developer B's style
public void processPayment( Payment payment )
{
if ( payment.isValid() )
{
payment.process();
}
}
// Developer C's style
public void processShipment(Shipment shipment) {
if (shipment.isValid()) {
shipment.process(); }}Correct (consistent team style):
public void processOrder(Order order) {
if (order.isValid()) {
order.process();
}
}
public void processPayment(Payment payment) {
if (payment.isValid()) {
payment.process();
}
}
public void processShipment(Shipment shipment) {
if (shipment.isValid()) {
shipment.process();
}
}Implementation:
- Document the team's coding standards
- Use automated formatters (Prettier, Black, google-java-format)
- Enforce in CI/CD pipelines
- Configure IDE to format on save
Your personal style preferences are less important than team consistency.
Reference: Clean Code, Chapter 5: Formatting
Use Vertical Formatting for Readability
Code should read like a newspaper article: headline at the top, details as you go down. Use vertical whitespace to separate concepts. Keep related code close together.
Incorrect (no vertical organization):
public class WikiPageResponder implements SecureResponder {
private static final String CONTENT_TYPE = "text/html";
private WikiPage page;
private PageData pageData;
private String pageTitle;
private Request request;
private PageCrawler crawler;
public Response makeResponse(FitNesseContext context, Request request) {
this.request = request;
this.page = loadPage();
if (page == null)
return notFoundResponse(context, request);
return makePageResponse(context);
}
private WikiPage loadPage() {
String resource = request.getResource();
return PageCrawlerImpl.getPageCrawler(context.root).getPage(resource);
}
}Correct (vertical formatting applied):
public class WikiPageResponder implements SecureResponder {
private static final String CONTENT_TYPE = "text/html";
private WikiPage page;
private PageData pageData;
private String pageTitle;
private Request request;
private PageCrawler crawler;
public Response makeResponse(FitNesseContext context, Request request) {
this.request = request;
this.page = loadPage();
if (page == null)
return notFoundResponse(context, request);
return makePageResponse(context);
}
private WikiPage loadPage() {
String resource = request.getResource();
return PageCrawlerImpl.getPageCrawler(context.root).getPage(resource);
}
}Guidelines:
- Blank lines between concepts (methods, logical sections)
- Related code grouped together
- Caller above callee (top-down reading)
- Variable declarations close to their usage
Reference: Clean Code, Chapter 5: Formatting
Maintain One Level of Abstraction
Statements within a function should all be at the same level of abstraction. Mixing high-level concepts with low-level details is confusing. Code should read like a top-down narrative.
Incorrect (mixed abstraction levels):
public void analyzeDocument(String documentPath) {
// High level
Document document = loadDocument(documentPath);
// Low level - string manipulation details
String text = document.getText();
text = text.replaceAll("\\s+", " ");
text = text.toLowerCase();
String[] words = text.split(" ");
// High level again
WordFrequency frequency = analyzeFrequency(words);
// Low level - file I/O details
FileOutputStream fos = new FileOutputStream("output.txt");
BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(fos));
for (Map.Entry<String, Integer> entry : frequency.getEntries()) {
writer.write(entry.getKey() + ": " + entry.getValue());
writer.newLine();
}
writer.close();
}Correct (consistent abstraction level):
public void analyzeDocument(String documentPath) {
Document document = loadDocument(documentPath);
String normalizedText = normalizeText(document);
WordFrequency frequency = analyzeFrequency(normalizedText);
saveResults(frequency);
}
private String normalizeText(Document document) {
String text = document.getText();
text = collapseWhitespace(text);
text = toLowerCase(text);
return text;
}
private void saveResults(WordFrequency frequency) {
ResultWriter writer = new ResultWriter("output.txt");
writer.write(frequency);
}Read the code from top to bottom. Each function introduces the next level of abstraction.
When mixing levels is acceptable:
- Short utility functions (under 10 lines) that perform one task can mix levels without confusion. The overhead of extracting one-line helpers often exceeds the readability benefit.
- Test code frequently mixes setup (low-level) with assertions (high-level) — this is expected and follows the Arrange-Act-Assert pattern.
Reference: Clean Code, Chapter 3: Functions
Separate Commands from Queries
Functions should either do something or answer something, but not both. Either your function changes the state of an object, or it returns information about that object.
Incorrect (command and query mixed):
// Does this set the attribute and return success?
// Or check if attribute exists?
public boolean set(String attribute, String value) {
// ... sets attribute and returns true if it was set
}
// Confusing usage
if (set("username", "bob")) {
// Was username set, or did it already exist?
}Correct (separated command and query):
// Query - returns information
public boolean attributeExists(String attribute) {
return attributes.containsKey(attribute);
}
// Command - changes state
public void setAttribute(String attribute, String value) {
attributes.put(attribute, value);
}
// Clear usage
if (!attributeExists("username")) {
setAttribute("username", "bob");
}Alternative (return new state for immutable design):
// For immutable patterns, returning new state is acceptable
public Settings withAttribute(String attribute, String value) {
return new Settings(this.attributes.plus(attribute, value));
}The separation makes code easier to read and reason about.
Reference: Clean Code, Chapter 3: Functions
Do Not Repeat Yourself
Duplication is the root of all evil in software. When an algorithm changes, you must change it in multiple places. When you fix a bug, you must remember to fix it everywhere.
Incorrect (duplicated logic):
public void processNewEmployee(Employee employee) {
if (employee.getName() == null || employee.getName().trim().isEmpty()) {
throw new ValidationException("Name is required");
}
if (employee.getEmail() == null || !employee.getEmail().contains("@")) {
throw new ValidationException("Valid email is required");
}
employeeRepository.save(employee);
emailService.sendWelcome(employee.getEmail());
}
public void updateEmployee(Employee employee) {
if (employee.getName() == null || employee.getName().trim().isEmpty()) {
throw new ValidationException("Name is required"); // Duplicated
}
if (employee.getEmail() == null || !employee.getEmail().contains("@")) {
throw new ValidationException("Valid email is required"); // Duplicated
}
employeeRepository.update(employee);
}Correct (extracted common logic):
public void processNewEmployee(Employee employee) {
validateEmployee(employee);
employeeRepository.save(employee);
emailService.sendWelcome(employee.getEmail());
}
public void updateEmployee(Employee employee) {
validateEmployee(employee);
employeeRepository.update(employee);
}
private void validateEmployee(Employee employee) {
requireNonEmpty(employee.getName(), "Name is required");
requireValidEmail(employee.getEmail());
}
private void requireNonEmpty(String value, String message) {
if (value == null || value.trim().isEmpty()) {
throw new ValidationException(message);
}
}Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.
When NOT to apply DRY:
- Coincidental duplication: Two code blocks look similar but serve different purposes and will evolve independently. Extracting a shared function creates a false coupling that makes both harder to change later.
- Premature abstraction: If the code has only been duplicated once, wait. The third occurrence reveals the true abstraction. Premature extraction often produces awkward, over-parameterized helpers.
- Cross-boundary duplication: Sometimes duplication across microservices or bounded contexts is preferable to introducing a shared library that couples the services.
Reference: Clean Code, Chapter 3: Functions
Minimize 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.
Incorrect (too many arguments):
// Flag arguments are bad - function does different things
public void render(boolean isSuite) {}
// Four arguments - hard to remember order
public Circle makeCircle(double x, double y, double radius, String color) {}
// Order matters and is easy to confuse
public void assertExpectedEqualsActual(String expected, String actual) {}Correct (minimized arguments):
// Separate functions instead of flag
public void renderForSingleTest() {}
public void renderForSuite() {}
// Encapsulate related arguments in objects
public Circle makeCircle(Point center, double radius, Color color) {}
// Even better - use a builder for many optional params
Circle circle = Circle.builder()
.center(new Point(0, 0))
.radius(5.0)
.color(Color.RED)
.build();
// Self-documenting method name
assertThat(actual).isEqualTo(expected);Why fewer arguments?
- Easier to understand
- Easier to test (fewer combinations)
- Less chance of argument order errors
Acceptable monadic forms:
- Asking a question:
boolean fileExists(String path) - Transforming input:
InputStream openFile(String path) - Events:
void passwordAttemptFailed(int attempts)
Reference: Clean Code, Chapter 3: Functions
Avoid Side Effects
Side effects are lies. Your function promises to do one thing, but it also does other hidden things. Avoid unexpected changes to class variables, globals, or passed arguments.
Incorrect (hidden side effect):
public class UserValidator {
private Cryptographer cryptographer;
private Session session;
public boolean checkPassword(String userName, String password) {
User user = UserGateway.findByName(userName);
if (user != null) {
String codedPhrase = user.getPhraseEncodedByPassword();
String phrase = cryptographer.decrypt(codedPhrase, password);
if ("Valid Password".equals(phrase)) {
session.initialize(); // Hidden side effect!
return true;
}
}
return false;
}
}
// Calling checkPassword twice destroys the session unexpectedlyCorrect (explicit behavior):
public class UserValidator {
private Cryptographer cryptographer;
public boolean checkPassword(String userName, String password) {
User user = UserGateway.findByName(userName);
if (user == null) return false;
String codedPhrase = user.getPhraseEncodedByPassword();
String phrase = cryptographer.decrypt(codedPhrase, password);
return "Valid Password".equals(phrase);
}
}
// Separate function with clear name
public class SessionManager {
public void initializeSessionForUser(String userName, String password) {
if (userValidator.checkPassword(userName, password)) {
session.initialize();
}
}
}If you must have a temporal coupling, make it explicit in the function name: checkPasswordAndInitializeSession.
When side effects are expected:
- I/O operations (file writes, network calls, database mutations) inherently have side effects — the principle here is about hidden side effects, not avoiding all mutation. A function named
saveUser()clearly signals side effects. - Event handlers, middleware, and lifecycle hooks are designed to produce side effects. Name them to make the effect obvious.
Reference: Clean Code, Chapter 3: Functions
Functions Should Do One Thing
Functions should do one thing. They should do it well. They should do it only. If a function does multiple things, extract those things into separate functions.
Incorrect (function does multiple things):
public void processEmployee(Employee employee) {
// Validate employee
if (employee.getName() == null || employee.getName().isEmpty()) {
throw new IllegalArgumentException("Name required");
}
if (employee.getSalary() < 0) {
throw new IllegalArgumentException("Salary cannot be negative");
}
// Calculate tax
double tax = employee.getSalary() * 0.3;
employee.setTax(tax);
// Save to database
Connection conn = getConnection();
PreparedStatement stmt = conn.prepareStatement(
"INSERT INTO employees VALUES (?, ?, ?)");
stmt.setString(1, employee.getName());
stmt.setDouble(2, employee.getSalary());
stmt.setDouble(3, tax);
stmt.execute();
// Send email notification
EmailService.send(employee.getEmail(), "Welcome!");
}Correct (each function does one thing):
public void processEmployee(Employee employee) {
validateEmployee(employee);
calculateTax(employee);
saveEmployee(employee);
sendWelcomeEmail(employee);
}
private void validateEmployee(Employee employee) {
validateName(employee);
validateSalary(employee);
}
private void calculateTax(Employee employee) {
double tax = employee.getSalary() * TAX_RATE;
employee.setTax(tax);
}
private void saveEmployee(Employee employee) {
employeeRepository.save(employee);
}
private void sendWelcomeEmail(Employee employee) {
emailService.sendWelcome(employee.getEmail());
}Test: If you can extract another function from it with a name that is not merely a restatement of its implementation, the function is doing more than one thing.
When NOT to split:
- If the extracted function's name would just restate its body (e.g.,
calculateTaxcontaining onlysalary * TAX_RATE), the extraction adds indirection without clarity. - Orchestration functions that call a sequence of steps at the same abstraction level (like
processEmployeeabove) are doing "one thing" — orchestrating. Don't try to extract the orchestration further.
Reference: Clean Code, Chapter 3: Functions
Keep Functions Small
Functions should be small. They should be smaller than that. Functions should hardly ever be 20 lines long. Each function should do one thing, do it well, and do it only.
Incorrect (large function doing multiple things):
public void renderPageWithSetupsAndTeardowns(PageData pageData, boolean isSuite) {
if (isTestPage(pageData)) {
WikiPage testPage = pageData.getWikiPage();
StringBuffer newPageContent = new StringBuffer();
includeSetupPages(testPage, newPageContent, isSuite);
newPageContent.append(pageData.getContent());
includeTeardownPages(testPage, newPageContent, isSuite);
pageData.setContent(newPageContent.toString());
}
}
private void includeSetupPages(WikiPage testPage, StringBuffer buffer, boolean isSuite) {
if (isSuite) {
WikiPage suitePage = PageCrawlerImpl.getInheritedPage(
SuiteResponder.SUITE_SETUP_NAME, testPage);
if (suitePage != null) {
WikiPagePath pagePath = testPage.getPageCrawler()
.getFullPath(suitePage);
String pagePathName = PathParser.render(pagePath);
buffer.append("!include -setup .")
.append(pagePathName)
.append("\n");
}
}
// ... 20 more lines
}Correct (small, focused functions):
public void renderPageWithSetupsAndTeardowns(PageData pageData, boolean isSuite) {
if (isTestPage(pageData))
includeSetupsAndTeardowns(pageData, isSuite);
}
private void includeSetupsAndTeardowns(PageData pageData, boolean isSuite) {
WikiPage testPage = pageData.getWikiPage();
String content = buildPageContent(testPage, isSuite);
pageData.setContent(content);
}
private String buildPageContent(WikiPage testPage, boolean isSuite) {
return getSetups(testPage, isSuite) +
testPage.getContent() +
getTeardowns(testPage, isSuite);
}Each function is 3-5 lines. Each does exactly one thing. Each is at one level of abstraction.
When NOT to apply this pattern:
- If a 15-20 line function does one thing at one level of abstraction, it does not need further decomposition. The goal is comprehensibility, not minimizing line count.
- Do not extract functions that are only called from one place and whose names merely restate their implementation — this adds indirection without clarity.
- Ousterhout's "deep modules" critique: extremely small functions can create "shallow modules" that spread logic across many files, forcing readers to jump between functions to understand a single behavior. Balance depth (functionality per interface) against size.
Reference: Clean Code, Chapter 3: Functions
Avoid Disinformation in Names
Avoid names that encode false information. Do not refer to a grouping as a "List" unless it is actually a List. Avoid names that vary in small ways that are hard to distinguish.
Incorrect (misleading type information):
// Not actually a List, it's a Map
private Map<String, Account> accountList;
// These look almost identical - easy to confuse
private String XYZControllerForEfficientHandlingOfStrings;
private String XYZControllerForEfficientStorageOfStrings;
// Lowercase L looks like 1, uppercase O looks like 0
int a = l;
if (O == l)
a = O1;Correct (accurate and distinguishable):
// Accurately describes the data structure
private Map<String, Account> accountsByName;
// Clearly distinguishable names
private String stringProcessingController;
private String stringStorageController;
// Clear variable names
int result = leftValue;
if (originalValue == leftValue)
result = outputValue;Reference: Clean Code, Chapter 2: Meaningful Names
Avoid Encodings in Names
Encoding type or scope information into names adds an extra burden of deciphering. Modern IDEs make Hungarian Notation and member prefixes unnecessary.
Incorrect (encoded type and scope):
// Hungarian Notation - type encoded in name
String strName;
int iAge;
boolean bIsActive;
PhoneNumber phoneString; // Type changed but name wasn't updated!
// Member prefixes
public class Part {
private String m_dsc; // Member description
void setDescription(String dsc) {
m_dsc = dsc;
}
}
// Interface prefix
public interface IShapeFactory {}Correct (no encodings):
// Let the type system handle types
String name;
int age;
boolean isActive;
PhoneNumber phone;
// No member prefixes - IDE highlights members
public class Part {
private String description;
void setDescription(String description) {
this.description = description;
}
}
// Drop the interface prefix — callers use ShapeFactory, not IShapeFactory
public interface ShapeFactory {}
// Name implementations by what distinguishes them
public class JsonShapeFactory implements ShapeFactory {}
public class SvgShapeFactory implements ShapeFactory {}
// If only one implementation exists, consider whether you need the interface at allReaders learn to ignore prefixes. You end up seeing only the meaningful part of the name.
Note: ShapeFactoryImpl is a lesser evil than IShapeFactory, but Impl is still an encoding. When possible, name implementations by their distinguishing characteristic (protocol, storage mechanism, algorithm).
Reference: Clean Code, Chapter 2: Meaningful Names
Use Noun Phrases for Class Names
Class names should be nouns or noun phrases. A class represents a thing or concept. Avoid verbs, manager-style names, and vague words.
Incorrect (verbs, vague, or manager-style):
// Verb as class name
class ProcessPayment {}
// Vague manager names
class Manager {}
class Processor {}
class Data {}
class Info {}
// Unnecessary suffixes
class CustomerManager {}
class AccountProcessor {}
class PaymentHandler {}Correct (specific noun phrases):
// Clear nouns representing entities
class Payment {}
class PaymentGateway {}
// Specific, descriptive nouns
class Customer {}
class WikiPage {}
class Account {}
class AddressParser {}
// Role-specific names when needed
class PaymentValidator {} // Validates payments
class AccountRepository {} // Stores accountsWhen NOT to use this pattern:
Manager-style names are acceptable when the class genuinely manages a resource lifecycle:
ConnectionPool- manages connection lifecycleThreadPoolExecutor- manages thread lifecycle
Reference: Clean Code, Chapter 2: Meaningful Names
Use Intention-Revealing Names
Names should reveal intent. A name should tell you why something exists, what it does, and how it is used. If a name requires a comment, it does not reveal its intent.
Incorrect (requires mental decoding):
int d; // elapsed time in days
public List<int[]> getThem() {
List<int[]> list1 = new ArrayList<>();
for (int[] x : theList)
if (x[0] == 4) // What is 4? What is x[0]?
list1.add(x);
return list1;
}Correct (self-documenting):
int elapsedTimeInDays;
public List<Cell> getFlaggedCells() {
List<Cell> flaggedCells = new ArrayList<>();
for (Cell cell : gameBoard)
if (cell.isFlagged())
flaggedCells.add(cell);
return flaggedCells;
}The code now explicitly communicates its purpose: finding flagged cells in a minesweeper game. No comment needed.
When short names are acceptable:
- Loop counters (
i,j,k) in small, tightly-scoped loops where the index has no domain meaning. - Mathematical algorithms where
x,y,dx,dtare conventional and understood by the target audience. - Lambda parameters in trivial callbacks:
items.filter(x -> x > 0)is clearer thanitems.filter(numberThatMustBePositive -> numberThatMustBePositive > 0).
Reference: Clean Code, Chapter 2: Meaningful Names
Make Meaningful Distinctions
If names must be different, then they should also mean something different. Avoid number-series naming and noise words that add no information.
Incorrect (indistinguishable names):
// Number series - meaningless distinction
public static void copyChars(char[] a1, char[] a2) {
for (int i = 0; i < a1.length; i++) {
a2[i] = a1[i];
}
}
// Noise words - what's the difference?
class Product {}
class ProductInfo {}
class ProductData {}
// Redundant prefixes
String nameString;
Customer customerObject;Correct (meaningful distinctions):
// Descriptive parameter names
public static void copyChars(char[] source, char[] destination) {
for (int i = 0; i < source.length; i++) {
destination[i] = source[i];
}
}
// If distinctions exist, name them
class Product {}
class ProductDetails {} // Shows additional information
class ProductInventory {} // Stock and availability
// No redundant type encoding
String name;
Customer customer;If you cannot distinguish what ProductInfo offers that Product does not, the names are noise.
Reference: Clean Code, Chapter 2: Meaningful Names
Use Verb Phrases for Method Names
Method names should be verbs or verb phrases. Methods do things, so their names should say what they do. Follow JavaBean conventions for accessors and mutators.
Incorrect (non-verb or unclear):
// Noun names for methods
public String name() {}
public int size() {} // Acceptable for collections
// Unclear action
public void customer() {} // Does what to customer?
// Missing verb
public boolean valid() {}Correct (verb phrases):
// Clear verb phrases
public void postPayment() {}
public void deletePage() {}
public void saveCustomer() {}
// Accessors, mutators, predicates follow conventions
public String getName() {}
public void setName(String name) {}
public boolean isPosted() {}
public boolean hasPermission() {}
// Static factory methods describe what is created
Complex fulcrumPoint = Complex.fromRealNumber(23.0);Use get, set, is, and has prefixes consistently for accessors, mutators, and predicates.
Language-specific conventions:
- Java/C#:
getName(),setName(),isActive()(JavaBean conventions) - Python: Use
@propertydecorators —user.namenotuser.get_name() - Kotlin/Swift: Direct property access with
val/var—user.namenotuser.getName() - TypeScript: Direct properties or getters —
get name(): string - Go:
Name()notGetName()(Go convention omitsGetprefix)
The underlying principle (methods describe actions, names describe things) is universal. The accessor/mutator conventions are language-specific.
Reference: Clean Code, Chapter 2: Meaningful Names
Use Pronounceable Names
Use pronounceable names. If you cannot pronounce a name, you cannot discuss it without sounding foolish. Programming is a social activity; names should facilitate discussion.
Incorrect (unpronounceable abbreviations):
class DtaRcrd102 {
private Date genymdhms; // generation year, month, day, hour, minute, second
private Date modymdhms;
private final String pszqint = "102";
}
// In conversation: "Hey, look at this dee-tee-ay-arr-cee-arr-dee"Correct (pronounceable names):
class Customer {
private Date generationTimestamp;
private Date modificationTimestamp;
private final String recordId = "102";
}
// In conversation: "Hey, look at this Customer record"The pronounceable version enables natural conversation: "Hey, when did we modify this customer's generation timestamp?"
Reference: Clean Code, Chapter 2: Meaningful Names
Use Searchable Names
Single-letter names and numeric constants are hard to locate across a body of text. Longer names trump shorter names, and searchable names trump convenient abbreviations.
Incorrect (unsearchable):
for (int j = 0; j < 34; j++) {
s += (t[j] * 4) / 5; // What is 34? What is 4? What is 5?
}Correct (searchable and meaningful):
int realDaysPerIdealDay = 4;
final int WORK_DAYS_PER_WEEK = 5;
final int NUMBER_OF_TASKS = 34;
int sum = 0;
for (int taskIndex = 0; taskIndex < NUMBER_OF_TASKS; taskIndex++) {
int realTaskDays = taskEstimate[taskIndex] * realDaysPerIdealDay;
int realTaskWeeks = realTaskDays / WORK_DAYS_PER_WEEK;
sum += realTaskWeeks;
}Now you can search for WORK_DAYS_PER_WEEK and find every place it is used. Try searching for 5 in a large codebase.
Exception: Single-letter names are acceptable only as local variables in short methods. The length of a name should correspond to the size of its scope.
Reference: Clean Code, Chapter 2: Meaningful Names
Avoid Hybrid Data-Object Structures
Hybrids are half object, half data structure. They have functions that do significant things, AND they have public variables or accessors that expose internal structure. Avoid creating these monstrosities.
Incorrect (hybrid structure):
// Half object, half data structure
public class Employee {
// Public data like a data structure
public String name;
public double salary;
public List<String> skills;
// Behavior like an object
public void promote() {
this.salary *= 1.1;
notifyHR();
}
public void addSkill(String skill) {
this.skills.add(skill);
updateTrainingRecord();
}
}
// Client code can bypass behavior
employee.salary = employee.salary * 1.5; // Skips notifyHR()
employee.skills.clear(); // Skips updateTrainingRecord()Correct (choose one approach):
// Pure object - hide data, expose behavior
public class Employee {
private String name;
private Money salary;
private SkillSet skills;
public void promote(PromotionDetails details) {
this.salary = salary.increase(details.getRaisePercentage());
notifyHR(details);
}
public void addSkill(Skill skill) {
this.skills.add(skill);
updateTrainingRecord(skill);
}
public EmployeeReport getReport() {
return new EmployeeReport(name, salary.getAmount(), skills.list());
}
}
// OR pure data structure - no behavior, expose data
public record EmployeeData(String name, double salary, List<String> skills) {}Hybrids make it hard to add new functions AND hard to add new data structures.
Reference: Clean Code, Chapter 6: Objects and Data Structures
Hide Data Behind Abstractions
Objects hide their data behind abstractions and expose functions that operate on that data. Data structures expose their data and have no meaningful functions. Do not mix these concepts.
Incorrect (exposing implementation):
// Exposes internal representation - clients depend on Cartesian coordinates
public class Point {
public double x;
public double y;
}
// Using it forces clients to know the implementation
double distance = Math.sqrt(point.x * point.x + point.y * point.y);Correct (hiding behind abstraction — immutable):
// Hides representation - could be Cartesian, polar, or something else
public interface Point {
double getX();
double getY();
double getR();
double getTheta();
static Point fromCartesian(double x, double y) {
return new CartesianPoint(x, y);
}
static Point fromPolar(double r, double theta) {
return new PolarPoint(r, theta);
}
}
// Clients work with the abstraction
double distance = point.getR(); // Works regardless of internal representation
Point moved = Point.fromCartesian(point.getX() + dx, point.getY() + dy);Key insight: The abstraction is not just about using getters/setters. It is about hiding the form of the data and exposing operations that work with the abstract concept. Prefer immutable objects with factory methods over mutable objects with setters — immutable designs are easier to reason about, thread-safe by default, and less prone to bugs.
Reference: Clean Code, Chapter 6: Objects and Data Structures
Understand Data/Object Anti-Symmetry
Objects and data structures are opposites. Objects hide data and expose behavior; data structures expose data and have no behavior. Choose based on what kind of changes you anticipate.
Incorrect (wrong approach for frequent type additions):
// Procedural approach - adding Triangle requires modifying EVERY function
public class Square { public double side; }
public class Circle { public double radius; }
public class Geometry {
public double area(Object shape) {
if (shape instanceof Square) {
Square s = (Square) shape;
return s.side * s.side;
} else if (shape instanceof Circle) {
Circle c = (Circle) shape;
return Math.PI * c.radius * c.radius;
}
// Must add Triangle case here AND in perimeter() AND in draw()...
throw new NoSuchShapeException();
}
}Correct (polymorphic approach for frequent type additions):
// Object approach - adding Triangle only requires one new class
public interface Shape {
double area();
}
public class Square implements Shape {
private double side;
public double area() {
return side * side;
}
}
public class Circle implements Shape {
private double radius;
public double area() {
return Math.PI * radius * radius;
}
}
// Adding Triangle is easy - just add one class
public class Triangle implements Shape {
private double base, height;
public double area() {
return 0.5 * base * height;
}
}Choose based on anticipated changes:
- Adding new functions often? Use data structures (procedural)
- Adding new types often? Use objects (polymorphic)
Reference: Clean Code, Chapter 6: Objects and Data Structures
Use DTOs for Data Transfer
Data Transfer Objects (DTOs) are pure data structures for transferring data between layers or systems. They should have no behavior beyond accessors. Use them at system boundaries.
Incorrect (domain object used for transfer):
// Domain object with behavior used for API response
public class User {
private String name;
private String hashedPassword;
private CreditCard creditCard;
public void authenticate(String password) { /* ... */ }
public void chargeCard(Money amount) { /* ... */ }
// Serialized directly - exposes sensitive data and internal structure
}Correct (separate DTO for transfer):
// Domain object with behavior (internal)
public class User {
private String name;
private HashedPassword password;
private CreditCard creditCard;
public void authenticate(String password) { /* ... */ }
public void chargeCard(Money amount) { /* ... */ }
public UserDTO toDTO() {
return new UserDTO(name, creditCard.getMaskedNumber());
}
}
// DTO for API response (external)
public record UserDTO(String name, String maskedCardNumber) {}
// Active Record variant - DTO with persistence methods only
public class UserRecord {
public String name;
public String email;
public void save() { /* SQL insert/update */ }
public static UserRecord findById(Long id) { /* SQL select */ }
}Use DTOs when:
- Transferring data across system boundaries (API, database, message queue)
- Hiding internal structure from external consumers
- Transforming data format between layers
Reference: Clean Code, Chapter 6: Objects and Data Structures
Follow the Law of Demeter
A method should only call methods on: its own object, objects passed as parameters, objects it creates, or its direct component objects. Avoid chained method calls on returned objects.
Incorrect (train wreck - violates Law of Demeter):
// Reaches through multiple objects
String outputDir = ctxt.getOptions().getScratchDir().getAbsolutePath();
// Forces knowledge of internal structure
customer.getAddress().getCity().getName();
// Chains expose implementation details
report.getFormatter().getConfiguration().getPageSize();Correct (respects object boundaries):
// Option 1: Create a direct method
String outputDir = ctxt.getOutputDirectory();
// Option 2: Tell, don't ask
customer.sendBillingNotice(); // Let customer handle its own address
// Option 3: If data structure, use it directly (no behavior expected)
BufferedOutputStream bos = ctxt.createScratchFileStream(classFileName);Exception: The Law of Demeter does not apply to data structures. If Options, ScratchDir, etc. are just data containers without behavior, the chaining is acceptable:
// Data transfer objects can be chained
final String outputDir = ctxt.options.scratchDir.absolutePath;Reference: Clean Code, Chapter 6: Objects and Data Structures
Use Build-Operate-Check Pattern
Structure tests in three distinct phases: Build (arrange), Operate (act), Check (assert). This pattern makes tests easy to read and understand at a glance.
Incorrect (mixed phases, unclear structure):
@Test
public void testOrder() {
Order order = new Order();
assertTrue(order.isEmpty());
Product product = new Product("Widget", 9.99);
order.add(product);
assertEquals(1, order.getItemCount());
assertEquals(9.99, order.getTotal(), 0.01);
order.add(product);
assertEquals(2, order.getItemCount());
assertEquals(19.98, order.getTotal(), 0.01);
}Correct (clear Build-Operate-Check):
@Test
public void addingProductShouldIncreaseItemCount() {
// Build
Order order = new Order();
Product widget = new Product("Widget", 9.99);
// Operate
order.add(widget);
// Check
assertEquals(1, order.getItemCount());
}
@Test
public void addingProductShouldUpdateTotal() {
// Build
Order order = new Order();
Product widget = new Product("Widget", 9.99);
// Operate
order.add(widget);
// Check
assertEquals(9.99, order.getTotal(), 0.01);
}
@Test
public void addingSameProductTwiceShouldDoubleTotal() {
// Build
Order order = new Order();
Product widget = new Product("Widget", 9.99);
// Operate
order.add(widget);
order.add(widget);
// Check
assertEquals(19.98, order.getTotal(), 0.01);
}Alias patterns:
- Build-Operate-Check (BOC)
- Arrange-Act-Assert (AAA)
- Given-When-Then (BDD)
Reference: Clean Code, Chapter 9: Unit Tests
Follow the Three Laws of TDD
Test-Driven Development follows three rules: (1) Write a failing test before production code, (2) Write only enough test to fail, (3) Write only enough production code to pass. This cycle repeats.
Incorrect (writing tests after code):
// Production code written first, then tests added later (or never)
public class Calculator {
public int add(int a, int b) {
return a + b;
}
public int divide(int a, int b) {
return a / b; // Edge cases not considered
}
}
// Test written after - may not cover edge cases
@Test
public void testAdd() {
assertEquals(4, calculator.add(2, 2));
}
// No test for divide by zero because code was written firstCorrect (TDD cycle):
// 1. Write failing test first
@Test
public void divideShouldReturnQuotient() {
assertEquals(2, calculator.divide(6, 3));
}
// 2. Write minimal code to pass
public int divide(int a, int b) {
return a / b;
}
// 3. Write next failing test
@Test(expected = IllegalArgumentException.class)
public void divideShouldThrowOnDivideByZero() {
calculator.divide(6, 0);
}
// 4. Update code to pass
public int divide(int a, int b) {
if (b == 0) {
throw new IllegalArgumentException("Cannot divide by zero");
}
return a / b;
}Benefits:
- Tests cover requirements, not implementation
- Edge cases discovered through test-first thinking
- High coverage achieved naturally
Reference: Clean Code, Chapter 9: Unit Tests
Follow FIRST Principles
Clean tests follow FIRST: Fast, Independent, Repeatable, Self-Validating, and Timely. These properties ensure tests remain useful and maintainable.
Incorrect (violating FIRST):
// Slow - depends on real database
@Test
public void testUserQuery() {
Connection conn = DriverManager.getConnection(PROD_DB_URL); // Slow!
// ...
}
// Not Independent - tests depend on each other
private static User sharedUser;
@Test
public void test1CreateUser() {
sharedUser = userService.create("bob");
}
@Test
public void test2UpdateUser() {
sharedUser.setName("alice"); // Depends on test1 running first
}
// Not Repeatable - depends on external state
@Test
public void testPayment() {
assertTrue(paymentGateway.charge(card, 100)); // Different result each time
}Correct (following FIRST):
// Fast - uses mocks, runs in milliseconds
@Test
public void queryShouldReturnMatchingUsers() {
UserRepository repo = mock(UserRepository.class);
when(repo.findByName("bob")).thenReturn(List.of(testUser));
List<User> result = userService.query("bob");
assertEquals(1, result.size());
}
// Independent - each test sets up its own state
@Test
public void createShouldReturnNewUser() {
User user = userService.create("bob");
assertEquals("bob", user.getName());
}
@Test
public void updateShouldModifyUserName() {
User user = userService.create("bob");
user.setName("alice");
assertEquals("alice", user.getName());
}
// Repeatable - uses test doubles
@Test
public void chargeShouldReturnTrueOnSuccess() {
PaymentGateway gateway = mock(PaymentGateway.class);
when(gateway.charge(any(), eq(100))).thenReturn(true);
assertTrue(paymentService.processPayment(100));
}Reference: Clean Code, Chapter 9: Unit Tests
Keep Tests Clean
Test code is just as important as production code. It requires thought, design, and care. If you let tests rot, they become a liability instead of an asset.
Incorrect (dirty tests):
@Test
public void test1() {
// Cryptic, no clear purpose
S s = new S();
s.a("x");
s.b("y");
assertTrue(s.c());
s.d();
assertEquals("xy", s.e());
}
@Test
public void testEverything() {
// Multiple assertions testing different concepts
assertNotNull(user);
assertEquals("Bob", user.getName());
assertTrue(user.isActive());
assertEquals(3, user.getOrders().size());
assertTrue(user.getOrders().get(0).isPaid());
}Correct (clean, readable tests):
@Test
public void concatenatingTwoStringsShouldReturnCombinedResult() {
StringBuffer buffer = new StringBuffer();
buffer.append("first");
buffer.append("second");
assertEquals("firstsecond", buffer.toString());
}
@Test
public void userShouldBeActiveAfterActivation() {
User user = new User("Bob");
user.activate();
assertTrue(user.isActive());
}
@Test
public void activatingUserShouldSendWelcomeEmail() {
User user = new User("Bob");
EmailService emailService = mock(EmailService.class);
user.activate(emailService);
verify(emailService).sendWelcome(user.getEmail());
}Qualities of clean tests:
- Readable: Clear setup, action, assertion
- One concept per test
- Descriptive names explaining what is being tested
Reference: Clean Code, Chapter 9: Unit Tests
One Concept Per Test
Each test should verify a single concept or behavior. Multiple assertions are fine when they all verify different aspects of that one concept. What you want to avoid is testing unrelated concerns in a single test.
Incorrect (multiple unrelated concepts in one test):
@Test
public void testUserRegistration() {
User user = userService.register("bob@example.com", "password123");
// Concept 1: user creation
assertNotNull(user);
assertNotNull(user.getId());
// Concept 2: email notification (unrelated to creation details)
verify(emailService).sendWelcome("bob@example.com");
// Concept 3: repository state (separate concern)
assertEquals(1, userRepository.count());
}
// If this fails, which concept is broken? Creation? Email? Persistence?Correct (one concept per test, multiple assertions are fine):
@Test
public void registerShouldCreateActiveUserWithGeneratedId() {
User user = userService.register("bob@example.com", "password123");
assertNotNull(user.getId());
assertEquals("bob@example.com", user.getEmail());
assertTrue(user.isActive());
}
@Test
public void registerShouldSendWelcomeEmail() {
userService.register("bob@example.com", "password123");
verify(emailService).sendWelcome("bob@example.com");
}
@Test
public void registerShouldPersistUser() {
userService.register("bob@example.com", "password123");
assertEquals(1, userRepository.count());
}The first test has three assertions — that's fine because they all verify a single concept: "the returned user object is correctly constructed." Each test can be understood from its name alone.
When NOT to split tests:
- When multiple assertions verify the same behavior from different angles (like checking all fields of a returned object), keep them together — splitting creates duplicate setup and slower test suites.
- When the operation under test has side effects (API calls, DB writes), avoid calling it multiple times just to isolate assertions.
Reference: Clean Code, Chapter 9: Unit Tests
Related skills
How it compares
Pick clean-code when you want principle-level refactors with explained exceptions; pick ESLint or SonarQube rules when you need automated CI enforcement only.
FAQ
What does the clean-code skill fix?
The clean-code skill applies Robert C. Martin Clean Code principles by flagging readability and maintainability issues, showing incorrect anti-patterns beside corrected examples, and noting explicit exception cases so agents do not over-apply each rule.
Does clean-code work for any programming language?
The clean-code skill is language-agnostic: each rule uses incorrect and correct code blocks that can target Java and other languages, with impact ratings and chapter references rather than a single-framework linter config.
Is Clean Code safe to install?
skills.sh reports 3 of 3 security scanners passed. Review the Security Audits panel on this page before installing in production.