---
name: Senior Engineer Coding
description: >
  Apply senior software engineering standards when writing, editing, or refactoring
  any code. Use this skill whenever the user asks to write a function, build a feature,
  fix a bug, design a module, or improve existing code — even if they don't mention
  quality explicitly. The goal is production-grade code: readable, correct, maintainable,
  secure, and failure-aware. Always active for any coding task, at any scale.
triggers:
  - "senior engineer"
  - "production code"
  - "production-grade"
  - "write quality code"
  - "robust code"
  - "maintainable code"
  - "secure code"
  - "production quality"
negatives:
license: MIT
compatibility: opencode
  - "design"
  - "architecture"
  - "testing"
  - "performance optimization"
  - "animation"
metadata:
  version: "1.0.0"

  workflow: general
  audience: developers---


# Senior Engineer Coding

## Mindset

Write code for the next engineer who has to change it at 2am with no context.
Clean code is risk management. Every production incident traces back to unclear,
poorly structured, or hard-to-change code — not missing cleverness.

The standard: a teammate can safely change the code without fear, and explain
what it does after a quick read.

---

## 1. Naming — Highest ROI Habit

Good names eliminate comments, prevent misunderstandings, and make refactoring safe.

**Rules:**
- Name by **intent**, not by type or storage: `emailAddress` not `emailString`
- Use **domain language**: reflect business concepts (`Invoice`, `Subscription`, `Refund`)
- Include **units** in names: `timeoutMs`, `priceCents`, `fileSizeBytes`
- Make **booleans** read as questions: `isEnabled`, `hasAccess`, `shouldRetry`
- Name from the **caller's perspective**: `order.chargePayment()` not `order.process()`
- Be **consistent**: one concept, one word. Never `user` in one place and `customer` in another for the same entity

**Avoid:**
- Noise words: `manager`, `helper`, `data`, `info`, `handler` (unless genuinely descriptive)
- Abbreviations that don't scale: `usrAcctMgr`, `calcDisc`
- Overly generic verbs: `process()`, `handle()`, `doWork()`, `run()`
- Single-letter variables outside tiny loops: `d`, `x`, `tmp` cause real production bugs

---

## 2. Functions — Small, Focused, Predictable

**One function = one job.** If you can't describe it in one sentence, it does too much.

**Guard clauses over nested ifs:**
```
// Wrong — pyramid of doom, hides assumptions
if (user != null) {
  if (user.isActive()) {
    if (hasAccess(user)) {
      process(user)
    }
  }
}

// Right — early returns, happy path stays flat
if (user == null) return
if (!user.isActive()) return
if (!hasAccess(user)) return
process(user)
```

**Limit parameters:**
- More than 3 parameters → create a request/options object
- Long param lists break silently when order is swapped; objects don't

**Separate computation from I/O:**
- Pure logic (calculation, transformation) → separate from functions that read files, call APIs, write DBs
- Pure functions are easier to test, easier to reason about, and have no hidden side effects

**Command/Query separation:**
- Functions that return data → must not mutate state
- Functions that mutate → return a status/result, not data

---

## 3. No Magic Numbers or Strings

Every hardcoded value with business meaning is a future bug.

```
// Wrong
price * 1.21
if (status === 3)
setTimeout(fn, 86400000)

// Right
const VAT_RATE = 0.21
const STATUS_SHIPPED = 3
const ONE_DAY_MS = 24 * 60 * 60 * 1000
```

If a value can change, or has meaning beyond its face value, it must be a named constant.

---

## 4. Error Handling — Fail Fast, Fail Clearly

**Validate inputs at the boundary:**
- Check early, fail fast, with an actionable message
- Never let bad data travel deep into the system before exploding

**Make failures explicit:**
```
// Wrong — vague, no context
throw new Error("Something went wrong")
catch (e) { console.log(e) }

// Right — who failed, what failed, what identifier
throw new Error(`Payment failed for orderId=${order.id}: ${e.message}`)
logger.error({ orderId: order.id, userId: user.id, error: e.message })
```

**Design for failure, not success:**
- APIs fail. Network is unreliable. Data can be malformed. Assume all of it.
- Every external call needs: timeout, error handling, and at minimum a logged failure

**Don't swallow exceptions:**
- Empty `catch {}` blocks are production bugs waiting to happen
- If you catch, you must log (with context) or rethrow

---

## 5. DRY — Don't Repeat Yourself

Duplication is a defect multiplier. One version rounds. Another doesn't. Both are "correct".

- Extract shared logic the moment you write it a second time
- Centralize business rules: discount logic, tax calculation, validation rules
- One source of truth per concept

**But don't over-DRY:**
- Two functions that look similar but represent different concepts → keep them separate
- Premature abstraction is worse than mild duplication

---

## 6. KISS — Simplest Solution That Works

```
// Wrong — "clever" but unmaintainable
Optional<Optional<List<Data>>> result = ...

// Right — obvious and safe
return include ? List.of(data) : List.of()
```

Rules:
- Choose the simplest approach that solves the problem
- If you're proud of how clever it is, it's probably too clever
- Future you with no sleep at 2am must understand it immediately

---

## 7. Structure — Separation of Concerns

**Boundary test:** if changing one feature requires edits in many unrelated places, the boundaries are wrong.

Keep these separated:
- **UI** vs **business logic** — never mix rendering with rules
- **Domain** vs **infrastructure** — business logic must not depend on DB/HTTP/filesystem directly
- **Feature modules** — group by feature, not by layer when it reduces cross-module coupling

**Composition over inheritance:**
```
// Wrong — tight coupling through inheritance
class EmailNotifier extends Order {}

// Right — explicit dependency
class OrderService {
  constructor(private notifier: Notifier) {}
}
```
Inheritance spreads behavior implicitly. Composition makes dependencies explicit and swappable.

---

## 8. Comments — Why, Never What

The code explains **what**. Comments explain **why**.

**Good comments:**
```
// Retry once to handle transient network blips — removing this caused incident #4821
retryOnce(request)

// Using soft deletes — this query MUST include deleted_at IS NULL
// or deactivated accounts appear in results
```

**Bad comments (delete these):**
```
// Loop through users
for (const user of users) { ... }

// Set name
this.name = name
```

**Best outcome:** refactor the code until the comment is unnecessary.

If a `TODO` exists, it must have an owner and a date. Ownerless TODOs become permanent.

---

## 9. Security — Non-Negotiable Defaults

Apply these on every piece of code, without being asked:

- **Never trust external input** — validate and sanitize everything from users, APIs, files
- **Parameterize queries** — never concatenate user input into SQL
- **No secrets in code** — no API keys, passwords, or tokens hardcoded anywhere
- **Auth checks at every endpoint** — don't assume a caller is authenticated
- **Principle of least privilege** — request only the permissions the code actually needs
- **Error messages must not leak internals** — stack traces, DB queries, and file paths stay in logs, never in user-facing responses

---

## 10. SOLID — Applied With Concrete Patterns

Apply when it reduces coupling or improves testability. Don't add layers that don't solve a real problem. Each principle below includes the specific code smell it prevents.

### S — Single Responsibility
**One reason to change per module.** If it changes for two different reasons, split it.

**Smell:** A class/service mixes persistence, business logic, and notification:
```
// Bad — 3 reasons to change
class OrderService {
  save(order) { /* write to DB */ }
  calculateTax(order) { /* math */ }
  sendConfirmation(order) { /* email */ }
}
```
**Fix:**
```
class OrderRepository { save(order) }
class TaxCalculator { calculate(order) }
class OrderNotifier { sendConfirmation(order) }
class OrderService {
  constructor(private repo: OrderRepository, private tax: TaxCalculator, private notifier: OrderNotifier) {}
  async process(order) { await this.repo.save(order); await this.notifier.sendConfirmation(order); }
}
```

### O — Open/Closed
**Extend behavior by adding code, not rewriting existing code.**

**Smell:** A function with a growing switch/if-else for each new type:
```
// Bad — adding a new payment method means editing this function
function processPayment(type, amount) {
  if (type === 'credit') { /* ... */ }
  else if (type === 'paypal') { /* ... */ }
  // add else-if for every new type
}
```
**Fix:**
```
interface PaymentProcessor { process(amount: number): void }
class CreditProcessor implements PaymentProcessor { process(amount) { /* ... */ } }
class PaypalProcessor implements PaymentProcessor { process(amount) { /* ... */ } }
// Add new processors without touching existing code
```

### L — Liskov Substitution
**Subtypes must be substitutable for their base types.** A caller using the parent type must work correctly with any child.

**Smell:** A subclass throws exceptions the parent doesn't, or returns different types:
```
class FileStorage { save(file: Buffer): void }
class S3Storage extends FileStorage {
  save(file: Buffer): void { throw new Error("S3 timeout") } // Breaks callers expecting silent success
}
```
**Fix:** Design by contract. Document preconditions/postconditions. Don't narrow the output or widen the input.

### I — Interface Segregation
**Small, focused interfaces over god-objects with 20 methods.**

**Smell:** A class is forced to implement methods it doesn't need:
```
interface Worker { eat(): void; work(): void; sleep(): void }
class Robot implements Worker {
  eat() { throw "I don't eat" }  // Forced to implement
  work() { /* ... */ }
  sleep() { throw "I don't sleep" }
}
```
**Fix:**
```
interface Workable { work(): void }
interface Eatable { eat(): void }
interface Sleepable { sleep(): void }
class Robot implements Workable { work() { /* ... */ } }
class Human implements Workable, Eatable, Sleepable { work() { /* ... */ }; eat() { /* ... */ }; sleep() { /* ... */ } }
```

### D — Dependency Inversion
**Depend on abstractions, not concretions.** High-level modules should not depend on low-level modules.

**Smell:** Business logic directly instantiates infrastructure:
```
class ReportGenerator {
  private db = new MySQLDatabase()  // Tight coupling to MySQL
}
```
**Fix:**
```
interface Database { query(sql: string): Result }
class ReportGenerator {
  constructor(private db: Database) {}  // Swappable: MySQL, Postgres, mock
}
```

**Reality check:** if SOLID adds layers without reducing risk or improving clarity → don't apply it.

---

## 11. Code to Be Reviewed

Every change will be read by another engineer. Write with that in mind.

- **Keep diffs small and focused** — one PR per concern
- **Meaningful commit messages**: `Add null check to prevent crash when user profile is missing` not `fix bug`
- **No reformatting mixed with logic changes** — they belong in separate commits
- **Self-review before submitting** — read your own diff as if you're reviewing someone else's work

---

## Gotchas — Mistakes That Cause Real Production Bugs

These are concrete errors to avoid, learned from real incidents:

- **Updating a value in 6 places instead of 1**: always ask "where is this defined?" before editing a value
- **Assuming a variable name's meaning**: `d` meant `days` in one context and `discount` in another — causing a retry miscalculation
- **Removing "useless" code without understanding why it exists**: a retry that looked dead was handling transient network failures — removing it caused incidents
- **Mutating state inside a query function**: violates Command/Query separation, causes unpredictable behavior
- **Writing the happy path only**: always ask "what happens if this returns null/undefined/error?"
- **Long functions that batch unrelated operations**: when email sending fails, the whole function retries, duplicating data — keep operations separate
- **Missing `deleted_at IS NULL` on soft-delete tables**: a query that returns deactivated users looks correct until a customer calls
- **Mocking too much in tests**: tests that mock internals break on every refactor while missing real bugs — test behavior, not implementation

---

## Quick Self-Check Before Finishing

Before marking any code task done, run through:

- [ ] Names reveal intent without needing comments?
- [ ] Every function does one thing?
- [ ] No magic numbers or strings?
- [ ] Edge cases handled (null, empty, error)?
- [ ] No secrets or sensitive data hardcoded?
- [ ] No duplicated logic?
- [ ] Lint and typecheck pass?
- [ ] The next engineer can change this safely?

---

## Workflow

1. **Understand intent** — read the task requirements. Identify the boundary of the change and what must not be touched.
2. **Survey surroundings** — grep/glob for callers, importers, and dependent code. Understand how the changed code fits into the larger system.
3. **Plan the change** — identify the minimal set of files and lines that must change. Prefer surgical edits over rewrites.
4. **Write clear, fail-safe code** — guard clauses over nested ifs. Named constants over magic values. Parameterized queries over string interpolation. Explicit error handling at every boundary.
5. **Self-review the diff** — read your own changes as if reviewing a teammate's PR. Check naming, edge cases, security, and style consistency.
6. **Verify correctness** — run lint, typecheck, and the narrowest test that exercises the change. Fix any failures before claiming completion.

## Error Handling

| Cause | Fix |
|-------|-----|
| Null/undefined cascading through functions | Validate at boundary. Return early with explicit null guards. Never propagate null silently. |
| Nested conditionals creating unreachable branches | Flatten with guard clauses. Return/fail early on invalid states. |
| Long functions mixing I/O with business logic | Separate pure computation from I/O. Extract I/O to dedicated functions. |
| Swallowed exceptions in empty catch blocks | Always log with context and rethrow, or handle explicitly. Never empty catch {}. |
| API/timeout failures with no recovery | Add retry with jitter, timeout, and circuit breaker for every external call. |
| Race conditions from shared mutable state | Use transactions for multi-step writes. Add locking or atomic operations for shared state. |
| Missing auth checks on endpoints | Verify auth middleware is applied to every endpoint. Check authorization for the specific resource, not just login status. |
| Magic numbers scattered across codebase | Extract to named constants with descriptive names and units. |

## Anti-Patterns

| Pattern | Problem | Fix |
|---------|---------|-----|
| "Manager"/"Helper"/"Handler" classes | Vague naming hides responsibility | Name by intent and domain concept: `OrderRepository`, `TaxCalculator` |
| Functions with 5+ parameters | Argument order bugs, hard to call | Create a request/options object. Limit to 3 max. |
| Comments explaining what the code does | Code should be self-documenting | Refactor until the comment is unnecessary. Keep only "why" comments. |
| Premature abstractions ("just in case") | Adds complexity for imaginary future needs | Abstract only when duplication exists. YAGNI. |
| Mixing concerns in one class/service | One class changes for 3 different reasons | Apply Single Responsibility: persistence, business logic, and notification belong in separate classes. |
| Inheritance chains over 2 levels deep | Implicit behavior, hard to trace | Prefer composition. Inject dependencies explicitly. |
| Processing user input without validation | Injection, corruption, crashes | Validate and sanitize at the system boundary before any processing. |
| Throwing generic Error with no context | Debugging requires grep, not error messages | Include what failed, what identifier, and actionable context in every error. |

## Checklist

- [ ] Function does one thing — if it needs a comment to explain sub-steps, extract them
- [ ] Error handling covers all failure modes (null, network, auth, rate limit)
- [ ] Names are precise and reveal intent (no abbreviations, no generic terms)
- [ ] Tests exist for the critical path (not 100% coverage, but core logic is covered)
- [ ] Security review done for any auth, input, or data exposure surface

## Sources

- Robert C. Martin — "Clean Code" (Prentice Hall)
- Martin Fowler — "Refactoring" (Addison-Wesley, 2nd Edition)
- John Ousterhout — "A Philosophy of Software Design" (Yaknyam Press)
- Michael Feathers — "Working Effectively with Legacy Code" (Prentice Hall)
- Steve McConnell — "Code Complete" (Microsoft Press, 2nd Edition)
- Hunt & Thomas — "The Pragmatic Programmer" (Addison-Wesley, 20th Anniversary)
- Kent Beck — "Implementation Patterns" (Addison-Wesley)
- OWASP Top 10 — owasp.org/www-project-top-ten
