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
code
❌ 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
code
❌ const d = new Date(); const temp = getUser(); const flag = true; ✅ const createdAt = new Date(); const currentUser = getUser(); const isAuthenticated = true;
3. God Functions
code
❌ 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
code
❌ 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
code
❌ // 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?" |