Auto-invoke for general code quality review. Enforces naming conventions, function size, DRY principles, SOLID principles, and code organization.
"Code is read more than it is written. Write for the reader, not the machine."
Activate this skill when reviewing:
user not usr)data, temp, info, stuff?is, has, can, should?❌ if (status === 2) { ... }
setTimeout(callback, 86400000);
✅ const STATUS = { ACTIVE: 2, INACTIVE: 1 };
if (status === STATUS.ACTIVE) { ... }
const ONE_DAY_MS = 24 * 60 * 60 * 1000;
setTimeout(callback, ONE_DAY_MS);
❌ const d = new Date();
const temp = getUser();
const flag = true;
✅ const createdAt = new Date();
const currentUser = getUser();
const isAuthenticated = true;
❌ function processOrder(order) {
// 200 lines: validate, calculate, save, email, log...
}
✅ function processOrder(order) {
validateOrder(order);
const total = calculateTotal(order);
await saveOrder(order, total);
await sendConfirmationEmail(order);
logOrderProcessed(order);
}
❌ function check(user) {
if (user) {
if (user.active) {
if (user.role === 'admin') {
return true;
}
}
}
return false;
}
✅ function check(user) {
if (!user) return false;
if (!user.active) return false;
if (user.role !== 'admin') return false;
return true;
}
❌ // Used once, but has 10 configuration options
createFlexibleReusableButton({ ... });
✅ // Just make the button
<button className="primary">Submit</button>
// Abstract when you need it 3+ times
| Principle | Question | Red Flag |
|---|---|---|
| Single Responsibility | "Does this class/function do one thing?" | Class with 10+ methods |
| Open/Closed | "Can I extend without modifying?" | Switch statements for types |
| Liskov Substitution | "Can I swap implementations?" | Overriding methods that break contracts |
| Interface Segregation | "Are interfaces focused?" | Clients forced to depend on unused methods |
| Dependency Inversion | "Do high-level modules depend on abstractions?" | Direct instantiation of dependencies |
Ask the junior these questions instead of giving answers:
| Type | Convention | Example |
|---|---|---|
| Variables | camelCase | userName, isActive |
| Constants | UPPER_SNAKE_CASE | MAX_RETRIES, API_URL |
| Functions | camelCase + verb | getUser(), handleSubmit() |
| Classes | PascalCase | UserService, AuthProvider |
| Files (components) | PascalCase | UserProfile.tsx |
| Files (utilities) | camelCase | formatDate.ts |
See detailed patterns in:
/standards/global/naming-conventions.md| Flag | Question to Ask |
|---|---|
| Single letter variables | "What does d represent?" |
| Functions > 30 lines | "Can we break this into smaller functions?" |
| > 3 levels of nesting | "Can we use early returns?" |
| Copy-pasted code | "If this logic changes, how many places need updating?" |
| Commented-out code | "Is this needed? Can we delete it?" |
| TODO without tracking | "Is there a ticket for this?" |
| Magic strings/numbers | "Should this be a named constant?" |