Software Engineering
Core Principles
| Principle |
Meaning |
| Simplicity |
Simplest code that works; complexity only when required |
| Single Responsibility |
Each function/class/module does one thing |
| Self-Documenting |
Code explains itself; comments are a smell |
| Fail Fast |
Validate early, propagate unexpected errors |
| Test Behavior |
What code does, not implementation |
| No Backwards Compat |
Don't add legacy support unless requested |
| Consistency |
Match project conventions over preference |
Design
SOLID
| Principle |
Violation Sign |
| Single Responsibility |
Class doing too many things |
| Open/Closed |
Modifying existing code for new features |
| Liskov Substitution |
Overridden methods breaking contracts |
| Interface Segregation |
Clients depend on unused methods |
| Dependency Inversion |
High-level imports low-level details |
Architecture
Presentation → Application → Domain ← Infrastructure
↓
Dependencies point DOWN only
Rules:
- Domain depends on nothing
- Infrastructure implements domain interfaces
- No circular dependencies
Design Patterns
Use when solving real problems, not preemptively:
| Pattern |
Use When |
| Factory |
Complex object creation |
| Builder |
Many optional parameters |
| Adapter |
Incompatible interfaces |
| Facade |
Simplifying subsystem |
| Strategy |
Runtime algorithm selection |
| Observer |
Many dependents need notification |
Security
- Validate all external input (allowlists > denylists)
- Encrypt sensitive data at rest and transit
- Never log secrets
- Parameterized queries (SQL injection)
- Escape output (XSS)
Implementation
Naming
| Type |
Convention |
Example |
| Variables |
camelCase noun |
userName, isValid |
| Functions |
camelCase verb |
getUser, validateInput |
| Booleans |
is/has/can prefix |
isActive, hasPermission |
| Constants |
UPPER_SNAKE |
MAX_RETRIES |
| Classes |
PascalCase noun |
UserService |
Rules:
- Names reveal intent
- No single-letter params
- No abbreviations (
ur → userRepository)
Self-Documenting Code
// ❌ Comment hiding bad code
if (u.r === 1 && u.s !== 0) { ... }
// ✅ Self-documenting
if (user.isAdmin && user.isActive) { ... }
Acceptable comments: RFC links, bug tracker refs, non-obvious warnings
Functions
| Do |
Don't |
| Small, focused |
God functions (100+ lines) |
| 2-3 params max |
6+ parameters |
| Return early |
Deep nesting |
| Pure when possible |
Hidden side effects |
| Single abstraction level |
Mixed levels |
Error Handling
// ❌ Silent catch
try {
await save(user);
} catch (e) {}
// ❌ Log only
try {
await save(user);
} catch (e) {
console.log(e);
}
// ✅ Let propagate or handle specific
try {
await save(user);
} catch (e) {
if (e instanceof DuplicateEmail) return { error: "Email taken" };
throw e;
}
Rules:
- Empty catch = always wrong
- Catch only what you can handle
- Re-throw with context or propagate
- Crash visibly > fail silently
File Organization
- Match existing conventions
- No barrel files (
index.ts re-exports)
- Import from concrete modules
- Co-locate tests with source
src/users/
user-service.ts
user-service.test.ts
Code Smells
| Smell |
Fix |
| God Class |
Split by responsibility |
| Feature Envy |
Move method to data owner |
| Long Param List |
Parameter object |
| Primitive Obsession |
Value objects |
| Dead Code |
Delete it |
Linting
| Tool |
Purpose |
| Formatter |
Style (Prettier, dprint) |
| Linter |
Quality (ESLint, Ruff) |
| Type Checker |
Safety (tsc, mypy) |
Rules:
- Automate formatting
- Zero warnings in CI
- Never disable rules—fix the code
Testing
Test Pyramid
E2E (few) - Critical journeys, slow
Integration (some) - Component interactions
Unit (many) - Fast, isolated, business logic
What to Test
| Test |
Skip |
| Business logic |
Framework code |
| Edge cases |
Trivial getters |
| Error paths |
Third-party libs |
| Public API |
Private internals |
Test Quality
- Independent and isolated
- Deterministic (no flakiness)
- Fast (< 100ms unit)
- Single reason to fail
- Test behavior, not implementation
BDD Structure
describe("UserService", () => {
describe("given valid data", () => {
describe("when creating user", () => {
it("then persists with ID", async () => {
// arrange, act, assert
});
});
});
});
Anti-Patterns
| Pattern |
Fix |
| Ice Cream Cone |
More unit, fewer E2E |
| Flaky Tests |
Fix races, use mocks |
| Testing Implementation |
Test behavior |
| No Assertions |
Add meaningful checks |
Review
Before PR
- Type check passes
- Lint passes
- Tests pass
- No debug/console.log
- No commented code
- Up to date with main
Review Checklist
Correctness:
- Does it work? Edge cases? Error handling?
Design:
- Right abstraction? SOLID? Dependencies appropriate?
Readability:
- Clear names? Straightforward logic? No unnecessary comments?
Security:
- Input validated? No injection? Secrets handled?
Performance:
- No N+1? No await in loops? Caching considered?
Tests:
- Sufficient coverage? Edge cases? Behavior-focused?
For Reviewers
- Code, not author
- Questions > demands
- Explain the "why"
- Blocking vs nitpick
For Authors
- Small, focused PRs
- Context in description
- Respond to all comments
Maintenance
Refactoring
- Ensure test coverage first
- Small, incremental changes
- Run tests after each change
- Refactor OR add features, never both
| Technique |
When |
| Extract Method |
Long method, reusable logic |
| Extract Class |
Multiple responsibilities |
| Move Method |
Uses other class's data more |
| Introduce Param Object |
Long parameter lists |
Technical Debt
| Type |
Handling |
| Deliberate |
Document, schedule payback |
| Accidental |
Fix when discovered |
| Bit Rot |
Regular maintenance |
| Outdated Deps |
Regular updates |
Find: unused code, duplicates, circular deps, outdated deps
Performance
- Don't optimize prematurely
- Measure before optimizing
- Focus on hot paths
| Pitfall |
Fix |
| N+1 queries |
Batch, joins |
| Blocking I/O |
Async |
| Memory leaks |
Weak refs, cleanup |
Documentation
| Document |
Skip |
| Public APIs |
Obvious code |
| ADRs |
Implementation details |
| Setup/deploy |
Self-documenting code |
| Non-obvious behavior |
Every function |
Anti-Patterns
| Pattern |
Problem |
Fix |
| Big Ball of Mud |
No structure |
Define boundaries |
| Spaghetti Code |
Tangled |
Modularize |
| Lava Flow |
Dead code |
Delete it |
| Copy-Paste |
Duplication |
Extract |
| Magic Numbers |
No context |
Named constants |
| Circular Deps |
Coupling |
Abstraction layer |
| Feature Flags |
Hidden complexity |
One code path |
| Backwards Compat |
Legacy burden |
Replace entirely |
AI-Generated Debt
Code written by LLMs (including by this agent) accumulates specific debt patterns. Detect and fix on sight.
| Pattern |
Description |
| Restating comments |
Comment says what the next line does in different words |
| Boilerplate sprawl |
Verbose wrappers, re-exports, adapters that add no value |
| Premature abstraction |
Generic solution for a single use case |
| Convention drift |
Each file follows a slightly different style |
| Cargo-cult error handling |
try/catch everywhere with no actual recovery strategy |
| Orphan interfaces |
Interface defined but only one implementor, no extension |
| Placeholder logic |
TODO/stub/mock left behind, never replaced |
| Over-typing |
Complex generics when a concrete type is clearer |
| Hallucinated API usage |
Calling methods/options that don't exist in the actual lib |
| Copy-paste signatures |
Same function shape repeated across files with minor changes |
Code Health Dimensions
When reviewing or writing code, assess across two axes.
Mechanical (verifiable by tools)
| Dimension |
What to check |
| Dead code |
Unused imports, exports, variables, functions |
| Duplication |
Near-identical blocks across files |
| Complexity |
Deep nesting, long functions, high cyclomatic count |
| Dependency |
Import cycles, orphaned files, coupling violations |
| Test health |
Critical paths without tests, flaky tests |
| Security |
Unsanitized input, hardcoded secrets, SQL injection |
Subjective (requires judgment)
| Dimension |
What to check |
| Naming quality |
Do names communicate intent without reading the implementation? |
| Logic clarity |
Can you follow control flow without mental stack overflow? |
| Error consistency |
Same error strategy across modules, or a mix of patterns? |
| Abstraction fitness |
Does each abstraction earn its cost with real leverage? |
| Contract coherence |
Do functions honor their stated interfaces? |
| Cross-module arch |
Dependency direction correct? Hub modules justified? |
| Convention consistency |
Same patterns in sibling files? Or style islands? |
Prioritizing Fixes
| Tier |
Effort |
Examples |
| T1 |
Auto-fixable |
Unused imports, debug logs, formatting |
| T2 |
Quick manual |
Unused vars, dead exports, simple rename |
| T3 |
Needs judgment |
Near-duplicates, questionable abstraction |
| T4 |
Major refactor |
God class split, mixed-concern untangle |
Fix T1/T2 immediately. T3/T4 require a plan.
Acknowledging vs Hiding Debt
When leaving an issue unfixed:
- Document the decision - why it stays, not just that it does
- Track the cost - wontfix debt is still debt, it just has a reason
- Revisit periodically - reasons expire; what was acceptable 6 months ago may not be now
- Never dismiss to improve metrics - if the code is worse, admit it