engineering-fundamentals
Auto-invoke for general code quality review. Enforces naming conventions, function size, DRY principles, SOLID principles, and code organization.
When & Why to Use This Skill
The Engineering Fundamentals skill acts as an automated code quality guardian, systematically enforcing industry best practices such as SOLID principles, DRY patterns, and clean naming conventions. It helps development teams maintain high-quality, readable, and scalable codebases by identifying common anti-patterns like magic numbers, deep nesting, and oversized functions during the review process.
Use Cases
- Automated Pull Request Reviews: Instantly audit code changes for naming consistency, function size, and adherence to organizational standards before human review.
- Refactoring Legacy Code: Identify and decompose 'God Functions' and complex logic into modular, single-responsibility components to improve maintainability.
- Technical Debt Prevention: Proactively detect and replace magic numbers, deep nesting, and duplicated logic to ensure long-term code health.
- Developer Mentorship and Training: Provide junior engineers with Socratic feedback and actionable suggestions based on established engineering standards and SOLID principles.
- Standardizing Code Style: Enforce unified naming conventions and documentation requirements across diverse teams and projects to ensure a consistent developer experience.
| 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?" |