clean-code

name	clean-code
description	Clean Code principles (DRY, KISS, YAGNI), naming conventions, function design, and refactoring. Use when user says "clean this code", "refactor", "improve readability", or when reviewing code quality.

Clean Code Principles for Java

Core Principles

Principle	Meaning	Rule of Thumb
DRY	Don't Repeat Yourself	If you copy-paste, extract
KISS	Keep It Simple, Stupid	Simplest solution that works
YAGNI	You Ain't Gonna Need It	Don't build for imaginary futures
SRP	Single Responsibility	One reason to change
LoD	Law of Demeter	Don't talk to strangers

---

DRY - Don't Repeat Yourself

Violation - Duplicated Validation Logic

// BAD: Validation logic duplicated across services
public class UserService {
    public void registerUser(String email) {
        if (email == null || !email.contains("@") || !email.contains(".")) {
            throw new IllegalArgumentException("Invalid email");
        }
        // ... registration logic
    }
}

public class NotificationService {
    public void sendEmail(String email) {
        if (email == null || !email.contains("@") || !email.contains(".")) {
            throw new IllegalArgumentException("Invalid email");
        }
        // ... send logic
    }
}

Refactored - Extract Validator

// GOOD: Single source of truth for email validation
public class EmailValidator {
    private static final Pattern EMAIL_PATTERN =
        Pattern.compile("^[A-Za-z0-9+_.-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,}$");

    public static void validate(String email) {
        if (email == null || !EMAIL_PATTERN.matcher(email).matches()) {
            throw new InvalidEmailException(email);
        }
    }
}

public class UserService {
    public void registerUser(String email) {
        EmailValidator.validate(email);
        // ... registration logic
    }
}

public class NotificationService {
    public void sendEmail(String email) {
        EmailValidator.validate(email);
        // ... send logic
    }
}

DRY Exceptions - When Duplication Is Acceptable

Sometimes two pieces of code look similar but serve different purposes and evolve independently.

// These look similar but have DIFFERENT business reasons to change:

// Shipping cost calculation - changes with logistics rules
public class ShippingCalculator {
    public BigDecimal calculate(Order order) {
        BigDecimal base = new BigDecimal("5.00");
        if (order.getWeight() > 10) {
            base = base.add(new BigDecimal("3.00"));
        }
        if (order.isInternational()) {
            base = base.multiply(new BigDecimal("2.5"));
        }
        return base;
    }
}

// Insurance cost calculation - changes with insurance policies
public class InsuranceCalculator {
    public BigDecimal calculate(Order order) {
        BigDecimal base = new BigDecimal("5.00");
        if (order.getWeight() > 10) {
            base = base.add(new BigDecimal("3.00"));
        }
        if (order.isInternational()) {
            base = base.multiply(new BigDecimal("2.5"));
        }
        return base;
    }
}

// DO NOT merge these - they will diverge as business rules evolve.
// Premature DRY here creates coupling between unrelated domains.

---

KISS - Keep It Simple, Stupid

Violation - Optional Abuse

// BAD: Overcomplicated Optional chain
public String getUserDisplayName(Long userId) {
    return Optional.ofNullable(userId)
        .flatMap(id -> Optional.ofNullable(userRepository.findById(id)))
        .flatMap(opt -> opt)
        .map(user -> Optional.ofNullable(user.getDisplayName())
            .orElseGet(() -> Optional.ofNullable(user.getFirstName())
                .map(first -> first + " " + Optional.ofNullable(user.getLastName()).orElse(""))
                .orElse(user.getEmail())))
        .orElse("Unknown User");
}

Refactored - Simple Null Check

// GOOD: Simple, readable null checks
public String getUserDisplayName(Long userId) {
    if (userId == null) {
        return "Unknown User";
    }

    User user = userRepository.findById(userId).orElse(null);
    if (user == null) {
        return "Unknown User";
    }

    if (user.getDisplayName() != null) {
        return user.getDisplayName();
    }

    if (user.getFirstName() != null) {
        String lastName = user.getLastName() != null ? user.getLastName() : "";
        return user.getFirstName() + " " + lastName;
    }

    return user.getEmail();
}

---

YAGNI - You Ain't Gonna Need It

Violation - Over-Engineered Generic Repository

// BAD: Building a "generic" framework for imaginary future needs
public abstract class AbstractGenericRepository<T, ID extends Serializable>
    implements GenericRepository<T, ID> {

    private final Class<T> entityClass;
    private final EntityManager entityManager;
    private final CacheManager cacheManager;
    private final EventPublisher eventPublisher;
    private final AuditLogger auditLogger;

    @Override
    public T findById(ID id) {
        T cached = cacheManager.get(entityClass, id);
        if (cached != null) return cached;

        T entity = entityManager.find(entityClass, id);
        cacheManager.put(entityClass, id, entity);
        auditLogger.logAccess(entityClass, id);
        eventPublisher.publish(new EntityAccessedEvent<>(entity));
        return entity;
    }

    // 500+ lines of "reusable" code nobody asked for
}

Refactored - Minimal Repository

// GOOD: Just what you need right now
public interface UserRepository extends JpaRepository<User, Long> {
    Optional<User> findByEmail(String email);
    List<User> findByActiveTrue();
}

// Add caching WHEN you have a measured performance problem
// Add events WHEN you have a consumer that needs them
// Add auditing WHEN compliance requires it

---

Naming Conventions

Variables

// BAD                           // GOOD
int d;                           int elapsedDays;
String s;                        String customerName;
List<int[]> lst;                 List<int[]> flaggedCells;
Map<String, Object> m;           Map<String, Object> userAttributes;

Booleans

// BAD                           // GOOD
boolean flag;                    boolean isActive;
boolean status;                  boolean hasPermission;
boolean check;                   boolean canExecute;
boolean process;                 boolean shouldRetry;

Methods

// BAD                           // GOOD
void process();                  void processPayment();
Data get();                      UserProfile getUserProfile();
void do();                       void sendNotification();
boolean check(Order o);          boolean isEligibleForDiscount(Order order);
void handle(Event e);            void handlePaymentFailed(PaymentEvent event);

Classes

// BAD                           // GOOD
class Data {}                    class UserProfile {}
class Manager {}                 class OrderProcessor {}
class Helper {}                  class PriceCalculator {}
class Utils {}                   class DateFormatter {}
class Processor {}               class PaymentGatewayClient {}

---

Functions

Keep Small

// BAD: Method does too many things
public void processOrder(Order order) {
    // validate (15 lines)
    // calculate prices (20 lines)
    // apply discounts (15 lines)
    // update inventory (10 lines)
    // send notifications (10 lines)
    // ... 70+ lines total
}

// GOOD: Each method does one thing
public void processOrder(Order order) {
    validateOrder(order);
    BigDecimal total = calculateTotal(order);
    total = applyDiscounts(order, total);
    updateInventory(order);
    sendOrderConfirmation(order, total);
}

Single Abstraction Level

// BAD: Mixed levels of abstraction
public void registerUser(UserDto dto) {
    // High level
    validateUserData(dto);

    // Low level - suddenly dealing with SQL/details
    String hashedPassword = BCrypt.hashpw(dto.getPassword(), BCrypt.gensalt(12));
    Connection conn = dataSource.getConnection();
    PreparedStatement ps = conn.prepareStatement(
        "INSERT INTO users (email, password_hash) VALUES (?, ?)");
    ps.setString(1, dto.getEmail());
    ps.setString(2, hashedPassword);
    ps.executeUpdate();

    // High level again
    sendWelcomeEmail(dto.getEmail());
}

// GOOD: Consistent abstraction level
public void registerUser(UserDto dto) {
    validateUserData(dto);
    User user = createUser(dto);
    userRepository.save(user);
    sendWelcomeEmail(user.getEmail());
}

Limit Parameters

// BAD: Too many parameters
public User createUser(String firstName, String lastName, String email,
                       String phone, String address, String city,
                       String state, String zip, String country) { ... }

// GOOD: Use a parameter object
public User createUser(CreateUserRequest request) { ... }

public record CreateUserRequest(
    String firstName,
    String lastName,
    String email,
    String phone,
    Address address
) {}

Avoid Flag Arguments

// BAD: Boolean flag changes method behavior
public void sendNotification(User user, boolean isUrgent) {
    if (isUrgent) {
        // SMS logic
    } else {
        // Email logic
    }
}

// GOOD: Separate methods with clear intent
public void sendUrgentNotification(User user) { ... }
public void sendStandardNotification(User user) { ... }

---

Comments

Avoid Obvious Comments

// BAD: Comment repeats the code
// Get the user's age
int age = user.getAge();

// Increment counter by one
counter++;

// Check if user is active
if (user.isActive()) { ... }

Good Comments - Explain Why

// GOOD: Explains business rule or non-obvious reasoning

// Users created before 2020 have legacy pricing grandfathered in per contract #A-2019-045
if (user.getCreatedAt().isBefore(LEGACY_CUTOFF_DATE)) {
    return applyLegacyPricing(order);
}

// Using insertion sort here because data is nearly sorted (< 5% out of order)
// and insertion sort outperforms quicksort for nearly-sorted small datasets
insertionSort(nearlyOrderedItems);

// Retry with exponential backoff: payment gateway returns 429 during peak hours (2-4 PM EST)
Thread.sleep((long) Math.pow(2, retryCount) * 1000);

---

Common Code Smells

Smell	Symptom	Fix
Long Method	> 20 lines	Extract methods
Long Parameter List	> 3 parameters	Parameter Object
Feature Envy	Uses another class's data more than its own	Move method
Data Clumps	Same group of fields together	Extract class
Primitive Obsession	Strings for emails, money as double	Value Objects
Shotgun Surgery	One change touches many classes	Move related code together
Divergent Change	One class changed for many reasons	Split class (SRP)
God Class	Class does everything	Break into focused classes
Dead Code	Unreachable or unused code	Delete it
Speculative Generality	Abstract/generic code with one implementation	Simplify (YAGNI)

Magic Numbers

// BAD
if (user.getAge() > 18) { ... }
if (retryCount > 3) { ... }
if (price > 1000.00) { ... }

// GOOD
private static final int LEGAL_AGE = 18;
private static final int MAX_RETRY_ATTEMPTS = 3;
private static final BigDecimal FREE_SHIPPING_THRESHOLD = new BigDecimal("1000.00");

if (user.getAge() > LEGAL_AGE) { ... }
if (retryCount > MAX_RETRY_ATTEMPTS) { ... }
if (price.compareTo(FREE_SHIPPING_THRESHOLD) > 0) { ... }

Primitive Obsession - Use Value Objects

// BAD: Primitive types for domain concepts
public class User {
    private String email;        // Any string? No validation?
    private String phoneNumber;  // What format?
    private double balance;      // double for money?!
}

// GOOD: Value objects with validation (using records)
public record Email(String value) {
    public Email {
        if (value == null || !value.matches("^[A-Za-z0-9+_.-]+@.+\\..+$")) {
            throw new InvalidEmailException(value);
        }
    }
}

public record PhoneNumber(String value) {
    public PhoneNumber {
        String digits = value.replaceAll("[^0-9]", "");
        if (digits.length() < 10 || digits.length() > 15) {
            throw new InvalidPhoneNumberException(value);
        }
    }
}

public record Money(BigDecimal amount, Currency currency) {
    public Money {
        Objects.requireNonNull(amount, "Amount cannot be null");
        Objects.requireNonNull(currency, "Currency cannot be null");
        if (amount.scale() > currency.getDefaultFractionDigits()) {
            throw new IllegalArgumentException("Too many decimal places");
        }
    }

    public Money add(Money other) {
        if (!this.currency.equals(other.currency)) {
            throw new CurrencyMismatchException(this.currency, other.currency);
        }
        return new Money(this.amount.add(other.amount), this.currency);
    }
}

---

Refactoring Quick Reference

Technique	When	How
Extract Method	Long method, commented sections	Pull block into named method
Extract Class	Class with multiple responsibilities	Split into focused classes
Inline Method	Method body is as clear as its name	Replace call with body
Rename	Name doesn't reveal intent	Choose descriptive name
Replace Temp with Query	Temp variable used once	Extract to method
Introduce Parameter Object	Multiple related parameters	Group into class/record
Replace Conditional with Polymorphism	Complex if/switch on type	Use strategy pattern
Guard Clauses	Deeply nested conditionals	Early returns

Guard Clauses

// BAD: Deep nesting
public double calculateDiscount(Order order) {
    double discount = 0;
    if (order != null) {
        if (order.getCustomer() != null) {
            if (order.getCustomer().isPremium()) {
                if (order.getTotal() > 100) {
                    discount = order.getTotal() * 0.1;
                }
            }
        }
    }
    return discount;
}

// GOOD: Guard clauses with early returns
public double calculateDiscount(Order order) {
    if (order == null) return 0;
    if (order.getCustomer() == null) return 0;
    if (!order.getCustomer().isPremium()) return 0;
    if (order.getTotal() <= 100) return 0;

    return order.getTotal() * 0.1;
}

---

Clean Code Checklist

• Naming: All names reveal intent?
• Functions: Each does one thing? Under 20 lines?
• DRY: No copy-pasted logic?
• KISS: Simplest approach that works?
• YAGNI: Only building what's needed now?
• Comments: Explain why, not what?
• Magic Numbers: All replaced with named constants?
• Error Handling: Proper exceptions, no swallowed errors?
• Formatting: Consistent style?
• Tests: Code is testable? Dependencies injectable?

---

Related Skills

• java-code-review - Systematic code review process
• design-patterns - When refactoring reveals need for patterns
• api-contract-review - Clean API design