code-quality

Code Quality Review Skill

Systematic code review combining clean code principles, API design, and Java best practices.

When to Use

• "review this code" / "code review" / "check this PR"
• "refactor" / "clean this code" / "improve readability"
• "review API" / "check endpoints" / "REST review"
• Before merging PR or releasing API changes

Review Strategy

1. Quick scan - Understand intent, identify scope
2. Checklist pass - Apply relevant categories below
3. Summary - List findings by severity (Critical → Minor → Good)

---

Clean Code Principles

DRY - Don't Repeat Yourself

Violation:

// ❌ Duplicated validation logic
public void createUser(UserRequest req) {
    if (req.getEmail() == null || !req.getEmail().contains("@")) {
        throw new ValidationException("Invalid email");
    }
}

public void updateUser(UserRequest req) {
    if (req.getEmail() == null || !req.getEmail().contains("@")) {
        throw new ValidationException("Invalid email");
    }
}

Fix:

// ✅ Single source of truth
public class EmailValidator {
    public void validate(String email) {
        if (email == null || !email.contains("@")) {
            throw new ValidationException("Invalid email");
        }
    }
}

KISS - Keep It Simple

Violation:

// ❌ Over-engineered
public interface UserFactory {
    User createUser();
}
public class ConcreteUserFactory implements UserFactory {
    public User createUser() { return new User(); }
}

Fix:

// ✅ Simple
public User createUser() { return new User(); }

YAGNI - You Aren't Gonna Need It

Violation:

// ❌ Premature abstraction
public class ConfigurableUserServiceFactoryProvider { }

Fix:

// ✅ Implement when actually needed
public class UserService { }

---

API Contract Review

HTTP Verb Semantics

Verb	Use For	Idempotent	Safe
GET	Retrieve resource	Yes	Yes
POST	Create new resource	No	No
PUT	Replace entire resource	Yes	No
PATCH	Partial update	No*	No
DELETE	Remove resource	Yes	No