takt/builtins/en/policies/coding.md
2026-02-12 14:49:35 +09:00

291 lines
9.6 KiB
Markdown

# Coding Policy
Prioritize correctness over speed, and code accuracy over ease of implementation.
## Principles
| Principle | Criteria |
|-----------|----------|
| Simple > Easy | Prioritize readability over writability |
| DRY | Eliminate essential duplication |
| Comments | Why only. Never write What/How |
| Function size | One function, one responsibility. ~30 lines |
| File size | ~300 lines as a guideline. Be flexible depending on the task |
| Boy Scout | Leave touched areas a little better than you found them |
| Fail Fast | Detect errors early. Never swallow them |
## No Fallbacks or Default Arguments
Do not write code that obscures the flow of values. Code where you must trace logic to understand a value is bad code.
### Prohibited Patterns
| Pattern | Example | Problem |
|---------|---------|---------|
| Fallback for required data | `user?.id ?? 'unknown'` | Processing continues in a state that should error |
| Default argument abuse | `function f(x = 'default')` where all call sites omit it | Impossible to tell where the value comes from |
| Null coalesce with no way to pass | `options?.cwd ?? process.cwd()` with no path from callers | Always falls back (meaningless) |
| Return empty value in try-catch | `catch { return ''; }` | Swallows the error |
| Silent skip on inconsistent values | `if (a !== expected) return undefined` | Config errors silently ignored at runtime |
### Correct Implementation
```typescript
// ❌ Prohibited - Fallback for required data
const userId = user?.id ?? 'unknown'
processUser(userId) // Processing continues with 'unknown'
// ✅ Correct - Fail Fast
if (!user?.id) {
throw new Error('User ID is required')
}
processUser(user.id)
// ❌ Prohibited - Default argument where all call sites omit
function loadConfig(path = './config.json') { ... }
// All call sites: loadConfig() ← path is never passed
// ✅ Correct - Make it required and pass explicitly
function loadConfig(path: string) { ... }
// Call site: loadConfig('./config.json') ← explicit
// ❌ Prohibited - Null coalesce with no way to pass
class Engine {
constructor(config, options?) {
this.cwd = options?.cwd ?? process.cwd()
// Problem: if there's no path to pass cwd via options, it always falls back to process.cwd()
}
}
// ✅ Correct - Allow passing from the caller
function createEngine(config, cwd: string) {
return new Engine(config, { cwd })
}
```
### Acceptable Cases
- Default values when validating external input (user input, API responses)
- Optional values in config files (explicitly designed to be omittable)
- Only some call sites use the default argument (prohibited if all callers omit it)
### Decision Criteria
1. **Is it required data?** → Throw an error, do not fall back
2. **Do all call sites omit it?** → Remove the default, make it required
3. **Is there a path to pass the value from above?** → If not, add a parameter or field
4. **Do related values have invariants?** → Cross-validate at load/setup time
## Abstraction
### Think Before Adding Conditionals
- Does the same condition exist elsewhere? → Abstract with a pattern
- Will more branches be added? → Use Strategy/Map pattern
- Branching on type? → Replace with polymorphism
```typescript
// ❌ Growing conditionals
if (type === 'A') { ... }
else if (type === 'B') { ... }
else if (type === 'C') { ... } // Yet another branch
// ✅ Abstract with a Map
const handlers = { A: handleA, B: handleB, C: handleC };
handlers[type]?.();
```
### Keep Abstraction Levels Consistent
Within a single function, keep operations at the same granularity. Extract detailed operations into separate functions. Do not mix "what to do" with "how to do it."
```typescript
// ❌ Mixed abstraction levels
function processOrder(order) {
validateOrder(order); // High level
const conn = pool.getConnection(); // Low-level detail
conn.query('INSERT...'); // Low-level detail
}
// ✅ Consistent abstraction levels
function processOrder(order) {
validateOrder(order);
saveOrder(order); // Details are hidden
}
```
### Follow Language and Framework Conventions
- Write Pythonic Python, idiomatic Kotlin, etc.
- Use framework-recommended patterns
- Prefer standard approaches over custom ones
- When unsure, research. Do not implement based on guesses
### Interface Design
Design interfaces from the consumer's perspective. Do not expose internal implementation details.
| Principle | Criteria |
|-----------|----------|
| Consumer perspective | Do not force things the caller does not need |
| Separate configuration from execution | Decide "what to use" at setup time, keep the execution API simple |
| No method proliferation | Absorb differences through configuration, not multiple methods doing the same thing |
```typescript
// ❌ Method proliferation — pushing configuration differences onto the caller
interface NotificationService {
sendEmail(to, subject, body)
sendSMS(to, message)
sendPush(to, title, body)
sendSlack(channel, message)
}
// ✅ Separate configuration from execution
interface NotificationService {
setup(config: ChannelConfig): Channel
}
interface Channel {
send(message: Message): Promise<Result>
}
```
### Leaky Abstraction
If a specific implementation appears in a generic layer, the abstraction is leaking. The generic layer should only know interfaces; branching should be absorbed by implementations.
```typescript
// ❌ Specific implementation imports and branching in generic layer
import { uploadToS3 } from '../aws/s3.js'
if (config.storage === 's3') {
return uploadToS3(config.bucket, file, options)
}
// ✅ Generic layer uses interface only. Unsupported cases error at creation time
const storage = createStorage(config)
return storage.upload(file, options)
```
## Structure
### Criteria for Splitting
- Has its own state → Separate
- UI/logic exceeding 50 lines → Separate
- Has multiple responsibilities → Separate
### Dependency Direction
- Upper layers → Lower layers (reverse direction prohibited)
- Fetch data at the root (View/Controller) and pass it down
- Children do not know about their parents
### State Management
- Confine state to where it is used
- Children do not modify state directly (notify parents via events)
- State flow is unidirectional
## Error Handling
Centralize error handling. Do not scatter try-catch everywhere.
```typescript
// ❌ Scattered try-catch
async function createUser(data) {
try {
const user = await userService.create(data)
return user
} catch (e) {
console.error(e)
throw new Error('Failed to create user')
}
}
// ✅ Centralized handling at the upper layer
// Catch collectively at the Controller/Handler layer
// Or handle via @ControllerAdvice / ErrorBoundary
async function createUser(data) {
return await userService.create(data) // Let exceptions propagate up
}
```
### Error Handling Placement
| Layer | Responsibility |
|-------|---------------|
| Domain/Service layer | Throw exceptions on business rule violations |
| Controller/Handler layer | Catch exceptions and convert to responses |
| Global handler | Handle common exceptions (NotFound, auth errors, etc.) |
## Conversion Placement
Place conversion methods on the DTO side.
```typescript
// ✅ Conversion methods on Request/Response DTOs
interface CreateUserRequest {
name: string
email: string
}
function toUseCaseInput(req: CreateUserRequest): CreateUserInput {
return { name: req.name, email: req.email }
}
// Controller
const input = toUseCaseInput(request)
const output = await useCase.execute(input)
return UserResponse.from(output)
```
Conversion direction:
```
Request → toInput() → UseCase/Service → Output → Response.from()
```
## Shared Code Decisions
Eliminate duplication by default. When logic is essentially the same and should be unified, apply DRY. Do not decide mechanically by count.
### Should Be Shared
- Essentially identical logic duplicated
- Same style/UI pattern
- Same validation logic
- Same formatting logic
### Should Not Be Shared
- Duplication across different domains (e.g., customer validation and admin validation are separate concerns)
- Superficially similar code with different reasons to change
- Based on "might need it in the future" predictions
```typescript
// ❌ Over-generalization
function formatValue(value, type, options) {
if (type === 'currency') { ... }
else if (type === 'date') { ... }
else if (type === 'percentage') { ... }
}
// ✅ Separate functions by purpose
function formatCurrency(amount: number): string { ... }
function formatDate(date: Date): string { ... }
function formatPercentage(value: number): string { ... }
```
## Prohibited
- **Fallbacks are prohibited by default** - Do not write fallbacks using `?? 'unknown'`, `|| 'default'`, or swallowing via `try-catch`. Propagate errors upward. If absolutely necessary, add a comment explaining why
- **Explanatory comments** - Express intent through code. Do not write What/How comments
- **Unused code** - Do not write "just in case" code
- **any type** - Do not break type safety
- **Direct mutation of objects/arrays** - Create new instances with spread operators
- **console.log** - Do not leave in production code
- **Hardcoded secrets**
- **Scattered try-catch** - Centralize error handling at the upper layer
- **Unsolicited backward compatibility / legacy support** - Not needed unless explicitly instructed
- **Internal implementation exported from public API** - Only export domain-level functions and types. Do not export infrastructure functions or internal classes
- **Replaced code surviving after refactoring** - Remove replaced code and exports. Do not keep unless explicitly told to
- **Workarounds that bypass safety mechanisms** - If the root fix is correct, no additional bypass is needed