| name | Engineering Fundamentals |
| description | Auto-invoke for general code quality review. Enforces naming conventions, function size, DRY principles, SOLID principles, and code organization. |
Engineering Fundamentals Review
"Code is read more than it is written. Write for the reader, not the machine."
When to Apply
Activate this skill when reviewing:
- Any code changes
- Function and variable naming
- Code organization and structure
- General refactoring decisions
Review Checklist
Naming
- Descriptive: Can you understand the variable without context?
- No abbreviations: Are names spelled out? (
usernotusr) - No generic names: No
data,temp,info,stuff? - Boolean prefix: Do booleans start with
is,has,can,should? - Function verbs: Do functions start with action verbs?
Function Design
- Single responsibility: Does each function do ONE thing?
- Size limit: Are functions under 20-30 lines?
- Parameter count: Are there fewer than 4 parameters?
- No side effects: Are pure functions actually pure?
- Early returns: Are guard clauses used instead of deep nesting?
Code Organization
- DRY: Is duplicated code extracted into functions?
- But not too DRY: Are abstractions justified (rule of three)?
- Cohesion: Are related things grouped together?
- Separation: Are unrelated things separated?
Comments & Documentation
- Why, not what: Do comments explain reasoning, not obvious code?
- No commented-out code: Is dead code deleted, not commented?
- JSDoc on public APIs: Are exported functions documented?
Common Mistakes (Anti-Patterns)
1. Magic Numbers
❌ 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);
2. Unclear Naming
❌ const d = new Date();
const temp = getUser();
const flag = true;
✅ const createdAt = new Date();
const currentUser = getUser();
const isAuthenticated = true;
3. God Functions
❌ 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);
}
4. Deep Nesting
❌ 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;
}
5. Premature Abstraction
❌ // Used once, but has 10 configuration options
createFlexibleReusableButton({ ... });
✅ // Just make the button
<button className="primary">Submit</button>
// Abstract when you need it 3+ times
SOLID Principles Quick Check
| 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 |
Socratic Questions
Ask the junior these questions instead of giving answers:
- Naming: "Would a new developer understand this name without context?"
- Function Size: "Can you describe what this function does in one sentence?"
- Duplication: "I see this pattern in three places. What happens if it needs to change?"
- Abstraction: "How many times is this abstraction actually used?"
- Readability: "If you came back to this code in 6 months, would you understand it?"
Naming Conventions
| 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 |
Standards Reference
See detailed patterns in:
/standards/global/naming-conventions.md
Red Flags to Call Out
| 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?" |