|
| 1 | +# Code Quality Checklist |
| 2 | + |
| 3 | +## Error Handling |
| 4 | + |
| 5 | +### Anti-patterns to Flag |
| 6 | +- **Swallowed exceptions**: Empty catch blocks or catch with only logging |
| 7 | + ```javascript |
| 8 | + try { ... } catch (e) { } // Silent failure |
| 9 | + try { ... } catch (e) { console.log(e) } // Log and forget |
| 10 | + ``` |
| 11 | +- **Overly broad catch**: Catching `Exception`/`Error` base class instead of specific types |
| 12 | +- **Error information leakage**: Stack traces or internal details exposed to users |
| 13 | +- **Missing error handling**: No try-catch around fallible operations (I/O, network, parsing) |
| 14 | +- **Async error handling**: Unhandled promise rejections, missing `.catch()`, no error boundary |
| 15 | + |
| 16 | +### Best Practices to Check |
| 17 | +- [ ] Errors are caught at appropriate boundaries |
| 18 | +- [ ] Error messages are user-friendly (no internal details exposed) |
| 19 | +- [ ] Errors are logged with sufficient context for debugging |
| 20 | +- [ ] Async errors are properly propagated or handled |
| 21 | +- [ ] Fallback behavior is defined for recoverable errors |
| 22 | +- [ ] Critical errors trigger alerts/monitoring |
| 23 | + |
| 24 | +### Questions to Ask |
| 25 | +- "What happens when this operation fails?" |
| 26 | +- "Will the caller know something went wrong?" |
| 27 | +- "Is there enough context to debug this error?" |
| 28 | + |
| 29 | +--- |
| 30 | + |
| 31 | +## Performance & Caching |
| 32 | + |
| 33 | +### CPU-Intensive Operations |
| 34 | +- **Expensive operations in hot paths**: Regex compilation, JSON parsing, crypto in loops |
| 35 | +- **Blocking main thread**: Sync I/O, heavy computation without worker/async |
| 36 | +- **Unnecessary recomputation**: Same calculation done multiple times |
| 37 | +- **Missing memoization**: Pure functions called repeatedly with same inputs |
| 38 | + |
| 39 | +### Database & I/O |
| 40 | +- **N+1 queries**: Loop that makes a query per item instead of batch |
| 41 | + ```javascript |
| 42 | + // Bad: N+1 |
| 43 | + for (const id of ids) { |
| 44 | + const user = await db.query(`SELECT * FROM users WHERE id = ?`, id) |
| 45 | + } |
| 46 | + // Good: Batch |
| 47 | + const users = await db.query(`SELECT * FROM users WHERE id IN (?)`, ids) |
| 48 | + ``` |
| 49 | +- **Missing indexes**: Queries on unindexed columns |
| 50 | +- **Over-fetching**: SELECT * when only few columns needed |
| 51 | +- **No pagination**: Loading entire dataset into memory |
| 52 | + |
| 53 | +### Caching Issues |
| 54 | +- **Missing cache for expensive operations**: Repeated API calls, DB queries, computations |
| 55 | +- **Cache without TTL**: Stale data served indefinitely |
| 56 | +- **Cache without invalidation strategy**: Data updated but cache not cleared |
| 57 | +- **Cache key collisions**: Insufficient key uniqueness |
| 58 | +- **Caching user-specific data globally**: Security/privacy issue |
| 59 | + |
| 60 | +### Memory |
| 61 | +- **Unbounded collections**: Arrays/maps that grow without limit |
| 62 | +- **Large object retention**: Holding references preventing GC |
| 63 | +- **String concatenation in loops**: Use StringBuilder/join instead |
| 64 | +- **Loading large files entirely**: Use streaming instead |
| 65 | + |
| 66 | +### Questions to Ask |
| 67 | +- "What's the time complexity of this operation?" |
| 68 | +- "How does this behave with 10x/100x data?" |
| 69 | +- "Is this result cacheable? Should it be?" |
| 70 | +- "Can this be batched instead of one-by-one?" |
| 71 | + |
| 72 | +--- |
| 73 | + |
| 74 | +## Boundary Conditions |
| 75 | + |
| 76 | +### Null/Undefined Handling |
| 77 | +- **Missing null checks**: Accessing properties on potentially null objects |
| 78 | +- **Truthy/falsy confusion**: `if (value)` when `0` or `""` are valid |
| 79 | +- **Optional chaining overuse**: `a?.b?.c?.d` hiding structural issues |
| 80 | +- **Null vs undefined inconsistency**: Mixed usage without clear convention |
| 81 | + |
| 82 | +### Empty Collections |
| 83 | +- **Empty array not handled**: Code assumes array has items |
| 84 | +- **Empty object edge case**: `for...in` or `Object.keys` on empty object |
| 85 | +- **First/last element access**: `arr[0]` or `arr[arr.length-1]` without length check |
| 86 | + |
| 87 | +### Numeric Boundaries |
| 88 | +- **Division by zero**: Missing check before division |
| 89 | +- **Integer overflow**: Large numbers exceeding safe integer range |
| 90 | +- **Floating point comparison**: Using `===` instead of epsilon comparison |
| 91 | +- **Negative values**: Index or count that shouldn't be negative |
| 92 | +- **Off-by-one errors**: Loop bounds, array slicing, pagination |
| 93 | + |
| 94 | +### String Boundaries |
| 95 | +- **Empty string**: Not handled as edge case |
| 96 | +- **Whitespace-only string**: Passes truthy check but is effectively empty |
| 97 | +- **Very long strings**: No length limits causing memory/display issues |
| 98 | +- **Unicode edge cases**: Emoji, RTL text, combining characters |
| 99 | + |
| 100 | +### Common Patterns to Flag |
| 101 | +```javascript |
| 102 | +// Dangerous: no null check |
| 103 | +const name = user.profile.name |
| 104 | + |
| 105 | +// Dangerous: array access without check |
| 106 | +const first = items[0] |
| 107 | + |
| 108 | +// Dangerous: division without check |
| 109 | +const avg = total / count |
| 110 | + |
| 111 | +// Dangerous: truthy check excludes valid values |
| 112 | +if (value) { ... } // fails for 0, "", false |
| 113 | +``` |
| 114 | + |
| 115 | +### Questions to Ask |
| 116 | +- "What if this is null/undefined?" |
| 117 | +- "What if this collection is empty?" |
| 118 | +- "What's the valid range for this number?" |
| 119 | +- "What happens at the boundaries (0, -1, MAX_INT)?" |
0 commit comments