engineering-fundamentals

# 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? (`user` not `usr`) - [ ] **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 | |-----------|----------|----------| | **S**ingle Responsibility | "Does this class/function do one thing?" | Class with 10+ methods | | **O**pen/Closed | "Can I extend without modifying?" | Switch statements for types | | **L**iskov Substitution | "Can I swap implementations?" | Overriding methods that break contracts | | **I**nterface Segregation | "Are interfaces focused?" | Clients forced to depend on unused methods | | **D**ependency Inversion | "Do high-level modules depend on abstractions?" | Direct instantiation of dependencies | --- ## Socratic Questions Ask the junior these questions instead of giving answers: 1. **Naming**: "Would a new developer understand this name without context?" 2. **Function Size**: "Can you describe what this function does in one sentence?" 3. **Duplication**: "I see this pattern in three places. What happens if it needs to change?" 4. **Abstraction**: "How many times is this abstraction actually used?" 5. **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?" |