Added AI skills
This commit is contained in:
320
.agents/skills/clean-code-principles/rules/_sections.md
Normal file
320
.agents/skills/clean-code-principles/rules/_sections.md
Normal file
@@ -0,0 +1,320 @@
|
||||
# Clean Code Principles - Rule Categories
|
||||
|
||||
This document defines the organizational structure for clean code principles, ordered by priority and impact.
|
||||
|
||||
## Category Overview
|
||||
|
||||
| Priority | Category | Impact | Rule Count | Prefix |
|
||||
|----------|----------|--------|------------|--------|
|
||||
| 1 | SOLID Principles | CRITICAL | 10 | `solid-` |
|
||||
| 2 | Core Principles | CRITICAL | 12 | `core-` |
|
||||
| 3 | Design Patterns | HIGH | 1 | `pattern-` |
|
||||
| 4 | Code Organization | HIGH | 0 | `org-` |
|
||||
| 5 | Naming & Readability | MEDIUM | 0 | `name-` |
|
||||
| 6 | Functions & Methods | MEDIUM | 0 | `func-` |
|
||||
| 7 | Comments & Documentation | LOW | 0 | `doc-` |
|
||||
|
||||
## 1. SOLID Principles (CRITICAL)
|
||||
|
||||
**Priority:** CRITICAL
|
||||
**Impact:** Architectural foundation, affects entire codebase structure
|
||||
**Prefix:** `solid-`
|
||||
|
||||
The five fundamental principles of object-oriented design that guide maintainable, scalable software architecture.
|
||||
|
||||
### Rules
|
||||
|
||||
#### Single Responsibility Principle (SRP)
|
||||
- `solid-srp-class` - A class should have only one reason to change
|
||||
- `solid-srp-function` - A function should do one thing and do it well
|
||||
|
||||
#### Open/Closed Principle (OCP)
|
||||
- `solid-ocp-extension` - Open for extension, closed for modification
|
||||
- `solid-ocp-abstraction` - Use abstractions to enable extension
|
||||
|
||||
#### Liskov Substitution Principle (LSP)
|
||||
- `solid-lsp-contracts` - Subtypes must honor base type contracts
|
||||
- `solid-lsp-preconditions` - Cannot strengthen preconditions or weaken postconditions
|
||||
|
||||
#### Interface Segregation Principle (ISP)
|
||||
- `solid-isp-clients` - Client-specific interfaces, not general-purpose
|
||||
- `solid-isp-interfaces` - Small, cohesive interfaces
|
||||
|
||||
#### Dependency Inversion Principle (DIP)
|
||||
- `solid-dip-abstractions` - Depend on abstractions, not concretions
|
||||
- `solid-dip-injection` - Inject dependencies from outside
|
||||
|
||||
**Key Concepts:**
|
||||
- Architectural soundness
|
||||
- Maintainability at scale
|
||||
- Testability through design
|
||||
- Flexibility for change
|
||||
- Reduced coupling
|
||||
|
||||
**When to Apply:**
|
||||
- Designing new features or systems
|
||||
- Refactoring existing architecture
|
||||
- Addressing technical debt
|
||||
- Improving testability
|
||||
- Planning for future extensibility
|
||||
|
||||
---
|
||||
|
||||
## 2. Core Principles (CRITICAL)
|
||||
|
||||
**Priority:** CRITICAL
|
||||
**Impact:** Daily coding practices, code quality foundation
|
||||
**Prefix:** `core-`
|
||||
|
||||
Fundamental principles that apply to every line of code you write, regardless of paradigm or language.
|
||||
|
||||
### Rules
|
||||
|
||||
#### DRY (Don't Repeat Yourself)
|
||||
- `core-dry` - Every piece of knowledge should have a single representation
|
||||
- `core-dry-extraction` - Extract duplicated code into reusable functions
|
||||
- `core-dry-single-source` - Single source of truth for configuration and data
|
||||
|
||||
#### KISS (Keep It Simple, Stupid)
|
||||
- `core-kiss-simplicity` - Choose the simplest solution that works
|
||||
- `core-kiss-readability` - Optimize for readability over cleverness
|
||||
|
||||
#### YAGNI (You Aren't Gonna Need It)
|
||||
- `core-yagni-features` - Don't implement features before they're needed
|
||||
- `core-yagni-abstractions` - Don't create abstractions prematurely
|
||||
|
||||
#### Other Core Principles
|
||||
- `core-separation-concerns` - Different concerns in different modules
|
||||
- `core-composition` - Favor composition over inheritance
|
||||
- `core-law-demeter` - Only talk to immediate friends
|
||||
- `core-fail-fast` - Detect and report errors early
|
||||
- `core-encapsulation` - Hide implementation details
|
||||
|
||||
**Key Concepts:**
|
||||
- Code duplication elimination
|
||||
- Simplicity over complexity
|
||||
- Lean development
|
||||
- Modularity
|
||||
- Information hiding
|
||||
|
||||
**When to Apply:**
|
||||
- Writing any new code
|
||||
- Code reviews
|
||||
- Refactoring sessions
|
||||
- Bug fixes
|
||||
- Performance optimization
|
||||
|
||||
---
|
||||
|
||||
## 3. Design Patterns (HIGH)
|
||||
|
||||
**Priority:** HIGH
|
||||
**Impact:** Solves recurring problems with proven solutions
|
||||
**Prefix:** `pattern-`
|
||||
|
||||
Common design patterns that provide tested solutions to recurring software design problems.
|
||||
|
||||
### Rules
|
||||
|
||||
- `pattern-repository` - Abstraction for data access layer
|
||||
- `pattern-factory` - Object creation without specifying exact class (planned)
|
||||
- `pattern-strategy` - Encapsulate algorithms for runtime selection (planned)
|
||||
- `pattern-decorator` - Add behavior without modifying objects (planned)
|
||||
- `pattern-observer` - Define one-to-many dependencies (planned)
|
||||
- `pattern-adapter` - Make incompatible interfaces work together (planned)
|
||||
- `pattern-facade` - Simplified interface to complex subsystems (planned)
|
||||
|
||||
**Key Concepts:**
|
||||
- Proven solutions
|
||||
- Common vocabulary
|
||||
- Design reusability
|
||||
- Best practices codified
|
||||
- Language-agnostic approaches
|
||||
|
||||
**When to Apply:**
|
||||
- Solving common architectural problems
|
||||
- Improving code structure
|
||||
- Reducing coupling between components
|
||||
- Making systems more testable
|
||||
- Communicating design intent
|
||||
|
||||
---
|
||||
|
||||
## 4. Code Organization (HIGH)
|
||||
|
||||
**Priority:** HIGH
|
||||
**Impact:** Project structure, module boundaries, discoverability
|
||||
**Prefix:** `org-`
|
||||
|
||||
Principles for organizing code into modules, packages, and directories for maintainability and scalability.
|
||||
|
||||
### Rules (Planned)
|
||||
|
||||
- `org-feature-folders` - Organize by feature, not by layer
|
||||
- `org-module-boundaries` - Clear boundaries between modules
|
||||
- `org-layered-architecture` - Proper separation of layers
|
||||
- `org-package-cohesion` - Keep related code together
|
||||
- `org-circular-dependencies` - Avoid circular imports
|
||||
|
||||
**Key Concepts:**
|
||||
- Feature-based organization
|
||||
- Module boundaries
|
||||
- Layer separation
|
||||
- Dependency direction
|
||||
- Discoverability
|
||||
|
||||
**When to Apply:**
|
||||
- Starting new projects
|
||||
- Restructuring existing codebases
|
||||
- Scaling applications
|
||||
- Onboarding new team members
|
||||
- Managing microservices
|
||||
|
||||
---
|
||||
|
||||
## 5. Naming & Readability (MEDIUM)
|
||||
|
||||
**Priority:** MEDIUM
|
||||
**Impact:** Code comprehension, maintenance speed
|
||||
**Prefix:** `name-`
|
||||
|
||||
Conventions and principles for naming variables, functions, classes, and other identifiers.
|
||||
|
||||
### Rules (Planned)
|
||||
|
||||
- `name-meaningful` - Use intention-revealing names
|
||||
- `name-consistent` - Follow consistent naming conventions
|
||||
- `name-searchable` - Avoid magic numbers and strings
|
||||
- `name-avoid-encodings` - No Hungarian notation
|
||||
- `name-domain-language` - Use ubiquitous domain language
|
||||
|
||||
**Key Concepts:**
|
||||
- Intention revelation
|
||||
- Consistency
|
||||
- Searchability
|
||||
- Domain terminology
|
||||
- Avoid abbreviations
|
||||
|
||||
**When to Apply:**
|
||||
- Creating new identifiers
|
||||
- Refactoring unclear names
|
||||
- Code reviews
|
||||
- Domain modeling
|
||||
- API design
|
||||
|
||||
---
|
||||
|
||||
## 6. Functions & Methods (MEDIUM)
|
||||
|
||||
**Priority:** MEDIUM
|
||||
**Impact:** Code readability, testability at function level
|
||||
**Prefix:** `func-`
|
||||
|
||||
Principles for writing clean, focused functions and methods.
|
||||
|
||||
### Rules (Planned)
|
||||
|
||||
- `func-small` - Keep functions small and focused
|
||||
- `func-single-purpose` - Do one thing only
|
||||
- `func-few-arguments` - Limit function parameters
|
||||
- `func-no-side-effects` - Minimize or document side effects
|
||||
- `func-command-query` - Separate commands from queries
|
||||
|
||||
**Key Concepts:**
|
||||
- Small functions
|
||||
- Single purpose
|
||||
- Few parameters
|
||||
- Pure functions when possible
|
||||
- Predictable behavior
|
||||
|
||||
**When to Apply:**
|
||||
- Writing new functions
|
||||
- Refactoring long methods
|
||||
- Improving testability
|
||||
- Code reviews
|
||||
- Performance optimization
|
||||
|
||||
---
|
||||
|
||||
## 7. Comments & Documentation (LOW)
|
||||
|
||||
**Priority:** LOW
|
||||
**Impact:** Code maintainability, knowledge transfer
|
||||
**Prefix:** `doc-`
|
||||
|
||||
Guidelines for when and how to use comments and documentation effectively.
|
||||
|
||||
### Rules (Planned)
|
||||
|
||||
- `doc-self-documenting` - Write code that explains itself
|
||||
- `doc-why-not-what` - Comments should explain why, not what
|
||||
- `doc-avoid-noise` - No redundant or obvious comments
|
||||
- `doc-api-docs` - Document public APIs and interfaces
|
||||
|
||||
**Key Concepts:**
|
||||
- Self-documenting code
|
||||
- Intent over implementation
|
||||
- Avoid redundancy
|
||||
- Public API documentation
|
||||
- Living documentation
|
||||
|
||||
**When to Apply:**
|
||||
- Complex business logic
|
||||
- Non-obvious algorithms
|
||||
- Public APIs
|
||||
- Architectural decisions
|
||||
- Workarounds and hacks
|
||||
|
||||
---
|
||||
|
||||
## Rule Naming Convention
|
||||
|
||||
All rules follow a consistent naming pattern:
|
||||
|
||||
```
|
||||
{prefix}-{concept}-{specificity}
|
||||
```
|
||||
|
||||
Examples:
|
||||
- `solid-srp-class` - SOLID principle, SRP concept, class level
|
||||
- `core-dry-extraction` - Core principle, DRY concept, extraction technique
|
||||
- `pattern-repository` - Design pattern category, repository pattern
|
||||
|
||||
## Priority Levels Explained
|
||||
|
||||
- **CRITICAL**: Core architectural and coding principles. Violations significantly impact maintainability, testability, and scalability.
|
||||
- **HIGH**: Important patterns and organizational principles. Violations complicate future development.
|
||||
- **MEDIUM**: Best practices that improve code quality. Violations make code harder to read and maintain.
|
||||
- **LOW**: Nice-to-have practices. Violations have minimal impact but reduce clarity.
|
||||
|
||||
## Impact Assessment
|
||||
|
||||
- **CRITICAL Impact**: Affects entire system architecture, multiple teams, long-term maintainability
|
||||
- **HIGH Impact**: Affects module design, team productivity, medium-term maintainability
|
||||
- **MEDIUM Impact**: Affects code readability, individual developer productivity
|
||||
- **LOW Impact**: Affects code clarity, documentation quality
|
||||
|
||||
## Usage Guidelines
|
||||
|
||||
1. Start with SOLID and Core Principles - these are non-negotiable
|
||||
2. Apply Design Patterns when solving specific architectural problems
|
||||
3. Use Code Organization principles when structuring projects
|
||||
4. Follow Naming & Readability guidelines for all new code
|
||||
5. Apply Function principles during refactoring and new development
|
||||
6. Add Comments only when necessary to explain complex logic
|
||||
|
||||
## Cross-References
|
||||
|
||||
Rules often relate to each other. The `related` field in each rule's frontmatter indicates:
|
||||
- Rules that commonly apply together
|
||||
- Rules that solve similar problems
|
||||
- Rules that complement each other
|
||||
- Rules that provide context or prerequisites
|
||||
|
||||
## Evolution
|
||||
|
||||
This categorization will evolve as:
|
||||
- New rules are added
|
||||
- Patterns emerge from practice
|
||||
- Team feedback is incorporated
|
||||
- Language-specific adaptations are needed
|
||||
211
.agents/skills/clean-code-principles/rules/_template.md
Normal file
211
.agents/skills/clean-code-principles/rules/_template.md
Normal file
@@ -0,0 +1,211 @@
|
||||
---
|
||||
id: {prefix}-{concept}-{specificity}
|
||||
title: {Full Descriptive Title}
|
||||
category: {solid-principles|core-principles|design-patterns|code-organization|naming-readability|functions-methods|comments-documentation}
|
||||
priority: {critical|high|medium|low}
|
||||
tags: [{tag1}, {tag2}, {tag3}, {tag4}]
|
||||
related: [{rule-id-1}, {rule-id-2}, {rule-id-3}]
|
||||
---
|
||||
|
||||
# {Rule Title}
|
||||
|
||||
{One or two sentence summary explaining the principle and why it matters. Should be clear and actionable.}
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: {Brief description of what's wrong}
|
||||
|
||||
{Code example demonstrating the violation}
|
||||
|
||||
// Problems:
|
||||
// 1. {Specific issue 1}
|
||||
// 2. {Specific issue 2}
|
||||
// 3. {Specific issue 3}
|
||||
```
|
||||
|
||||
**Why This Is Wrong:**
|
||||
- {Consequence 1}
|
||||
- {Consequence 2}
|
||||
- {Consequence 3}
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: {Brief description of the solution}
|
||||
|
||||
{Code example demonstrating proper implementation}
|
||||
|
||||
// Benefits:
|
||||
// 1. {Benefit 1}
|
||||
// 2. {Benefit 2}
|
||||
// 3. {Benefit 3}
|
||||
```
|
||||
|
||||
**Alternative Approach (Optional):**
|
||||
|
||||
```typescript
|
||||
// Another valid solution: {When this might be preferred}
|
||||
|
||||
{Alternative code example if applicable}
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
Explanation of the principle and its benefits:
|
||||
|
||||
1. **{Benefit Category 1}**: {Detailed explanation}
|
||||
|
||||
2. **{Benefit Category 2}**: {Detailed explanation}
|
||||
|
||||
3. **{Benefit Category 3}**: {Detailed explanation}
|
||||
|
||||
4. **{Benefit Category 4}**: {Detailed explanation}
|
||||
|
||||
5. **{Benefit Category 5}**: {Detailed explanation}
|
||||
|
||||
6. **{Benefit Category 6}**: {Detailed explanation}
|
||||
|
||||
7. **{Benefit Category 7}**: {Detailed explanation}
|
||||
|
||||
## When to Apply
|
||||
|
||||
- {Situation 1}
|
||||
- {Situation 2}
|
||||
- {Situation 3}
|
||||
- {Situation 4}
|
||||
|
||||
## When NOT to Apply (Optional)
|
||||
|
||||
```typescript
|
||||
// Acceptable exception: {Scenario where the rule can be relaxed}
|
||||
|
||||
{Code example of acceptable violation with clear reasoning}
|
||||
|
||||
// This is acceptable because:
|
||||
// - {Reason 1}
|
||||
// - {Reason 2}
|
||||
```
|
||||
|
||||
## Common Mistakes (Optional)
|
||||
|
||||
### Mistake 1: {Common misunderstanding}
|
||||
|
||||
```typescript
|
||||
// ❌ Wrong
|
||||
{Code showing mistake}
|
||||
|
||||
// ✅ Correct
|
||||
{Code showing correction}
|
||||
```
|
||||
|
||||
### Mistake 2: {Another common issue}
|
||||
|
||||
```typescript
|
||||
// ❌ Wrong
|
||||
{Code showing mistake}
|
||||
|
||||
// ✅ Correct
|
||||
{Code showing correction}
|
||||
```
|
||||
|
||||
## Testing Implications (Optional)
|
||||
|
||||
How this principle affects testing:
|
||||
|
||||
```typescript
|
||||
// Test example showing improved testability
|
||||
{Test code demonstrating benefits}
|
||||
```
|
||||
|
||||
## Real-World Example (Optional)
|
||||
|
||||
{Brief description of how this applies in production scenarios}
|
||||
|
||||
```typescript
|
||||
// Production scenario: {Description}
|
||||
{Realistic code example}
|
||||
```
|
||||
|
||||
## Related Principles
|
||||
|
||||
- **{Related Rule 1}**: {Brief explanation of relationship}
|
||||
- **{Related Rule 2}**: {Brief explanation of relationship}
|
||||
- **{Related Rule 3}**: {Brief explanation of relationship}
|
||||
|
||||
## Further Reading (Optional)
|
||||
|
||||
- {Resource title} - {URL or reference}
|
||||
- {Resource title} - {URL or reference}
|
||||
|
||||
## Language-Specific Notes (Optional)
|
||||
|
||||
### TypeScript/JavaScript
|
||||
{Language-specific considerations}
|
||||
|
||||
### Python
|
||||
{Language-specific considerations}
|
||||
|
||||
### Java
|
||||
{Language-specific considerations}
|
||||
|
||||
### Go
|
||||
{Language-specific considerations}
|
||||
|
||||
---
|
||||
|
||||
## Template Guidelines
|
||||
|
||||
### Frontmatter
|
||||
|
||||
- **id**: Use format `{prefix}-{concept}-{specificity}`. Must be unique and match filename.
|
||||
- **title**: Full descriptive title, human-readable
|
||||
- **category**: One of the 7 defined categories
|
||||
- **priority**: critical (SOLID, Core) | high (Patterns, Org) | medium (Naming, Functions) | low (Comments)
|
||||
- **tags**: 3-5 relevant tags for searchability
|
||||
- **related**: 2-4 related rule IDs that commonly apply together
|
||||
|
||||
### Content Structure
|
||||
|
||||
1. **Title & Summary**: Clear, one-sentence explanation
|
||||
2. **Bad Example**: Show the anti-pattern with clear problems listed
|
||||
3. **Good Example**: Show proper implementation with benefits
|
||||
4. **Why**: 5-7 benefits explaining the value
|
||||
5. **When to Apply**: Practical scenarios
|
||||
6. **Optional Sections**: Add as needed for complex rules
|
||||
|
||||
### Code Examples
|
||||
|
||||
- Use TypeScript for primary examples (language-agnostic)
|
||||
- Keep examples focused and minimal
|
||||
- Show realistic scenarios, not toy examples
|
||||
- Include comments explaining key points
|
||||
- Use ❌ for bad examples, ✅ for good examples
|
||||
|
||||
### Writing Style
|
||||
|
||||
- Be direct and actionable
|
||||
- Focus on "why" not just "what"
|
||||
- Use active voice
|
||||
- Keep explanations concise
|
||||
- Provide context for decisions
|
||||
- Assume intermediate developer knowledge
|
||||
|
||||
### Length Guidelines
|
||||
|
||||
- Minimum: 200 lines (simple rules)
|
||||
- Target: 300-400 lines (most rules)
|
||||
- Maximum: 600 lines (complex patterns)
|
||||
|
||||
### Quality Checklist
|
||||
|
||||
- [ ] Frontmatter complete and accurate
|
||||
- [ ] Clear bad example with explained problems
|
||||
- [ ] Clear good example with explained benefits
|
||||
- [ ] At least 5 benefits in "Why" section
|
||||
- [ ] Practical "When to Apply" scenarios
|
||||
- [ ] Related rules referenced
|
||||
- [ ] Code examples are realistic
|
||||
- [ ] Comments explain key concepts
|
||||
- [ ] Language-agnostic where possible
|
||||
- [ ] Proofread for clarity and typos
|
||||
317
.agents/skills/clean-code-principles/rules/core-composition.md
Normal file
317
.agents/skills/clean-code-principles/rules/core-composition.md
Normal file
@@ -0,0 +1,317 @@
|
||||
---
|
||||
id: core-composition
|
||||
title: Composition Over Inheritance
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [composition, inheritance, flexibility, design]
|
||||
related: [solid-srp-class, solid-dip-injection, core-encapsulation]
|
||||
---
|
||||
|
||||
# Composition Over Inheritance
|
||||
|
||||
Favor composing objects from smaller, focused pieces over building deep inheritance hierarchies. Composition provides more flexibility, better encapsulation, and avoids the fragile base class problem.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Deep inheritance hierarchy
|
||||
|
||||
class Animal {
|
||||
protected name: string;
|
||||
protected energy: number = 100;
|
||||
|
||||
constructor(name: string) {
|
||||
this.name = name;
|
||||
}
|
||||
|
||||
eat(amount: number): void {
|
||||
this.energy += amount;
|
||||
console.log(`${this.name} is eating. Energy: ${this.energy}`);
|
||||
}
|
||||
|
||||
sleep(hours: number): void {
|
||||
this.energy += hours * 10;
|
||||
console.log(`${this.name} slept for ${hours} hours. Energy: ${this.energy}`);
|
||||
}
|
||||
}
|
||||
|
||||
class Bird extends Animal {
|
||||
fly(): void {
|
||||
this.energy -= 20;
|
||||
console.log(`${this.name} is flying. Energy: ${this.energy}`);
|
||||
}
|
||||
}
|
||||
|
||||
class Duck extends Bird {
|
||||
swim(): void {
|
||||
this.energy -= 5;
|
||||
console.log(`${this.name} is swimming. Energy: ${this.energy}`);
|
||||
}
|
||||
|
||||
quack(): void {
|
||||
console.log(`${this.name} says quack!`);
|
||||
}
|
||||
}
|
||||
|
||||
class FlyingFish extends Animal {
|
||||
// Problem: Can't inherit from both Bird and Fish
|
||||
// Must duplicate flying code or create awkward hierarchy
|
||||
|
||||
swim(): void {
|
||||
this.energy -= 5;
|
||||
console.log(`${this.name} is swimming. Energy: ${this.energy}`);
|
||||
}
|
||||
|
||||
// Duplicated from Bird class!
|
||||
fly(): void {
|
||||
this.energy -= 20;
|
||||
console.log(`${this.name} is flying. Energy: ${this.energy}`);
|
||||
}
|
||||
}
|
||||
|
||||
class Penguin extends Bird {
|
||||
// Problem: Penguins can't fly but inherit fly()
|
||||
// Must override to throw error - LSP violation
|
||||
|
||||
fly(): void {
|
||||
throw new Error('Penguins cannot fly!');
|
||||
}
|
||||
|
||||
swim(): void {
|
||||
this.energy -= 5;
|
||||
console.log(`${this.name} is swimming. Energy: ${this.energy}`);
|
||||
}
|
||||
}
|
||||
|
||||
// More problems:
|
||||
// - What about a robot bird? It doesn't eat or sleep.
|
||||
// - What about a bat? It flies but isn't a bird.
|
||||
// - Every change to Animal affects all subclasses.
|
||||
// - Testing requires understanding entire hierarchy.
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Composition with focused behaviors
|
||||
|
||||
// Define behaviors as interfaces
|
||||
interface Eater {
|
||||
eat(amount: number): void;
|
||||
}
|
||||
|
||||
interface Sleeper {
|
||||
sleep(hours: number): void;
|
||||
}
|
||||
|
||||
interface Flyer {
|
||||
fly(): void;
|
||||
}
|
||||
|
||||
interface Swimmer {
|
||||
swim(): void;
|
||||
}
|
||||
|
||||
interface Speaker {
|
||||
speak(): void;
|
||||
}
|
||||
|
||||
// Implement behaviors as standalone classes
|
||||
class StandardEater implements Eater {
|
||||
constructor(private entity: { name: string; energy: number }) {}
|
||||
|
||||
eat(amount: number): void {
|
||||
this.entity.energy += amount;
|
||||
console.log(`${this.entity.name} is eating. Energy: ${this.entity.energy}`);
|
||||
}
|
||||
}
|
||||
|
||||
class StandardSleeper implements Sleeper {
|
||||
constructor(private entity: { name: string; energy: number }) {}
|
||||
|
||||
sleep(hours: number): void {
|
||||
this.entity.energy += hours * 10;
|
||||
console.log(`${this.entity.name} slept for ${hours} hours. Energy: ${this.entity.energy}`);
|
||||
}
|
||||
}
|
||||
|
||||
class WingedFlyer implements Flyer {
|
||||
constructor(
|
||||
private entity: { name: string; energy: number },
|
||||
private energyCost: number = 20
|
||||
) {}
|
||||
|
||||
fly(): void {
|
||||
this.entity.energy -= this.energyCost;
|
||||
console.log(`${this.entity.name} is flying. Energy: ${this.entity.energy}`);
|
||||
}
|
||||
}
|
||||
|
||||
class AquaticSwimmer implements Swimmer {
|
||||
constructor(
|
||||
private entity: { name: string; energy: number },
|
||||
private energyCost: number = 5
|
||||
) {}
|
||||
|
||||
swim(): void {
|
||||
this.entity.energy -= this.energyCost;
|
||||
console.log(`${this.entity.name} is swimming. Energy: ${this.entity.energy}`);
|
||||
}
|
||||
}
|
||||
|
||||
// Compose animals from behaviors
|
||||
class Duck implements Eater, Sleeper, Flyer, Swimmer, Speaker {
|
||||
public name: string;
|
||||
public energy: number = 100;
|
||||
|
||||
private eater: Eater;
|
||||
private sleeper: Sleeper;
|
||||
private flyer: Flyer;
|
||||
private swimmer: Swimmer;
|
||||
|
||||
constructor(name: string) {
|
||||
this.name = name;
|
||||
this.eater = new StandardEater(this);
|
||||
this.sleeper = new StandardSleeper(this);
|
||||
this.flyer = new WingedFlyer(this);
|
||||
this.swimmer = new AquaticSwimmer(this);
|
||||
}
|
||||
|
||||
eat(amount: number): void {
|
||||
this.eater.eat(amount);
|
||||
}
|
||||
|
||||
sleep(hours: number): void {
|
||||
this.sleeper.sleep(hours);
|
||||
}
|
||||
|
||||
fly(): void {
|
||||
this.flyer.fly();
|
||||
}
|
||||
|
||||
swim(): void {
|
||||
this.swimmer.swim();
|
||||
}
|
||||
|
||||
speak(): void {
|
||||
console.log(`${this.name} says quack!`);
|
||||
}
|
||||
}
|
||||
|
||||
// Penguin: swims but doesn't fly - no problem!
|
||||
class Penguin implements Eater, Sleeper, Swimmer, Speaker {
|
||||
public name: string;
|
||||
public energy: number = 100;
|
||||
|
||||
private eater: Eater;
|
||||
private sleeper: Sleeper;
|
||||
private swimmer: Swimmer;
|
||||
|
||||
constructor(name: string) {
|
||||
this.name = name;
|
||||
this.eater = new StandardEater(this);
|
||||
this.sleeper = new StandardSleeper(this);
|
||||
this.swimmer = new AquaticSwimmer(this);
|
||||
}
|
||||
|
||||
eat(amount: number): void {
|
||||
this.eater.eat(amount);
|
||||
}
|
||||
|
||||
sleep(hours: number): void {
|
||||
this.sleeper.sleep(hours);
|
||||
}
|
||||
|
||||
swim(): void {
|
||||
this.swimmer.swim();
|
||||
}
|
||||
|
||||
speak(): void {
|
||||
console.log(`${this.name} says squawk!`);
|
||||
}
|
||||
}
|
||||
|
||||
// Flying fish: swims and flies - easy!
|
||||
class FlyingFish implements Swimmer, Flyer {
|
||||
public name: string;
|
||||
public energy: number = 100;
|
||||
|
||||
private swimmer: Swimmer;
|
||||
private flyer: Flyer;
|
||||
|
||||
constructor(name: string) {
|
||||
this.name = name;
|
||||
this.swimmer = new AquaticSwimmer(this);
|
||||
this.flyer = new WingedFlyer(this, 30); // Different energy cost
|
||||
}
|
||||
|
||||
swim(): void {
|
||||
this.swimmer.swim();
|
||||
}
|
||||
|
||||
fly(): void {
|
||||
this.flyer.fly();
|
||||
}
|
||||
}
|
||||
|
||||
// Robot bird: flies but doesn't eat or sleep
|
||||
class RobotBird implements Flyer, Speaker {
|
||||
public name: string;
|
||||
public energy: number = 100;
|
||||
|
||||
private flyer: Flyer;
|
||||
|
||||
constructor(name: string) {
|
||||
this.name = name;
|
||||
this.flyer = new WingedFlyer(this, 10); // Efficient robot
|
||||
}
|
||||
|
||||
fly(): void {
|
||||
this.flyer.fly();
|
||||
}
|
||||
|
||||
speak(): void {
|
||||
console.log(`${this.name} says BEEP BOOP!`);
|
||||
}
|
||||
|
||||
recharge(): void {
|
||||
this.energy = 100;
|
||||
console.log(`${this.name} recharged to full energy.`);
|
||||
}
|
||||
}
|
||||
|
||||
// Functions work with any entity that has the required behavior
|
||||
function makeEntityFly(flyer: Flyer): void {
|
||||
flyer.fly();
|
||||
}
|
||||
|
||||
function feedEntity(eater: Eater, amount: number): void {
|
||||
eater.eat(amount);
|
||||
}
|
||||
|
||||
// Works with duck, flying fish, or robot bird
|
||||
makeEntityFly(new Duck('Donald'));
|
||||
makeEntityFly(new FlyingFish('Nemo'));
|
||||
makeEntityFly(new RobotBird('R2D2'));
|
||||
|
||||
// Works with duck or penguin, but not robot bird (correctly!)
|
||||
feedEntity(new Duck('Donald'), 50);
|
||||
feedEntity(new Penguin('Pingu'), 50);
|
||||
// feedEntity(new RobotBird('R2D2'), 50); // Type error - RobotBird isn't an Eater
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Flexibility**: Compose any combination of behaviors. No artificial hierarchy constraints.
|
||||
|
||||
2. **Avoids Diamond Problem**: No multiple inheritance issues. Just implement multiple interfaces.
|
||||
|
||||
3. **LSP Compliance**: No need to override methods to throw errors. Types only have methods they actually support.
|
||||
|
||||
4. **Reusability**: Behaviors can be reused across unrelated types.
|
||||
|
||||
5. **Testability**: Test behaviors in isolation. Mock specific behaviors easily.
|
||||
|
||||
6. **Runtime Flexibility**: Can change behaviors at runtime by swapping implementations.
|
||||
|
||||
7. **Stable Dependencies**: Behavior implementations are stable. Adding new composed types doesn't affect existing code.
|
||||
@@ -0,0 +1,316 @@
|
||||
---
|
||||
id: core-dry-extraction
|
||||
title: DRY - Code Extraction
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [DRY, refactoring, extraction, code-reuse]
|
||||
related: [core-dry, core-dry-single-source, solid-srp-function]
|
||||
---
|
||||
|
||||
# DRY - Code Extraction
|
||||
|
||||
Don't Repeat Yourself. When you find duplicated code, extract it into a reusable function, method, or module. Every piece of knowledge should have a single, unambiguous representation.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Same validation logic repeated in multiple places
|
||||
|
||||
class UserController {
|
||||
async createUser(req: Request, res: Response): Promise<void> {
|
||||
const { email, password, name } = req.body;
|
||||
|
||||
// Email validation - duplicated
|
||||
if (!email) {
|
||||
res.status(400).json({ error: 'Email is required' });
|
||||
return;
|
||||
}
|
||||
if (!email.includes('@') || !email.includes('.')) {
|
||||
res.status(400).json({ error: 'Invalid email format' });
|
||||
return;
|
||||
}
|
||||
if (email.length > 255) {
|
||||
res.status(400).json({ error: 'Email too long' });
|
||||
return;
|
||||
}
|
||||
|
||||
// Password validation - duplicated
|
||||
if (!password) {
|
||||
res.status(400).json({ error: 'Password is required' });
|
||||
return;
|
||||
}
|
||||
if (password.length < 8) {
|
||||
res.status(400).json({ error: 'Password must be at least 8 characters' });
|
||||
return;
|
||||
}
|
||||
if (!/[A-Z]/.test(password)) {
|
||||
res.status(400).json({ error: 'Password must contain uppercase letter' });
|
||||
return;
|
||||
}
|
||||
if (!/[0-9]/.test(password)) {
|
||||
res.status(400).json({ error: 'Password must contain a number' });
|
||||
return;
|
||||
}
|
||||
|
||||
// Create user...
|
||||
}
|
||||
|
||||
async updateUser(req: Request, res: Response): Promise<void> {
|
||||
const { email, password } = req.body;
|
||||
|
||||
// Same email validation repeated
|
||||
if (email) {
|
||||
if (!email.includes('@') || !email.includes('.')) {
|
||||
res.status(400).json({ error: 'Invalid email format' });
|
||||
return;
|
||||
}
|
||||
if (email.length > 255) {
|
||||
res.status(400).json({ error: 'Email too long' });
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Same password validation repeated
|
||||
if (password) {
|
||||
if (password.length < 8) {
|
||||
res.status(400).json({ error: 'Password must be at least 8 characters' });
|
||||
return;
|
||||
}
|
||||
if (!/[A-Z]/.test(password)) {
|
||||
res.status(400).json({ error: 'Password must contain uppercase letter' });
|
||||
return;
|
||||
}
|
||||
if (!/[0-9]/.test(password)) {
|
||||
res.status(400).json({ error: 'Password must contain a number' });
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Update user...
|
||||
}
|
||||
|
||||
async resetPassword(req: Request, res: Response): Promise<void> {
|
||||
const { email, newPassword } = req.body;
|
||||
|
||||
// Email validation repeated again
|
||||
if (!email) {
|
||||
res.status(400).json({ error: 'Email is required' });
|
||||
return;
|
||||
}
|
||||
if (!email.includes('@') || !email.includes('.')) {
|
||||
res.status(400).json({ error: 'Invalid email format' });
|
||||
return;
|
||||
}
|
||||
|
||||
// Password validation repeated again
|
||||
if (!newPassword) {
|
||||
res.status(400).json({ error: 'New password is required' });
|
||||
return;
|
||||
}
|
||||
if (newPassword.length < 8) {
|
||||
res.status(400).json({ error: 'Password must be at least 8 characters' });
|
||||
return;
|
||||
}
|
||||
if (!/[A-Z]/.test(newPassword)) {
|
||||
res.status(400).json({ error: 'Password must contain uppercase letter' });
|
||||
return;
|
||||
}
|
||||
if (!/[0-9]/.test(newPassword)) {
|
||||
res.status(400).json({ error: 'Password must contain a number' });
|
||||
return;
|
||||
}
|
||||
|
||||
// Reset password...
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Extract reusable validation functions
|
||||
|
||||
// Validation result type
|
||||
interface ValidationResult {
|
||||
isValid: boolean;
|
||||
errors: string[];
|
||||
}
|
||||
|
||||
// Reusable validation functions
|
||||
class Validators {
|
||||
static email(email: string | undefined, options: { required?: boolean } = {}): ValidationResult {
|
||||
const errors: string[] = [];
|
||||
|
||||
if (!email) {
|
||||
if (options.required) {
|
||||
errors.push('Email is required');
|
||||
}
|
||||
return { isValid: !options.required, errors };
|
||||
}
|
||||
|
||||
if (!email.includes('@') || !email.includes('.')) {
|
||||
errors.push('Invalid email format');
|
||||
}
|
||||
|
||||
if (email.length > 255) {
|
||||
errors.push('Email must be 255 characters or less');
|
||||
}
|
||||
|
||||
return { isValid: errors.length === 0, errors };
|
||||
}
|
||||
|
||||
static password(password: string | undefined, options: { required?: boolean } = {}): ValidationResult {
|
||||
const errors: string[] = [];
|
||||
|
||||
if (!password) {
|
||||
if (options.required) {
|
||||
errors.push('Password is required');
|
||||
}
|
||||
return { isValid: !options.required, errors };
|
||||
}
|
||||
|
||||
if (password.length < 8) {
|
||||
errors.push('Password must be at least 8 characters');
|
||||
}
|
||||
|
||||
if (!/[A-Z]/.test(password)) {
|
||||
errors.push('Password must contain at least one uppercase letter');
|
||||
}
|
||||
|
||||
if (!/[a-z]/.test(password)) {
|
||||
errors.push('Password must contain at least one lowercase letter');
|
||||
}
|
||||
|
||||
if (!/[0-9]/.test(password)) {
|
||||
errors.push('Password must contain at least one number');
|
||||
}
|
||||
|
||||
return { isValid: errors.length === 0, errors };
|
||||
}
|
||||
|
||||
static combine(...results: ValidationResult[]): ValidationResult {
|
||||
const errors = results.flatMap(r => r.errors);
|
||||
return { isValid: errors.length === 0, errors };
|
||||
}
|
||||
}
|
||||
|
||||
// Reusable error response helper
|
||||
function validationError(res: Response, errors: string[]): void {
|
||||
res.status(400).json({ errors });
|
||||
}
|
||||
|
||||
// Clean controller using extracted validations
|
||||
class UserController {
|
||||
async createUser(req: Request, res: Response): Promise<void> {
|
||||
const { email, password, name } = req.body;
|
||||
|
||||
const validation = Validators.combine(
|
||||
Validators.email(email, { required: true }),
|
||||
Validators.password(password, { required: true })
|
||||
);
|
||||
|
||||
if (!validation.isValid) {
|
||||
return validationError(res, validation.errors);
|
||||
}
|
||||
|
||||
// Create user...
|
||||
}
|
||||
|
||||
async updateUser(req: Request, res: Response): Promise<void> {
|
||||
const { email, password } = req.body;
|
||||
|
||||
const validation = Validators.combine(
|
||||
Validators.email(email),
|
||||
Validators.password(password)
|
||||
);
|
||||
|
||||
if (!validation.isValid) {
|
||||
return validationError(res, validation.errors);
|
||||
}
|
||||
|
||||
// Update user...
|
||||
}
|
||||
|
||||
async resetPassword(req: Request, res: Response): Promise<void> {
|
||||
const { email, newPassword } = req.body;
|
||||
|
||||
const validation = Validators.combine(
|
||||
Validators.email(email, { required: true }),
|
||||
Validators.password(newPassword, { required: true })
|
||||
);
|
||||
|
||||
if (!validation.isValid) {
|
||||
return validationError(res, validation.errors);
|
||||
}
|
||||
|
||||
// Reset password...
|
||||
}
|
||||
}
|
||||
|
||||
// Validators can be reused across the application
|
||||
class AdminController {
|
||||
async inviteUser(req: Request, res: Response): Promise<void> {
|
||||
const { email } = req.body;
|
||||
|
||||
const validation = Validators.email(email, { required: true });
|
||||
|
||||
if (!validation.isValid) {
|
||||
return validationError(res, validation.errors);
|
||||
}
|
||||
|
||||
// Send invite...
|
||||
}
|
||||
}
|
||||
|
||||
// Easy to test in isolation
|
||||
describe('Validators', () => {
|
||||
describe('email', () => {
|
||||
it('should reject invalid email format', () => {
|
||||
const result = Validators.email('invalid');
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.errors).toContain('Invalid email format');
|
||||
});
|
||||
|
||||
it('should accept valid email', () => {
|
||||
const result = Validators.email('user@example.com');
|
||||
expect(result.isValid).toBe(true);
|
||||
});
|
||||
|
||||
it('should require email when required option is set', () => {
|
||||
const result = Validators.email(undefined, { required: true });
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.errors).toContain('Email is required');
|
||||
});
|
||||
});
|
||||
|
||||
describe('password', () => {
|
||||
it('should reject short passwords', () => {
|
||||
const result = Validators.password('short');
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.errors).toContain('Password must be at least 8 characters');
|
||||
});
|
||||
|
||||
it('should require uppercase letter', () => {
|
||||
const result = Validators.password('lowercase123');
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.errors).toContain('Password must contain at least one uppercase letter');
|
||||
});
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Single Source of Truth**: Password rules are defined once. Change them in one place, and all usages are updated.
|
||||
|
||||
2. **Consistency**: All email validations behave the same way. No risk of inconsistent error messages or rules.
|
||||
|
||||
3. **Easier Testing**: Test the validation logic once, thoroughly. No need to test the same logic in every controller.
|
||||
|
||||
4. **Bug Fixes Propagate**: Fix a bug in `Validators.email()`, and it's fixed everywhere.
|
||||
|
||||
5. **Reduced Code Size**: Less code means less to read, less to maintain, and fewer places for bugs.
|
||||
|
||||
6. **Better Abstraction**: Controllers focus on HTTP concerns, validators focus on validation.
|
||||
|
||||
7. **Reusability**: Same validators work in controllers, services, CLI tools, or anywhere else.
|
||||
@@ -0,0 +1,282 @@
|
||||
---
|
||||
id: core-dry-single-source
|
||||
title: DRY - Single Source of Truth
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [DRY, single-source-of-truth, constants, configuration]
|
||||
related: [core-dry, core-dry-extraction, core-encapsulation]
|
||||
---
|
||||
|
||||
# DRY - Single Source of Truth
|
||||
|
||||
Every piece of knowledge or configuration should exist in exactly one place. When data or logic needs to be referenced from multiple locations, use a single authoritative source.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Same values defined in multiple places
|
||||
|
||||
// In constants file
|
||||
const API_BASE_URL = 'https://api.example.com/v1';
|
||||
|
||||
// In another file - duplicate!
|
||||
const baseUrl = 'https://api.example.com/v1';
|
||||
|
||||
// In config.ts - another duplicate!
|
||||
export const config = {
|
||||
apiUrl: 'https://api.example.com/v1'
|
||||
};
|
||||
|
||||
// In API client - yet another!
|
||||
class ApiClient {
|
||||
private baseUrl = 'https://api.example.com/v1'; // Duplicated!
|
||||
}
|
||||
|
||||
// Status codes defined in multiple places
|
||||
class OrderService {
|
||||
async getOrder(id: string): Promise<Order> {
|
||||
const order = await this.repository.findById(id);
|
||||
if (order.status === 'pending') { // Magic string
|
||||
// ...
|
||||
}
|
||||
if (order.status === 'completed') { // Magic string
|
||||
// ...
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
class OrderController {
|
||||
async listPendingOrders(): Promise<Order[]> {
|
||||
return this.repository.findByStatus('pending'); // Same magic string
|
||||
}
|
||||
}
|
||||
|
||||
// In frontend code
|
||||
const isPending = order.status === 'pending'; // And again
|
||||
|
||||
// Database seeds with duplicated data
|
||||
const seedRoles = [
|
||||
{ id: 1, name: 'admin', permissions: ['read', 'write', 'delete', 'admin'] },
|
||||
{ id: 2, name: 'editor', permissions: ['read', 'write'] },
|
||||
{ id: 3, name: 'viewer', permissions: ['read'] }
|
||||
];
|
||||
|
||||
// In authorization middleware - duplicated permission logic
|
||||
function checkPermission(user: User, action: string): boolean {
|
||||
if (user.role === 'admin') {
|
||||
return true; // Admin can do anything - duplicated knowledge
|
||||
}
|
||||
if (user.role === 'editor' && ['read', 'write'].includes(action)) {
|
||||
return true; // Editor permissions - duplicated
|
||||
}
|
||||
if (user.role === 'viewer' && action === 'read') {
|
||||
return true; // Viewer permissions - duplicated
|
||||
}
|
||||
return false;
|
||||
}
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Single source of truth for all shared knowledge
|
||||
|
||||
// Configuration - one authoritative source
|
||||
// config/index.ts
|
||||
export const Config = {
|
||||
api: {
|
||||
baseUrl: process.env.API_BASE_URL || 'https://api.example.com/v1',
|
||||
timeout: Number(process.env.API_TIMEOUT) || 30000,
|
||||
retries: Number(process.env.API_RETRIES) || 3
|
||||
},
|
||||
database: {
|
||||
url: process.env.DATABASE_URL!,
|
||||
poolSize: Number(process.env.DB_POOL_SIZE) || 10
|
||||
}
|
||||
} as const;
|
||||
|
||||
// All code references the single config
|
||||
class ApiClient {
|
||||
constructor(private baseUrl: string = Config.api.baseUrl) {}
|
||||
}
|
||||
|
||||
// Enums for finite sets of values - single source of truth
|
||||
// domain/order/status.ts
|
||||
export const OrderStatus = {
|
||||
PENDING: 'pending',
|
||||
PROCESSING: 'processing',
|
||||
SHIPPED: 'shipped',
|
||||
DELIVERED: 'delivered',
|
||||
CANCELLED: 'cancelled'
|
||||
} as const;
|
||||
|
||||
export type OrderStatus = typeof OrderStatus[keyof typeof OrderStatus];
|
||||
|
||||
// All code uses the enum
|
||||
class OrderService {
|
||||
async getOrder(id: string): Promise<Order> {
|
||||
const order = await this.repository.findById(id);
|
||||
if (order.status === OrderStatus.PENDING) {
|
||||
// Single source of truth
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
class OrderController {
|
||||
async listPendingOrders(): Promise<Order[]> {
|
||||
return this.repository.findByStatus(OrderStatus.PENDING); // Same source
|
||||
}
|
||||
}
|
||||
|
||||
// Roles and permissions - single authoritative definition
|
||||
// domain/auth/roles.ts
|
||||
export const Permission = {
|
||||
READ: 'read',
|
||||
WRITE: 'write',
|
||||
DELETE: 'delete',
|
||||
ADMIN: 'admin'
|
||||
} as const;
|
||||
|
||||
export type Permission = typeof Permission[keyof typeof Permission];
|
||||
|
||||
export const RoleDefinitions = {
|
||||
admin: {
|
||||
name: 'Administrator',
|
||||
permissions: [Permission.READ, Permission.WRITE, Permission.DELETE, Permission.ADMIN]
|
||||
},
|
||||
editor: {
|
||||
name: 'Editor',
|
||||
permissions: [Permission.READ, Permission.WRITE]
|
||||
},
|
||||
viewer: {
|
||||
name: 'Viewer',
|
||||
permissions: [Permission.READ]
|
||||
}
|
||||
} as const;
|
||||
|
||||
export type RoleName = keyof typeof RoleDefinitions;
|
||||
|
||||
// Permission checking uses the definitions
|
||||
export function hasPermission(role: RoleName, permission: Permission): boolean {
|
||||
const roleDef = RoleDefinitions[role];
|
||||
return roleDef.permissions.includes(permission);
|
||||
}
|
||||
|
||||
// Database seeds generated from the single source
|
||||
export function generateRoleSeeds(): RoleSeed[] {
|
||||
return Object.entries(RoleDefinitions).map(([key, def], index) => ({
|
||||
id: index + 1,
|
||||
name: key,
|
||||
displayName: def.name,
|
||||
permissions: [...def.permissions]
|
||||
}));
|
||||
}
|
||||
|
||||
// Validation rules - single source
|
||||
// domain/user/validation.ts
|
||||
export const UserValidationRules = {
|
||||
email: {
|
||||
maxLength: 255,
|
||||
pattern: /^[^\s@]+@[^\s@]+\.[^\s@]+$/
|
||||
},
|
||||
password: {
|
||||
minLength: 8,
|
||||
maxLength: 128,
|
||||
requireUppercase: true,
|
||||
requireLowercase: true,
|
||||
requireNumber: true,
|
||||
requireSpecial: false
|
||||
},
|
||||
username: {
|
||||
minLength: 3,
|
||||
maxLength: 30,
|
||||
pattern: /^[a-zA-Z0-9_]+$/
|
||||
}
|
||||
} as const;
|
||||
|
||||
// Validators use the rules
|
||||
export function validateEmail(email: string): ValidationResult {
|
||||
const rules = UserValidationRules.email;
|
||||
|
||||
if (email.length > rules.maxLength) {
|
||||
return { valid: false, error: `Email must be ${rules.maxLength} characters or less` };
|
||||
}
|
||||
if (!rules.pattern.test(email)) {
|
||||
return { valid: false, error: 'Invalid email format' };
|
||||
}
|
||||
return { valid: true };
|
||||
}
|
||||
|
||||
// Database schema uses the same rules
|
||||
// migrations/001_create_users.ts
|
||||
export const createUsersTable = `
|
||||
CREATE TABLE users (
|
||||
id UUID PRIMARY KEY,
|
||||
email VARCHAR(${UserValidationRules.email.maxLength}) NOT NULL UNIQUE,
|
||||
username VARCHAR(${UserValidationRules.username.maxLength}) NOT NULL UNIQUE,
|
||||
-- ...
|
||||
)
|
||||
`;
|
||||
|
||||
// Frontend validation uses the same rules (shared module)
|
||||
// shared/validation.ts (used by both frontend and backend)
|
||||
export function getPasswordRequirements(): string[] {
|
||||
const rules = UserValidationRules.password;
|
||||
const requirements: string[] = [];
|
||||
|
||||
requirements.push(`At least ${rules.minLength} characters`);
|
||||
if (rules.requireUppercase) requirements.push('At least one uppercase letter');
|
||||
if (rules.requireLowercase) requirements.push('At least one lowercase letter');
|
||||
if (rules.requireNumber) requirements.push('At least one number');
|
||||
if (rules.requireSpecial) requirements.push('At least one special character');
|
||||
|
||||
return requirements;
|
||||
}
|
||||
|
||||
// Error messages - single source
|
||||
// errors/messages.ts
|
||||
export const ErrorMessages = {
|
||||
user: {
|
||||
notFound: 'User not found',
|
||||
alreadyExists: 'A user with this email already exists',
|
||||
invalidCredentials: 'Invalid email or password',
|
||||
accountLocked: 'Account is locked. Please contact support.'
|
||||
},
|
||||
order: {
|
||||
notFound: 'Order not found',
|
||||
alreadyCancelled: 'Order has already been cancelled',
|
||||
cannotCancel: 'Order cannot be cancelled in its current state'
|
||||
},
|
||||
auth: {
|
||||
tokenExpired: 'Your session has expired. Please log in again.',
|
||||
unauthorized: 'You do not have permission to perform this action'
|
||||
}
|
||||
} as const;
|
||||
|
||||
// All code uses the same error messages
|
||||
class UserService {
|
||||
async findById(id: string): Promise<User> {
|
||||
const user = await this.repository.findById(id);
|
||||
if (!user) {
|
||||
throw new NotFoundError(ErrorMessages.user.notFound);
|
||||
}
|
||||
return user;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Consistency**: The same value is always the same everywhere. No "this worked yesterday" bugs.
|
||||
|
||||
2. **Easy Updates**: Change a validation rule, config value, or status code in one place.
|
||||
|
||||
3. **Prevents Drift**: Without a single source, values diverge over time as different developers make changes.
|
||||
|
||||
4. **Documentation**: The source file serves as documentation for what values are valid.
|
||||
|
||||
5. **Type Safety**: TypeScript can enforce that only valid values are used.
|
||||
|
||||
6. **Searchability**: Easy to find all usages by searching for the constant name.
|
||||
|
||||
7. **Refactoring**: Rename a status? Change it in one place and let the compiler find all usages.
|
||||
320
.agents/skills/clean-code-principles/rules/core-dry.md
Normal file
320
.agents/skills/clean-code-principles/rules/core-dry.md
Normal file
@@ -0,0 +1,320 @@
|
||||
---
|
||||
id: core-dry
|
||||
title: Don't Repeat Yourself (DRY)
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [DRY, duplication, single-source-of-truth, maintainability]
|
||||
related: [core-dry-extraction, core-dry-single-source, solid-srp-class]
|
||||
---
|
||||
|
||||
# Don't Repeat Yourself (DRY)
|
||||
|
||||
## Why It Matters
|
||||
|
||||
"Don't Repeat Yourself" means every piece of knowledge should have a single, authoritative representation. Duplication leads to inconsistencies, increases maintenance burden, and makes bugs harder to fix. When you change one copy but forget others, bugs creep in.
|
||||
|
||||
## Incorrect
|
||||
|
||||
```typescript
|
||||
// ❌ Duplicated validation logic
|
||||
class UserController {
|
||||
createUser(data) {
|
||||
if (!data.email || !data.email.includes('@')) {
|
||||
throw new Error('Invalid email');
|
||||
}
|
||||
if (!data.password || data.password.length < 8) {
|
||||
throw new Error('Password too short');
|
||||
}
|
||||
// create user...
|
||||
}
|
||||
|
||||
updateUser(id, data) {
|
||||
if (!data.email || !data.email.includes('@')) { // Duplicated
|
||||
throw new Error('Invalid email');
|
||||
}
|
||||
if (!data.password || data.password.length < 8) { // Duplicated
|
||||
throw new Error('Password too short');
|
||||
}
|
||||
// update user...
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ Duplicated business rules
|
||||
function calculateOrderTotal(items) {
|
||||
let total = 0;
|
||||
for (const item of items) {
|
||||
total += item.price * item.quantity;
|
||||
}
|
||||
if (total > 100) {
|
||||
total = total * 0.9; // 10% discount over $100
|
||||
}
|
||||
return total;
|
||||
}
|
||||
|
||||
function calculateCartTotal(cartItems) {
|
||||
let total = 0;
|
||||
for (const item of cartItems) {
|
||||
total += item.price * item.quantity; // Duplicated
|
||||
}
|
||||
if (total > 100) {
|
||||
total = total * 0.9; // Duplicated discount logic
|
||||
}
|
||||
return total;
|
||||
}
|
||||
|
||||
// ❌ Duplicated constants
|
||||
// file: checkout.ts
|
||||
const TAX_RATE = 0.08;
|
||||
const FREE_SHIPPING_THRESHOLD = 50;
|
||||
|
||||
// file: cart.ts
|
||||
const TAX_RATE = 0.08; // Duplicated
|
||||
const FREE_SHIPPING_THRESHOLD = 50; // Duplicated
|
||||
```
|
||||
|
||||
## Correct
|
||||
|
||||
### Extract Shared Validation
|
||||
|
||||
```typescript
|
||||
// ✅ Single validation module
|
||||
// validators/user.ts
|
||||
export class UserValidator {
|
||||
static validateEmail(email: string): void {
|
||||
if (!email || !email.includes('@')) {
|
||||
throw new ValidationError('Invalid email address');
|
||||
}
|
||||
}
|
||||
|
||||
static validatePassword(password: string): void {
|
||||
if (!password || password.length < 8) {
|
||||
throw new ValidationError('Password must be at least 8 characters');
|
||||
}
|
||||
}
|
||||
|
||||
static validate(data: UserData): void {
|
||||
this.validateEmail(data.email);
|
||||
this.validatePassword(data.password);
|
||||
}
|
||||
}
|
||||
|
||||
// controller.ts
|
||||
class UserController {
|
||||
createUser(data) {
|
||||
UserValidator.validate(data);
|
||||
// create user...
|
||||
}
|
||||
|
||||
updateUser(id, data) {
|
||||
UserValidator.validate(data);
|
||||
// update user...
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Extract Shared Business Logic
|
||||
|
||||
```typescript
|
||||
// ✅ Single source of truth for pricing
|
||||
// services/pricing.ts
|
||||
export class PricingService {
|
||||
private static readonly BULK_DISCOUNT_THRESHOLD = 100;
|
||||
private static readonly BULK_DISCOUNT_RATE = 0.1;
|
||||
|
||||
static calculateSubtotal(items: LineItem[]): number {
|
||||
return items.reduce(
|
||||
(sum, item) => sum + item.price * item.quantity,
|
||||
0
|
||||
);
|
||||
}
|
||||
|
||||
static applyDiscount(subtotal: number): number {
|
||||
if (subtotal > this.BULK_DISCOUNT_THRESHOLD) {
|
||||
return subtotal * (1 - this.BULK_DISCOUNT_RATE);
|
||||
}
|
||||
return subtotal;
|
||||
}
|
||||
|
||||
static calculateTotal(items: LineItem[]): number {
|
||||
const subtotal = this.calculateSubtotal(items);
|
||||
return this.applyDiscount(subtotal);
|
||||
}
|
||||
}
|
||||
|
||||
// Both order and cart use the same logic
|
||||
const orderTotal = PricingService.calculateTotal(order.items);
|
||||
const cartTotal = PricingService.calculateTotal(cart.items);
|
||||
```
|
||||
|
||||
### Centralize Constants
|
||||
|
||||
```typescript
|
||||
// ✅ Single constants file
|
||||
// constants/pricing.ts
|
||||
export const PRICING = {
|
||||
TAX_RATE: 0.08,
|
||||
FREE_SHIPPING_THRESHOLD: 50,
|
||||
BULK_DISCOUNT_THRESHOLD: 100,
|
||||
BULK_DISCOUNT_RATE: 0.1,
|
||||
} as const;
|
||||
|
||||
// Used everywhere
|
||||
import { PRICING } from '@/constants/pricing';
|
||||
|
||||
const tax = subtotal * PRICING.TAX_RATE;
|
||||
const freeShipping = total >= PRICING.FREE_SHIPPING_THRESHOLD;
|
||||
```
|
||||
|
||||
### Extract Shared Components
|
||||
|
||||
```tsx
|
||||
// ❌ Duplicated UI patterns
|
||||
function UserCard({ user }) {
|
||||
return (
|
||||
<div className="p-4 rounded-lg shadow bg-white">
|
||||
<img src={user.avatar} className="w-12 h-12 rounded-full" />
|
||||
<h3 className="font-bold">{user.name}</h3>
|
||||
<p className="text-gray-600">{user.email}</p>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function TeamMemberCard({ member }) {
|
||||
return (
|
||||
<div className="p-4 rounded-lg shadow bg-white"> {/* Same styles */}
|
||||
<img src={member.avatar} className="w-12 h-12 rounded-full" />
|
||||
<h3 className="font-bold">{member.name}</h3>
|
||||
<p className="text-gray-600">{member.role}</p>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// ✅ Reusable component
|
||||
function Card({ children, className }) {
|
||||
return (
|
||||
<div className={cn("p-4 rounded-lg shadow bg-white", className)}>
|
||||
{children}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function Avatar({ src, alt }) {
|
||||
return <img src={src} alt={alt} className="w-12 h-12 rounded-full" />;
|
||||
}
|
||||
|
||||
function UserCard({ user }) {
|
||||
return (
|
||||
<Card>
|
||||
<Avatar src={user.avatar} alt={user.name} />
|
||||
<h3 className="font-bold">{user.name}</h3>
|
||||
<p className="text-gray-600">{user.email}</p>
|
||||
</Card>
|
||||
);
|
||||
}
|
||||
|
||||
function TeamMemberCard({ member }) {
|
||||
return (
|
||||
<Card>
|
||||
<Avatar src={member.avatar} alt={member.name} />
|
||||
<h3 className="font-bold">{member.name}</h3>
|
||||
<p className="text-gray-600">{member.role}</p>
|
||||
</Card>
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
### Extract Shared Hooks
|
||||
|
||||
```typescript
|
||||
// ❌ Duplicated fetch logic
|
||||
function UserProfile() {
|
||||
const [user, setUser] = useState(null);
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [error, setError] = useState(null);
|
||||
|
||||
useEffect(() => {
|
||||
fetch('/api/user')
|
||||
.then(res => res.json())
|
||||
.then(setUser)
|
||||
.catch(setError)
|
||||
.finally(() => setLoading(false));
|
||||
}, []);
|
||||
// ...
|
||||
}
|
||||
|
||||
function ProductList() {
|
||||
const [products, setProducts] = useState(null);
|
||||
const [loading, setLoading] = useState(true); // Duplicated
|
||||
const [error, setError] = useState(null); // Duplicated
|
||||
|
||||
useEffect(() => {
|
||||
fetch('/api/products') // Same pattern
|
||||
.then(res => res.json())
|
||||
.then(setProducts)
|
||||
.catch(setError)
|
||||
.finally(() => setLoading(false));
|
||||
}, []);
|
||||
// ...
|
||||
}
|
||||
|
||||
// ✅ Custom hook (or use React Query)
|
||||
function useFetch<T>(url: string) {
|
||||
const [data, setData] = useState<T | null>(null);
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [error, setError] = useState<Error | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
fetch(url)
|
||||
.then(res => res.json())
|
||||
.then(setData)
|
||||
.catch(setError)
|
||||
.finally(() => setLoading(false));
|
||||
}, [url]);
|
||||
|
||||
return { data, loading, error };
|
||||
}
|
||||
|
||||
function UserProfile() {
|
||||
const { data: user, loading, error } = useFetch<User>('/api/user');
|
||||
// ...
|
||||
}
|
||||
|
||||
function ProductList() {
|
||||
const { data: products, loading, error } = useFetch<Product[]>('/api/products');
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
## When NOT to DRY
|
||||
|
||||
```typescript
|
||||
// ⚠️ Don't extract coincidentally similar code
|
||||
// These might look similar but serve different purposes
|
||||
|
||||
function validateUserAge(age: number) {
|
||||
return age >= 18; // Legal adult age
|
||||
}
|
||||
|
||||
function validateMinimumOrderQuantity(quantity: number) {
|
||||
return quantity >= 18; // Business rule: minimum order
|
||||
}
|
||||
|
||||
// These should remain separate even though both check >= 18
|
||||
// Their reasons for change are different
|
||||
```
|
||||
|
||||
## Rule of Three
|
||||
|
||||
```
|
||||
Wait until you see duplication three times before extracting.
|
||||
Two occurrences might be coincidental. Three indicates a pattern.
|
||||
```
|
||||
|
||||
## Benefits
|
||||
|
||||
- Single source of truth
|
||||
- Fix bugs in one place
|
||||
- Consistent behavior across codebase
|
||||
- Easier refactoring
|
||||
- Reduced code size
|
||||
- Lower maintenance cost
|
||||
335
.agents/skills/clean-code-principles/rules/core-encapsulation.md
Normal file
335
.agents/skills/clean-code-principles/rules/core-encapsulation.md
Normal file
@@ -0,0 +1,335 @@
|
||||
---
|
||||
id: core-encapsulation
|
||||
title: Encapsulation
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [encapsulation, information-hiding, data-protection]
|
||||
related: [solid-srp-class, core-law-demeter, solid-isp-interfaces]
|
||||
---
|
||||
|
||||
# Encapsulation
|
||||
|
||||
Hide internal implementation details and expose only what's necessary through a well-defined interface. Protect data integrity by controlling access to internal state.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Exposed internals, no encapsulation
|
||||
|
||||
class BankAccount {
|
||||
// Public fields - anyone can modify directly
|
||||
public accountNumber: string;
|
||||
public balance: number;
|
||||
public transactions: Transaction[];
|
||||
public overdraftLimit: number;
|
||||
public isLocked: boolean;
|
||||
|
||||
constructor(accountNumber: string, initialBalance: number) {
|
||||
this.accountNumber = accountNumber;
|
||||
this.balance = initialBalance;
|
||||
this.transactions = [];
|
||||
this.overdraftLimit = 0;
|
||||
this.isLocked = false;
|
||||
}
|
||||
}
|
||||
|
||||
// External code can violate business rules
|
||||
const account = new BankAccount('12345', 1000);
|
||||
|
||||
// Direct modification bypasses validation
|
||||
account.balance = -999999; // Negative balance without check!
|
||||
account.balance = account.balance + 1000; // No transaction record!
|
||||
|
||||
// Can manipulate transaction history
|
||||
account.transactions.push({
|
||||
id: 'fake',
|
||||
amount: 1000000,
|
||||
type: 'deposit'
|
||||
}); // Fraudulent transaction!
|
||||
|
||||
// Can unlock locked accounts
|
||||
account.isLocked = false; // Bypasses security!
|
||||
|
||||
// Can change overdraft without authorization
|
||||
account.overdraftLimit = 100000; // Unauthorized overdraft!
|
||||
|
||||
// Other classes depend on internal structure
|
||||
class AccountReport {
|
||||
generate(account: BankAccount): Report {
|
||||
// Directly accesses internal array
|
||||
const deposits = account.transactions.filter(t => t.type === 'deposit');
|
||||
const withdrawals = account.transactions.filter(t => t.type === 'withdrawal');
|
||||
|
||||
// Depends on internal structure of Transaction
|
||||
const totalDeposits = deposits.reduce((sum, t) => sum + t.amount, 0);
|
||||
|
||||
return {
|
||||
balance: account.balance,
|
||||
totalDeposits,
|
||||
transactionCount: account.transactions.length
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// Problems:
|
||||
// 1. Anyone can modify balance without recording transaction
|
||||
// 2. Business rules can be bypassed
|
||||
// 3. No audit trail for changes
|
||||
// 4. Internal structure changes break external code
|
||||
// 5. No way to add validation later without breaking changes
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Proper encapsulation
|
||||
|
||||
class BankAccount {
|
||||
private readonly _accountNumber: string;
|
||||
private _balance: number;
|
||||
private readonly _transactions: Transaction[] = [];
|
||||
private _overdraftLimit: number = 0;
|
||||
private _isLocked: boolean = false;
|
||||
private _lockReason: string | null = null;
|
||||
|
||||
constructor(accountNumber: string, initialBalance: number) {
|
||||
if (!accountNumber || accountNumber.length < 5) {
|
||||
throw new Error('Invalid account number');
|
||||
}
|
||||
if (initialBalance < 0) {
|
||||
throw new Error('Initial balance cannot be negative');
|
||||
}
|
||||
|
||||
this._accountNumber = accountNumber;
|
||||
this._balance = initialBalance;
|
||||
this._transactions.push(
|
||||
Transaction.createInitial(initialBalance)
|
||||
);
|
||||
}
|
||||
|
||||
// Read-only access to account number
|
||||
get accountNumber(): string {
|
||||
return this._accountNumber;
|
||||
}
|
||||
|
||||
// Read-only access to balance
|
||||
get balance(): number {
|
||||
return this._balance;
|
||||
}
|
||||
|
||||
// Read-only access to lock status
|
||||
get isLocked(): boolean {
|
||||
return this._isLocked;
|
||||
}
|
||||
|
||||
// Controlled deposit with validation and audit trail
|
||||
deposit(amount: number, description: string = 'Deposit'): Transaction {
|
||||
this.ensureNotLocked();
|
||||
|
||||
if (amount <= 0) {
|
||||
throw new InvalidAmountError('Deposit amount must be positive');
|
||||
}
|
||||
|
||||
this._balance += amount;
|
||||
|
||||
const transaction = Transaction.createDeposit(amount, description, this._balance);
|
||||
this._transactions.push(transaction);
|
||||
|
||||
return transaction;
|
||||
}
|
||||
|
||||
// Controlled withdrawal with business rules
|
||||
withdraw(amount: number, description: string = 'Withdrawal'): Transaction {
|
||||
this.ensureNotLocked();
|
||||
|
||||
if (amount <= 0) {
|
||||
throw new InvalidAmountError('Withdrawal amount must be positive');
|
||||
}
|
||||
|
||||
const availableBalance = this._balance + this._overdraftLimit;
|
||||
if (amount > availableBalance) {
|
||||
throw new InsufficientFundsError(amount, availableBalance);
|
||||
}
|
||||
|
||||
this._balance -= amount;
|
||||
|
||||
const transaction = Transaction.createWithdrawal(amount, description, this._balance);
|
||||
this._transactions.push(transaction);
|
||||
|
||||
return transaction;
|
||||
}
|
||||
|
||||
// Transfer with proper validation
|
||||
transferTo(recipient: BankAccount, amount: number): TransferResult {
|
||||
this.ensureNotLocked();
|
||||
recipient.ensureNotLocked();
|
||||
|
||||
if (amount <= 0) {
|
||||
throw new InvalidAmountError('Transfer amount must be positive');
|
||||
}
|
||||
|
||||
const withdrawalTx = this.withdraw(amount, `Transfer to ${recipient.accountNumber}`);
|
||||
const depositTx = recipient.deposit(amount, `Transfer from ${this._accountNumber}`);
|
||||
|
||||
return { withdrawalTx, depositTx };
|
||||
}
|
||||
|
||||
// Controlled overdraft limit modification
|
||||
setOverdraftLimit(limit: number, authorizedBy: string): void {
|
||||
if (limit < 0) {
|
||||
throw new Error('Overdraft limit cannot be negative');
|
||||
}
|
||||
if (limit > 10000) {
|
||||
throw new Error('Overdraft limit exceeds maximum allowed');
|
||||
}
|
||||
|
||||
this._overdraftLimit = limit;
|
||||
|
||||
// Audit trail for limit changes
|
||||
this._transactions.push(
|
||||
Transaction.createAdministrative(
|
||||
`Overdraft limit set to ${limit} by ${authorizedBy}`
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
// Security control with audit
|
||||
lock(reason: string): void {
|
||||
this._isLocked = true;
|
||||
this._lockReason = reason;
|
||||
this._transactions.push(
|
||||
Transaction.createAdministrative(`Account locked: ${reason}`)
|
||||
);
|
||||
}
|
||||
|
||||
unlock(authorizedBy: string): void {
|
||||
this._isLocked = false;
|
||||
this._lockReason = null;
|
||||
this._transactions.push(
|
||||
Transaction.createAdministrative(`Account unlocked by ${authorizedBy}`)
|
||||
);
|
||||
}
|
||||
|
||||
// Return copy of transactions, not the internal array
|
||||
getTransactionHistory(): ReadonlyArray<Transaction> {
|
||||
return [...this._transactions];
|
||||
}
|
||||
|
||||
// Provide summary without exposing internals
|
||||
getSummary(): AccountSummary {
|
||||
const deposits = this._transactions.filter(t => t.type === 'deposit');
|
||||
const withdrawals = this._transactions.filter(t => t.type === 'withdrawal');
|
||||
|
||||
return {
|
||||
accountNumber: this._accountNumber,
|
||||
balance: this._balance,
|
||||
isLocked: this._isLocked,
|
||||
totalDeposits: deposits.reduce((sum, t) => sum + t.amount, 0),
|
||||
totalWithdrawals: withdrawals.reduce((sum, t) => sum + t.amount, 0),
|
||||
transactionCount: this._transactions.length
|
||||
};
|
||||
}
|
||||
|
||||
// Private helper method
|
||||
private ensureNotLocked(): void {
|
||||
if (this._isLocked) {
|
||||
throw new AccountLockedError(this._lockReason || 'Account is locked');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Transaction is also encapsulated
|
||||
class Transaction {
|
||||
private constructor(
|
||||
public readonly id: string,
|
||||
public readonly type: TransactionType,
|
||||
public readonly amount: number,
|
||||
public readonly description: string,
|
||||
public readonly balanceAfter: number,
|
||||
public readonly timestamp: Date
|
||||
) {}
|
||||
|
||||
static createDeposit(amount: number, description: string, balanceAfter: number): Transaction {
|
||||
return new Transaction(
|
||||
generateId(),
|
||||
'deposit',
|
||||
amount,
|
||||
description,
|
||||
balanceAfter,
|
||||
new Date()
|
||||
);
|
||||
}
|
||||
|
||||
static createWithdrawal(amount: number, description: string, balanceAfter: number): Transaction {
|
||||
return new Transaction(
|
||||
generateId(),
|
||||
'withdrawal',
|
||||
amount,
|
||||
description,
|
||||
balanceAfter,
|
||||
new Date()
|
||||
);
|
||||
}
|
||||
|
||||
static createInitial(balance: number): Transaction {
|
||||
return new Transaction(
|
||||
generateId(),
|
||||
'initial',
|
||||
balance,
|
||||
'Account opened',
|
||||
balance,
|
||||
new Date()
|
||||
);
|
||||
}
|
||||
|
||||
static createAdministrative(description: string): Transaction {
|
||||
return new Transaction(
|
||||
generateId(),
|
||||
'administrative',
|
||||
0,
|
||||
description,
|
||||
0,
|
||||
new Date()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Usage - business rules are enforced
|
||||
const account = new BankAccount('12345', 1000);
|
||||
|
||||
// Proper deposit with audit trail
|
||||
account.deposit(500, 'Paycheck');
|
||||
|
||||
// Withdrawal validates funds
|
||||
try {
|
||||
account.withdraw(2000); // Will throw InsufficientFundsError
|
||||
} catch (error) {
|
||||
console.log('Cannot withdraw more than available');
|
||||
}
|
||||
|
||||
// Cannot manipulate balance directly
|
||||
// account.balance = 999999; // Error: Property 'balance' is read-only
|
||||
|
||||
// Cannot manipulate transactions
|
||||
// account.getTransactionHistory().push(fakeTx); // Original array unaffected
|
||||
|
||||
// Account summary provides what reports need
|
||||
const summary = account.getSummary();
|
||||
console.log(`Balance: ${summary.balance}, Transactions: ${summary.transactionCount}`);
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Data Integrity**: Balance can only change through proper deposit/withdraw methods that maintain consistency.
|
||||
|
||||
2. **Business Rules**: All rules (positive amounts, sufficient funds, locked accounts) are enforced in one place.
|
||||
|
||||
3. **Audit Trail**: Every change is recorded. Cannot modify history without proper methods.
|
||||
|
||||
4. **Flexibility**: Internal representation can change without affecting clients. Switch from array to database later.
|
||||
|
||||
5. **Security**: Cannot bypass validation or manipulate internal state directly.
|
||||
|
||||
6. **Testing**: Can verify all edge cases through the public interface. Internal state is controlled.
|
||||
|
||||
7. **Documentation**: The public interface documents what operations are allowed and how to use them.
|
||||
325
.agents/skills/clean-code-principles/rules/core-fail-fast.md
Normal file
325
.agents/skills/clean-code-principles/rules/core-fail-fast.md
Normal file
@@ -0,0 +1,325 @@
|
||||
---
|
||||
id: core-fail-fast
|
||||
title: Fail Fast Principle
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [fail-fast, error-handling, validation]
|
||||
related: [solid-lsp-preconditions, core-encapsulation]
|
||||
---
|
||||
|
||||
# Fail Fast Principle
|
||||
|
||||
Detect and report errors as early as possible. Validate inputs at system boundaries, check preconditions at the start of functions, and throw exceptions immediately when something is wrong.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Delayed error detection
|
||||
|
||||
class OrderProcessor {
|
||||
async processOrder(order: any): Promise<ProcessResult> {
|
||||
// No validation - problems will surface later
|
||||
|
||||
// Proceeds even with potentially invalid data
|
||||
const customer = await this.customerRepo.findById(order.customerId);
|
||||
|
||||
// Customer might be null, but we keep going
|
||||
const items = order.items;
|
||||
|
||||
// Calculates total even if items might be undefined or empty
|
||||
let total = 0;
|
||||
if (items) {
|
||||
for (const item of items) {
|
||||
// item.productId might not exist
|
||||
const product = await this.productRepo.findById(item.productId);
|
||||
// product might be null
|
||||
if (product) {
|
||||
total += product.price * (item.quantity || 1);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Attempts payment even if customer is null
|
||||
let paymentResult;
|
||||
if (customer && customer.paymentMethod) {
|
||||
paymentResult = await this.paymentService.charge(
|
||||
customer.paymentMethod,
|
||||
total
|
||||
);
|
||||
}
|
||||
|
||||
// Creates order record even if payment failed
|
||||
const orderRecord = await this.orderRepo.create({
|
||||
customerId: order.customerId,
|
||||
total,
|
||||
status: paymentResult?.success ? 'paid' : 'pending'
|
||||
});
|
||||
|
||||
// Sends email even if we don't have a valid email
|
||||
if (customer?.email) {
|
||||
await this.emailService.send(customer.email, 'Order confirmation');
|
||||
}
|
||||
|
||||
// Returns "success" even though many things might have gone wrong
|
||||
return { success: true, orderId: orderRecord.id };
|
||||
}
|
||||
}
|
||||
|
||||
// Problems:
|
||||
// 1. Null customer leads to silent failures
|
||||
// 2. Empty order goes through system doing nothing useful
|
||||
// 3. Payment failure creates orphaned order records
|
||||
// 4. No indication of what went wrong
|
||||
// 5. Database in inconsistent state
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Fail fast with immediate validation
|
||||
|
||||
// Custom error types for clear failure reasons
|
||||
class ValidationError extends Error {
|
||||
constructor(
|
||||
message: string,
|
||||
public readonly field: string,
|
||||
public readonly value: unknown
|
||||
) {
|
||||
super(message);
|
||||
this.name = 'ValidationError';
|
||||
}
|
||||
}
|
||||
|
||||
class NotFoundError extends Error {
|
||||
constructor(
|
||||
public readonly entity: string,
|
||||
public readonly id: string
|
||||
) {
|
||||
super(`${entity} not found: ${id}`);
|
||||
this.name = 'NotFoundError';
|
||||
}
|
||||
}
|
||||
|
||||
class PaymentError extends Error {
|
||||
constructor(
|
||||
message: string,
|
||||
public readonly code: string
|
||||
) {
|
||||
super(message);
|
||||
this.name = 'PaymentError';
|
||||
}
|
||||
}
|
||||
|
||||
// Input validation schema
|
||||
interface CreateOrderInput {
|
||||
customerId: string;
|
||||
items: OrderItemInput[];
|
||||
}
|
||||
|
||||
interface OrderItemInput {
|
||||
productId: string;
|
||||
quantity: number;
|
||||
}
|
||||
|
||||
class OrderProcessor {
|
||||
async processOrder(input: unknown): Promise<ProcessResult> {
|
||||
// STEP 1: Validate input immediately
|
||||
const validatedInput = this.validateInput(input);
|
||||
|
||||
// STEP 2: Verify customer exists before proceeding
|
||||
const customer = await this.loadCustomer(validatedInput.customerId);
|
||||
|
||||
// STEP 3: Verify all products exist before any processing
|
||||
const products = await this.loadProducts(validatedInput.items);
|
||||
|
||||
// STEP 4: Verify payment method before creating order
|
||||
this.verifyPaymentMethod(customer);
|
||||
|
||||
// STEP 5: Calculate total (now safe - all data validated)
|
||||
const total = this.calculateTotal(validatedInput.items, products);
|
||||
|
||||
// STEP 6: Verify sufficient inventory before payment
|
||||
await this.verifyInventory(validatedInput.items, products);
|
||||
|
||||
// STEP 7: Process payment before creating order
|
||||
const payment = await this.processPayment(customer, total);
|
||||
|
||||
// STEP 8: Create order only after successful payment
|
||||
const order = await this.createOrder(validatedInput, customer, total, payment);
|
||||
|
||||
// STEP 9: Send confirmation (non-critical, can fail gracefully)
|
||||
await this.sendConfirmation(customer, order);
|
||||
|
||||
return { success: true, orderId: order.id };
|
||||
}
|
||||
|
||||
private validateInput(input: unknown): CreateOrderInput {
|
||||
if (!input || typeof input !== 'object') {
|
||||
throw new ValidationError('Input must be an object', 'input', input);
|
||||
}
|
||||
|
||||
const obj = input as Record<string, unknown>;
|
||||
|
||||
if (!obj.customerId || typeof obj.customerId !== 'string') {
|
||||
throw new ValidationError('customerId is required and must be a string', 'customerId', obj.customerId);
|
||||
}
|
||||
|
||||
if (!Array.isArray(obj.items) || obj.items.length === 0) {
|
||||
throw new ValidationError('items must be a non-empty array', 'items', obj.items);
|
||||
}
|
||||
|
||||
const validatedItems: OrderItemInput[] = [];
|
||||
for (let i = 0; i < obj.items.length; i++) {
|
||||
const item = obj.items[i];
|
||||
if (!item || typeof item !== 'object') {
|
||||
throw new ValidationError(`Item at index ${i} must be an object`, `items[${i}]`, item);
|
||||
}
|
||||
|
||||
const itemObj = item as Record<string, unknown>;
|
||||
|
||||
if (!itemObj.productId || typeof itemObj.productId !== 'string') {
|
||||
throw new ValidationError(
|
||||
`productId is required at index ${i}`,
|
||||
`items[${i}].productId`,
|
||||
itemObj.productId
|
||||
);
|
||||
}
|
||||
|
||||
if (typeof itemObj.quantity !== 'number' || itemObj.quantity < 1) {
|
||||
throw new ValidationError(
|
||||
`quantity must be a positive number at index ${i}`,
|
||||
`items[${i}].quantity`,
|
||||
itemObj.quantity
|
||||
);
|
||||
}
|
||||
|
||||
validatedItems.push({
|
||||
productId: itemObj.productId,
|
||||
quantity: itemObj.quantity
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
customerId: obj.customerId,
|
||||
items: validatedItems
|
||||
};
|
||||
}
|
||||
|
||||
private async loadCustomer(customerId: string): Promise<Customer> {
|
||||
const customer = await this.customerRepo.findById(customerId);
|
||||
|
||||
if (!customer) {
|
||||
throw new NotFoundError('Customer', customerId);
|
||||
}
|
||||
|
||||
if (!customer.isActive) {
|
||||
throw new ValidationError('Customer account is inactive', 'customerId', customerId);
|
||||
}
|
||||
|
||||
return customer;
|
||||
}
|
||||
|
||||
private async loadProducts(items: OrderItemInput[]): Promise<Map<string, Product>> {
|
||||
const productIds = items.map(item => item.productId);
|
||||
const products = await this.productRepo.findByIds(productIds);
|
||||
|
||||
const productMap = new Map<string, Product>();
|
||||
for (const product of products) {
|
||||
productMap.set(product.id, product);
|
||||
}
|
||||
|
||||
// Verify all products were found
|
||||
for (const item of items) {
|
||||
if (!productMap.has(item.productId)) {
|
||||
throw new NotFoundError('Product', item.productId);
|
||||
}
|
||||
}
|
||||
|
||||
return productMap;
|
||||
}
|
||||
|
||||
private verifyPaymentMethod(customer: Customer): void {
|
||||
if (!customer.paymentMethodId) {
|
||||
throw new ValidationError(
|
||||
'Customer has no payment method configured',
|
||||
'paymentMethod',
|
||||
null
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
private calculateTotal(items: OrderItemInput[], products: Map<string, Product>): number {
|
||||
return items.reduce((total, item) => {
|
||||
const product = products.get(item.productId)!; // Safe - already validated
|
||||
return total + product.price * item.quantity;
|
||||
}, 0);
|
||||
}
|
||||
|
||||
private async verifyInventory(
|
||||
items: OrderItemInput[],
|
||||
products: Map<string, Product>
|
||||
): Promise<void> {
|
||||
for (const item of items) {
|
||||
const product = products.get(item.productId)!;
|
||||
if (product.stock < item.quantity) {
|
||||
throw new ValidationError(
|
||||
`Insufficient stock for product ${product.name}. Available: ${product.stock}, Requested: ${item.quantity}`,
|
||||
'quantity',
|
||||
item.quantity
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private async processPayment(customer: Customer, amount: number): Promise<Payment> {
|
||||
try {
|
||||
return await this.paymentService.charge(customer.paymentMethodId, amount);
|
||||
} catch (error) {
|
||||
throw new PaymentError(
|
||||
`Payment failed: ${error.message}`,
|
||||
error.code || 'UNKNOWN'
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
private async createOrder(
|
||||
input: CreateOrderInput,
|
||||
customer: Customer,
|
||||
total: number,
|
||||
payment: Payment
|
||||
): Promise<Order> {
|
||||
return this.orderRepo.create({
|
||||
customerId: customer.id,
|
||||
items: input.items,
|
||||
total,
|
||||
paymentId: payment.id,
|
||||
status: 'paid'
|
||||
});
|
||||
}
|
||||
|
||||
private async sendConfirmation(customer: Customer, order: Order): Promise<void> {
|
||||
try {
|
||||
await this.emailService.sendOrderConfirmation(customer.email, order);
|
||||
} catch (error) {
|
||||
// Log but don't fail - email is non-critical
|
||||
this.logger.error('Failed to send order confirmation', { orderId: order.id, error });
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Clear Error Messages**: When validation fails, you know exactly what's wrong and where.
|
||||
|
||||
2. **No Wasted Work**: Invalid requests fail immediately, not after expensive operations.
|
||||
|
||||
3. **Data Integrity**: The database never enters an inconsistent state because we validate before mutating.
|
||||
|
||||
4. **Debugging**: Stack traces point to the actual problem, not to a downstream symptom.
|
||||
|
||||
5. **Predictability**: Either the operation fully succeeds or it cleanly fails with a clear reason.
|
||||
|
||||
6. **Security**: Invalid inputs are rejected at the boundary, not passed through the system.
|
||||
|
||||
7. **Recovery**: Callers can handle specific error types appropriately (retry, report, fallback).
|
||||
@@ -0,0 +1,202 @@
|
||||
---
|
||||
id: core-kiss-readability
|
||||
title: KISS - Readability
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [KISS, readability, clear-code, maintainability]
|
||||
related: [core-kiss-simplicity, solid-srp-function]
|
||||
---
|
||||
|
||||
# KISS - Readability
|
||||
|
||||
Code is read far more often than it is written. Optimize for readability by using clear names, straightforward logic, and avoiding clever tricks that obscure intent.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Clever but cryptic code
|
||||
|
||||
// One-liner that's hard to understand
|
||||
const r = d.filter(x => x.s === 'a' && x.t > Date.now() - 864e5).reduce((a, x) => ({ ...a, [x.c]: (a[x.c] || 0) + x.v }), {});
|
||||
|
||||
// Nested ternaries
|
||||
const status = x > 100 ? 'high' : x > 50 ? 'medium' : x > 20 ? 'low' : x > 0 ? 'minimal' : 'none';
|
||||
|
||||
// Bitwise operations for boolean logic
|
||||
const isValid = !!(flags & 0x1) && !!(flags & 0x2) || !!(flags & 0x4);
|
||||
|
||||
// Regex that nobody can read
|
||||
const isValidEmail = /^(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])$/i.test(email);
|
||||
|
||||
// Clever use of || and && for control flow
|
||||
user && user.isActive && user.hasPermission('admin') && doAdminStuff() || showError();
|
||||
|
||||
// Abusing array methods
|
||||
const result = [...Array(10)].map((_, i) => i * 2).filter(Boolean).reduce((a, b) => a + b, 0);
|
||||
|
||||
// Short variable names that save typing but cost understanding
|
||||
function p(d, o) {
|
||||
return d.map(i => ({ ...i, t: i.t * o.r, s: o.s ? i.s + o.s : i.s })).filter(i => i.t > o.m);
|
||||
}
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Clear, self-documenting code
|
||||
|
||||
// Break complex operations into named steps
|
||||
function getActiveOrdersSummaryByCategory(orders: Order[]): Record<string, number> {
|
||||
const oneDayAgo = Date.now() - 24 * 60 * 60 * 1000;
|
||||
|
||||
const activeRecentOrders = orders.filter(order =>
|
||||
order.status === 'active' && order.timestamp > oneDayAgo
|
||||
);
|
||||
|
||||
const summaryByCategory: Record<string, number> = {};
|
||||
|
||||
for (const order of activeRecentOrders) {
|
||||
const currentTotal = summaryByCategory[order.category] || 0;
|
||||
summaryByCategory[order.category] = currentTotal + order.value;
|
||||
}
|
||||
|
||||
return summaryByCategory;
|
||||
}
|
||||
|
||||
// Use clear conditional logic
|
||||
function getAlertLevel(value: number): AlertLevel {
|
||||
if (value > 100) {
|
||||
return 'high';
|
||||
}
|
||||
if (value > 50) {
|
||||
return 'medium';
|
||||
}
|
||||
if (value > 20) {
|
||||
return 'low';
|
||||
}
|
||||
if (value > 0) {
|
||||
return 'minimal';
|
||||
}
|
||||
return 'none';
|
||||
}
|
||||
|
||||
// Use named constants for flags
|
||||
const UserFlags = {
|
||||
IS_VERIFIED: 0x1,
|
||||
IS_PREMIUM: 0x2,
|
||||
IS_ADMIN: 0x4
|
||||
} as const;
|
||||
|
||||
function hasUserFlag(flags: number, flag: number): boolean {
|
||||
return (flags & flag) !== 0;
|
||||
}
|
||||
|
||||
function isValidPremiumAdmin(flags: number): boolean {
|
||||
const isVerified = hasUserFlag(flags, UserFlags.IS_VERIFIED);
|
||||
const isPremium = hasUserFlag(flags, UserFlags.IS_PREMIUM);
|
||||
const isAdmin = hasUserFlag(flags, UserFlags.IS_ADMIN);
|
||||
|
||||
return (isVerified && isPremium) || isAdmin;
|
||||
}
|
||||
|
||||
// Use a simple email validation
|
||||
function isValidEmail(email: string): boolean {
|
||||
if (!email || email.length > 254) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const atIndex = email.indexOf('@');
|
||||
if (atIndex < 1) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const domain = email.slice(atIndex + 1);
|
||||
if (!domain || !domain.includes('.')) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
// Or use a well-tested library with clear intent
|
||||
import { isEmail } from 'validator';
|
||||
const isValidEmail = isEmail(email);
|
||||
|
||||
// Use explicit control flow
|
||||
function handleUserAction(user: User | null): void {
|
||||
if (!user) {
|
||||
showError('User not found');
|
||||
return;
|
||||
}
|
||||
|
||||
if (!user.isActive) {
|
||||
showError('User account is inactive');
|
||||
return;
|
||||
}
|
||||
|
||||
if (!user.hasPermission('admin')) {
|
||||
showError('Admin permission required');
|
||||
return;
|
||||
}
|
||||
|
||||
doAdminStuff();
|
||||
}
|
||||
|
||||
// Use descriptive variable and function names
|
||||
function generateEvenNumbersSum(count: number): number {
|
||||
const evenNumbers: number[] = [];
|
||||
|
||||
for (let i = 0; i < count; i++) {
|
||||
evenNumbers.push(i * 2);
|
||||
}
|
||||
|
||||
const sum = evenNumbers.reduce((total, num) => total + num, 0);
|
||||
|
||||
return sum;
|
||||
}
|
||||
|
||||
// Use full, descriptive parameter names
|
||||
interface PriceAdjustmentOptions {
|
||||
rateMultiplier: number;
|
||||
shippingSurcharge?: number;
|
||||
minimumThreshold: number;
|
||||
}
|
||||
|
||||
function adjustProductPrices(
|
||||
products: Product[],
|
||||
options: PriceAdjustmentOptions
|
||||
): Product[] {
|
||||
return products
|
||||
.map(product => ({
|
||||
...product,
|
||||
totalPrice: product.basePrice * options.rateMultiplier,
|
||||
shippingCost: options.shippingSurcharge
|
||||
? product.shippingCost + options.shippingSurcharge
|
||||
: product.shippingCost
|
||||
}))
|
||||
.filter(product => product.totalPrice > options.minimumThreshold);
|
||||
}
|
||||
|
||||
// Usage is self-documenting
|
||||
const adjustedProducts = adjustProductPrices(products, {
|
||||
rateMultiplier: 1.1,
|
||||
shippingSurcharge: 5,
|
||||
minimumThreshold: 20
|
||||
});
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Comprehension Speed**: Readable code is understood quickly. Clever code requires deciphering.
|
||||
|
||||
2. **Fewer Bugs**: When code clearly expresses intent, mistakes are obvious.
|
||||
|
||||
3. **Onboarding**: New team members can contribute faster with readable code.
|
||||
|
||||
4. **Code Reviews**: Reviewers can focus on logic, not translation.
|
||||
|
||||
5. **Future You**: Code you wrote 6 months ago might as well have been written by someone else.
|
||||
|
||||
6. **Maintenance Cost**: Most of a codebase's lifetime is spent in maintenance, not initial development.
|
||||
|
||||
7. **Collaboration**: Teams work better when everyone can understand everyone's code.
|
||||
@@ -0,0 +1,255 @@
|
||||
---
|
||||
id: core-kiss-simplicity
|
||||
title: KISS - Simplicity
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [KISS, simplicity, over-engineering, maintainability]
|
||||
related: [core-kiss-readability, core-yagni-abstractions, solid-srp-function]
|
||||
---
|
||||
|
||||
# KISS Principle - Simplicity
|
||||
|
||||
Keep It Simple, Stupid. Choose the simplest solution that solves the problem. Avoid unnecessary complexity, over-engineering, and clever code that's hard to understand.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Over-engineered solution for a simple problem
|
||||
|
||||
// Simple task: Check if a user can access a resource
|
||||
// Over-engineered approach with unnecessary abstractions
|
||||
|
||||
interface AccessControlStrategy {
|
||||
evaluate(context: AccessContext): AccessDecision;
|
||||
}
|
||||
|
||||
interface AccessContext {
|
||||
subject: Subject;
|
||||
resource: Resource;
|
||||
action: Action;
|
||||
environment: Environment;
|
||||
}
|
||||
|
||||
interface Subject {
|
||||
id: string;
|
||||
attributes: Map<string, AttributeValue>;
|
||||
}
|
||||
|
||||
interface Resource {
|
||||
id: string;
|
||||
type: string;
|
||||
attributes: Map<string, AttributeValue>;
|
||||
}
|
||||
|
||||
interface Action {
|
||||
id: string;
|
||||
attributes: Map<string, AttributeValue>;
|
||||
}
|
||||
|
||||
interface Environment {
|
||||
currentTime: Date;
|
||||
ipAddress: string;
|
||||
attributes: Map<string, AttributeValue>;
|
||||
}
|
||||
|
||||
type AttributeValue = string | number | boolean | string[];
|
||||
|
||||
interface AccessDecision {
|
||||
decision: 'permit' | 'deny' | 'indeterminate' | 'not_applicable';
|
||||
obligations?: Obligation[];
|
||||
advice?: Advice[];
|
||||
}
|
||||
|
||||
interface Obligation {
|
||||
id: string;
|
||||
fulfillOn: 'permit' | 'deny';
|
||||
attributes: Map<string, AttributeValue>;
|
||||
}
|
||||
|
||||
interface Advice {
|
||||
id: string;
|
||||
appliesTo: 'permit' | 'deny';
|
||||
attributes: Map<string, AttributeValue>;
|
||||
}
|
||||
|
||||
class AttributeBasedAccessControl {
|
||||
private strategies: AccessControlStrategy[] = [];
|
||||
private combiningAlgorithm: CombiningAlgorithm;
|
||||
|
||||
constructor(combiningAlgorithm: CombiningAlgorithm) {
|
||||
this.combiningAlgorithm = combiningAlgorithm;
|
||||
}
|
||||
|
||||
addStrategy(strategy: AccessControlStrategy): void {
|
||||
this.strategies.push(strategy);
|
||||
}
|
||||
|
||||
evaluate(context: AccessContext): AccessDecision {
|
||||
const decisions = this.strategies.map(s => s.evaluate(context));
|
||||
return this.combiningAlgorithm.combine(decisions);
|
||||
}
|
||||
}
|
||||
|
||||
// 500+ more lines of abstraction layers...
|
||||
|
||||
// Actually usage for a simple check:
|
||||
const context: AccessContext = {
|
||||
subject: {
|
||||
id: user.id,
|
||||
attributes: new Map([['role', user.role]])
|
||||
},
|
||||
resource: {
|
||||
id: document.id,
|
||||
type: 'document',
|
||||
attributes: new Map([['ownerId', document.ownerId]])
|
||||
},
|
||||
action: {
|
||||
id: 'read',
|
||||
attributes: new Map()
|
||||
},
|
||||
environment: {
|
||||
currentTime: new Date(),
|
||||
ipAddress: request.ip,
|
||||
attributes: new Map()
|
||||
}
|
||||
};
|
||||
|
||||
const decision = accessControl.evaluate(context);
|
||||
if (decision.decision === 'permit') {
|
||||
// Allow access
|
||||
}
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Simple, direct solution
|
||||
|
||||
// Simple function that does what's needed
|
||||
function canUserAccessDocument(user: User, document: Document, action: 'read' | 'write' | 'delete'): boolean {
|
||||
// Admin can do anything
|
||||
if (user.role === 'admin') {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Owner can do anything with their document
|
||||
if (document.ownerId === user.id) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check explicit permissions
|
||||
const permission = document.permissions.find(p => p.userId === user.id);
|
||||
if (!permission) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check if permission level is sufficient
|
||||
switch (action) {
|
||||
case 'read':
|
||||
return ['read', 'write', 'admin'].includes(permission.level);
|
||||
case 'write':
|
||||
return ['write', 'admin'].includes(permission.level);
|
||||
case 'delete':
|
||||
return permission.level === 'admin';
|
||||
default:
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// Usage is straightforward
|
||||
if (canUserAccessDocument(user, document, 'read')) {
|
||||
// Allow access
|
||||
}
|
||||
|
||||
// If requirements grow, evolve the solution incrementally
|
||||
// Add time-based access when actually needed
|
||||
function canUserAccessDocument(
|
||||
user: User,
|
||||
document: Document,
|
||||
action: 'read' | 'write' | 'delete'
|
||||
): boolean {
|
||||
// Admin can do anything
|
||||
if (user.role === 'admin') {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Owner can do anything with their document
|
||||
if (document.ownerId === user.id) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check explicit permissions
|
||||
const permission = document.permissions.find(p => p.userId === user.id);
|
||||
if (!permission) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check expiration (added when actually needed)
|
||||
if (permission.expiresAt && permission.expiresAt < new Date()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check if permission level is sufficient
|
||||
const requiredLevel = getRequiredLevel(action);
|
||||
return hasRequiredLevel(permission.level, requiredLevel);
|
||||
}
|
||||
|
||||
function getRequiredLevel(action: 'read' | 'write' | 'delete'): PermissionLevel {
|
||||
const levels: Record<string, PermissionLevel> = {
|
||||
read: 'read',
|
||||
write: 'write',
|
||||
delete: 'admin'
|
||||
};
|
||||
return levels[action];
|
||||
}
|
||||
|
||||
function hasRequiredLevel(userLevel: PermissionLevel, required: PermissionLevel): boolean {
|
||||
const hierarchy: PermissionLevel[] = ['read', 'write', 'admin'];
|
||||
return hierarchy.indexOf(userLevel) >= hierarchy.indexOf(required);
|
||||
}
|
||||
|
||||
// Still simple, still readable, handles new requirement
|
||||
|
||||
// For multiple resources, create focused helper
|
||||
class DocumentAccessChecker {
|
||||
canRead(user: User, document: Document): boolean {
|
||||
return canUserAccessDocument(user, document, 'read');
|
||||
}
|
||||
|
||||
canWrite(user: User, document: Document): boolean {
|
||||
return canUserAccessDocument(user, document, 'write');
|
||||
}
|
||||
|
||||
canDelete(user: User, document: Document): boolean {
|
||||
return canUserAccessDocument(user, document, 'delete');
|
||||
}
|
||||
|
||||
// Filter a list of documents to only accessible ones
|
||||
filterReadable(user: User, documents: Document[]): Document[] {
|
||||
return documents.filter(doc => this.canRead(user, doc));
|
||||
}
|
||||
}
|
||||
|
||||
// Usage remains simple
|
||||
const checker = new DocumentAccessChecker();
|
||||
if (checker.canRead(user, document)) {
|
||||
// Show document
|
||||
}
|
||||
const accessibleDocs = checker.filterReadable(user, allDocuments);
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Readability**: Simple code can be understood in seconds. Complex abstractions require studying.
|
||||
|
||||
2. **Maintainability**: New developers can work with simple code immediately. Complex frameworks need training.
|
||||
|
||||
3. **Debugging**: When something breaks, simple code has obvious failure points.
|
||||
|
||||
4. **Performance**: Simple code is often faster - fewer layers, fewer allocations, less indirection.
|
||||
|
||||
5. **Time to Market**: Simple solutions are built and shipped faster.
|
||||
|
||||
6. **YAGNI Alignment**: The complex solution solves problems you don't have (and may never have).
|
||||
|
||||
7. **Incremental Complexity**: Start simple, add complexity only when real requirements demand it. The simple solution can evolve.
|
||||
321
.agents/skills/clean-code-principles/rules/core-law-demeter.md
Normal file
321
.agents/skills/clean-code-principles/rules/core-law-demeter.md
Normal file
@@ -0,0 +1,321 @@
|
||||
---
|
||||
id: core-law-demeter
|
||||
title: Law of Demeter
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [law-of-demeter, coupling, encapsulation]
|
||||
related: [core-encapsulation, solid-srp-class, core-separation-concerns]
|
||||
---
|
||||
|
||||
# Law of Demeter
|
||||
|
||||
A method should only talk to its immediate friends, not to strangers. Don't reach through objects to access their internal structure. This reduces coupling and makes code more maintainable.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Reaching through object chains
|
||||
|
||||
class Address {
|
||||
street: string;
|
||||
city: string;
|
||||
country: Country;
|
||||
}
|
||||
|
||||
class Country {
|
||||
name: string;
|
||||
code: string;
|
||||
taxRules: TaxRules;
|
||||
}
|
||||
|
||||
class TaxRules {
|
||||
vatRate: number;
|
||||
calculateTax(amount: number): number {
|
||||
return amount * this.vatRate;
|
||||
}
|
||||
}
|
||||
|
||||
class Customer {
|
||||
name: string;
|
||||
address: Address;
|
||||
wallet: Wallet;
|
||||
}
|
||||
|
||||
class Wallet {
|
||||
balance: number;
|
||||
currency: Currency;
|
||||
|
||||
deduct(amount: number): void {
|
||||
this.balance -= amount;
|
||||
}
|
||||
}
|
||||
|
||||
class Currency {
|
||||
code: string;
|
||||
exchangeRate: number;
|
||||
}
|
||||
|
||||
class Order {
|
||||
customer: Customer;
|
||||
items: OrderItem[];
|
||||
|
||||
// Violation: Reaching deep into customer's structure
|
||||
getCustomerCountry(): string {
|
||||
return this.customer.address.country.name; // 4 levels deep!
|
||||
}
|
||||
|
||||
// Violation: Reaching into customer's wallet
|
||||
calculateTax(): number {
|
||||
const amount = this.getTotal();
|
||||
// Reaching through customer -> address -> country -> taxRules
|
||||
return this.customer.address.country.taxRules.calculateTax(amount);
|
||||
}
|
||||
|
||||
// Violation: Manipulating customer's wallet directly
|
||||
processPayment(): void {
|
||||
const total = this.calculateTax() + this.getTotal();
|
||||
|
||||
// Reaching into wallet to check and modify
|
||||
if (this.customer.wallet.balance < total) {
|
||||
throw new Error('Insufficient funds');
|
||||
}
|
||||
|
||||
// Reaching into wallet's currency for conversion
|
||||
const exchangeRate = this.customer.wallet.currency.exchangeRate;
|
||||
const convertedAmount = total * exchangeRate;
|
||||
|
||||
this.customer.wallet.deduct(convertedAmount);
|
||||
}
|
||||
|
||||
getTotal(): number {
|
||||
return this.items.reduce((sum, item) => sum + item.price, 0);
|
||||
}
|
||||
}
|
||||
|
||||
// Problems with this approach:
|
||||
// 1. Order knows too much about Customer's internal structure
|
||||
// 2. Changes to Address, Country, or Wallet break Order
|
||||
// 3. Hard to test - must mock entire object graph
|
||||
// 4. Tight coupling between unrelated classes
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Talk only to immediate friends
|
||||
|
||||
class Address {
|
||||
private street: string;
|
||||
private city: string;
|
||||
private country: Country;
|
||||
|
||||
constructor(street: string, city: string, country: Country) {
|
||||
this.street = street;
|
||||
this.city = city;
|
||||
this.country = country;
|
||||
}
|
||||
|
||||
getCountryName(): string {
|
||||
return this.country.getName();
|
||||
}
|
||||
|
||||
calculateTax(amount: number): number {
|
||||
return this.country.calculateTax(amount);
|
||||
}
|
||||
}
|
||||
|
||||
class Country {
|
||||
private name: string;
|
||||
private code: string;
|
||||
private taxRules: TaxRules;
|
||||
|
||||
constructor(name: string, code: string, taxRules: TaxRules) {
|
||||
this.name = name;
|
||||
this.code = code;
|
||||
this.taxRules = taxRules;
|
||||
}
|
||||
|
||||
getName(): string {
|
||||
return this.name;
|
||||
}
|
||||
|
||||
getCode(): string {
|
||||
return this.code;
|
||||
}
|
||||
|
||||
calculateTax(amount: number): number {
|
||||
return this.taxRules.calculate(amount);
|
||||
}
|
||||
}
|
||||
|
||||
class TaxRules {
|
||||
private vatRate: number;
|
||||
|
||||
constructor(vatRate: number) {
|
||||
this.vatRate = vatRate;
|
||||
}
|
||||
|
||||
calculate(amount: number): number {
|
||||
return amount * this.vatRate;
|
||||
}
|
||||
}
|
||||
|
||||
class Wallet {
|
||||
private balance: number;
|
||||
private currency: Currency;
|
||||
|
||||
constructor(balance: number, currency: Currency) {
|
||||
this.balance = balance;
|
||||
this.currency = currency;
|
||||
}
|
||||
|
||||
canAfford(amount: number): boolean {
|
||||
const convertedAmount = this.currency.convert(amount);
|
||||
return this.balance >= convertedAmount;
|
||||
}
|
||||
|
||||
pay(amount: number): PaymentResult {
|
||||
const convertedAmount = this.currency.convert(amount);
|
||||
|
||||
if (!this.canAfford(amount)) {
|
||||
return { success: false, error: 'Insufficient funds' };
|
||||
}
|
||||
|
||||
this.balance -= convertedAmount;
|
||||
return { success: true, amountPaid: convertedAmount };
|
||||
}
|
||||
|
||||
getBalance(): number {
|
||||
return this.balance;
|
||||
}
|
||||
}
|
||||
|
||||
class Currency {
|
||||
private code: string;
|
||||
private exchangeRate: number;
|
||||
|
||||
constructor(code: string, exchangeRate: number) {
|
||||
this.code = code;
|
||||
this.exchangeRate = exchangeRate;
|
||||
}
|
||||
|
||||
convert(amount: number): number {
|
||||
return amount * this.exchangeRate;
|
||||
}
|
||||
}
|
||||
|
||||
class Customer {
|
||||
private name: string;
|
||||
private address: Address;
|
||||
private wallet: Wallet;
|
||||
|
||||
constructor(name: string, address: Address, wallet: Wallet) {
|
||||
this.name = name;
|
||||
this.address = address;
|
||||
this.wallet = wallet;
|
||||
}
|
||||
|
||||
getName(): string {
|
||||
return this.name;
|
||||
}
|
||||
|
||||
getCountryName(): string {
|
||||
return this.address.getCountryName();
|
||||
}
|
||||
|
||||
calculateTaxFor(amount: number): number {
|
||||
return this.address.calculateTax(amount);
|
||||
}
|
||||
|
||||
canAfford(amount: number): boolean {
|
||||
return this.wallet.canAfford(amount);
|
||||
}
|
||||
|
||||
pay(amount: number): PaymentResult {
|
||||
return this.wallet.pay(amount);
|
||||
}
|
||||
}
|
||||
|
||||
class Order {
|
||||
private customer: Customer;
|
||||
private items: OrderItem[];
|
||||
|
||||
constructor(customer: Customer, items: OrderItem[]) {
|
||||
this.customer = customer;
|
||||
this.items = items;
|
||||
}
|
||||
|
||||
// Only talks to immediate friend (customer)
|
||||
getCustomerCountry(): string {
|
||||
return this.customer.getCountryName();
|
||||
}
|
||||
|
||||
getTotal(): number {
|
||||
return this.items.reduce((sum, item) => sum + item.getPrice(), 0);
|
||||
}
|
||||
|
||||
// Asks customer to calculate tax (customer knows how)
|
||||
calculateTax(): number {
|
||||
return this.customer.calculateTaxFor(this.getTotal());
|
||||
}
|
||||
|
||||
getTotalWithTax(): number {
|
||||
return this.getTotal() + this.calculateTax();
|
||||
}
|
||||
|
||||
// Asks customer to pay (customer handles wallet)
|
||||
processPayment(): PaymentResult {
|
||||
const total = this.getTotalWithTax();
|
||||
|
||||
if (!this.customer.canAfford(total)) {
|
||||
return { success: false, error: 'Insufficient funds' };
|
||||
}
|
||||
|
||||
return this.customer.pay(total);
|
||||
}
|
||||
}
|
||||
|
||||
// Usage
|
||||
const order = new Order(customer, items);
|
||||
const result = order.processPayment();
|
||||
|
||||
if (result.success) {
|
||||
console.log(`Payment successful: ${result.amountPaid}`);
|
||||
} else {
|
||||
console.log(`Payment failed: ${result.error}`);
|
||||
}
|
||||
|
||||
// Testing is now easy - only mock the immediate friend
|
||||
describe('Order', () => {
|
||||
it('should process payment through customer', () => {
|
||||
const mockCustomer: Customer = {
|
||||
getCountryName: () => 'USA',
|
||||
calculateTaxFor: (amount: number) => amount * 0.1,
|
||||
canAfford: () => true,
|
||||
pay: jest.fn().mockReturnValue({ success: true, amountPaid: 110 })
|
||||
} as any;
|
||||
|
||||
const order = new Order(mockCustomer, [{ getPrice: () => 100 }]);
|
||||
const result = order.processPayment();
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(mockCustomer.pay).toHaveBeenCalledWith(110);
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Reduced Coupling**: Order only knows about Customer. Changes to Address, Country, or Wallet don't affect Order.
|
||||
|
||||
2. **Encapsulation**: Internal structure is hidden. Customer can change how it stores address without affecting clients.
|
||||
|
||||
3. **Easier Testing**: Mock only the immediate friend. No need to construct deep object graphs.
|
||||
|
||||
4. **Better Abstraction**: Customer is responsible for customer things. Order doesn't need to know about wallets.
|
||||
|
||||
5. **Maintainability**: When requirements change, changes are localized to the responsible class.
|
||||
|
||||
6. **Readability**: `customer.calculateTaxFor(amount)` is clearer than `customer.address.country.taxRules.calculateTax(amount)`.
|
||||
|
||||
7. **Flexibility**: Can change internal implementations without affecting clients. Customer could switch from wallet to payment service.
|
||||
@@ -0,0 +1,337 @@
|
||||
---
|
||||
id: core-separation-concerns
|
||||
title: Separation of Concerns
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [separation-of-concerns, modularity, cohesion]
|
||||
related: [solid-srp-class, solid-srp-function, core-law-demeter]
|
||||
---
|
||||
|
||||
# Separation of Concerns
|
||||
|
||||
Different concerns should be handled by different parts of the system. Each module, class, or function should address a single concern, making the code easier to understand, test, and modify.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Multiple concerns mixed together
|
||||
|
||||
class OrderProcessor {
|
||||
async processOrder(orderData: any, request: Request): Promise<Response> {
|
||||
// Concern 1: HTTP request parsing
|
||||
const authHeader = request.headers.get('Authorization');
|
||||
if (!authHeader || !authHeader.startsWith('Bearer ')) {
|
||||
return new Response(JSON.stringify({ error: 'Unauthorized' }), { status: 401 });
|
||||
}
|
||||
const token = authHeader.slice(7);
|
||||
|
||||
// Concern 2: Authentication
|
||||
let userId: string;
|
||||
try {
|
||||
const decoded = jwt.verify(token, process.env.JWT_SECRET!) as { userId: string };
|
||||
userId = decoded.userId;
|
||||
} catch {
|
||||
return new Response(JSON.stringify({ error: 'Invalid token' }), { status: 401 });
|
||||
}
|
||||
|
||||
// Concern 3: Validation
|
||||
if (!orderData.items || orderData.items.length === 0) {
|
||||
return new Response(JSON.stringify({ error: 'Order must have items' }), { status: 400 });
|
||||
}
|
||||
for (const item of orderData.items) {
|
||||
if (!item.productId || item.quantity < 1) {
|
||||
return new Response(JSON.stringify({ error: 'Invalid item' }), { status: 400 });
|
||||
}
|
||||
}
|
||||
|
||||
// Concern 4: Database access
|
||||
const connection = await mysql.createConnection(process.env.DATABASE_URL!);
|
||||
|
||||
try {
|
||||
// Concern 5: Business logic - inventory check
|
||||
for (const item of orderData.items) {
|
||||
const [rows] = await connection.execute(
|
||||
'SELECT stock FROM products WHERE id = ?',
|
||||
[item.productId]
|
||||
);
|
||||
if (rows[0].stock < item.quantity) {
|
||||
return new Response(
|
||||
JSON.stringify({ error: `Insufficient stock for ${item.productId}` }),
|
||||
{ status: 400 }
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Concern 6: Business logic - pricing
|
||||
let total = 0;
|
||||
for (const item of orderData.items) {
|
||||
const [rows] = await connection.execute(
|
||||
'SELECT price FROM products WHERE id = ?',
|
||||
[item.productId]
|
||||
);
|
||||
total += rows[0].price * item.quantity;
|
||||
}
|
||||
|
||||
// Concern 7: Business logic - create order
|
||||
const orderId = crypto.randomUUID();
|
||||
await connection.execute(
|
||||
'INSERT INTO orders (id, user_id, total, status) VALUES (?, ?, ?, ?)',
|
||||
[orderId, userId, total, 'pending']
|
||||
);
|
||||
|
||||
// Concern 8: Payment processing
|
||||
const stripe = new Stripe(process.env.STRIPE_KEY!);
|
||||
const paymentIntent = await stripe.paymentIntents.create({
|
||||
amount: Math.round(total * 100),
|
||||
currency: 'usd',
|
||||
customer: userId
|
||||
});
|
||||
|
||||
// Concern 9: Inventory update
|
||||
for (const item of orderData.items) {
|
||||
await connection.execute(
|
||||
'UPDATE products SET stock = stock - ? WHERE id = ?',
|
||||
[item.quantity, item.productId]
|
||||
);
|
||||
}
|
||||
|
||||
// Concern 10: Notification
|
||||
const transporter = nodemailer.createTransport({ /* SMTP config */ });
|
||||
await transporter.sendMail({
|
||||
to: orderData.email,
|
||||
subject: 'Order Confirmation',
|
||||
html: `<h1>Order ${orderId} confirmed</h1><p>Total: $${total}</p>`
|
||||
});
|
||||
|
||||
// Concern 11: Logging
|
||||
console.log(`Order ${orderId} created for user ${userId}`);
|
||||
|
||||
// Concern 12: HTTP response formatting
|
||||
return new Response(
|
||||
JSON.stringify({ orderId, total, paymentIntentId: paymentIntent.id }),
|
||||
{ status: 201, headers: { 'Content-Type': 'application/json' } }
|
||||
);
|
||||
|
||||
} finally {
|
||||
await connection.end();
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Each concern in its own module
|
||||
|
||||
// Concern: HTTP handling
|
||||
class OrderController {
|
||||
constructor(private orderService: OrderService) {}
|
||||
|
||||
async createOrder(req: Request, res: Response): Promise<void> {
|
||||
try {
|
||||
const userId = req.user.id; // Auth middleware already handled this
|
||||
const orderData = req.body; // Validation middleware already validated
|
||||
|
||||
const order = await this.orderService.createOrder(userId, orderData);
|
||||
|
||||
res.status(201).json({
|
||||
orderId: order.id,
|
||||
total: order.total,
|
||||
status: order.status
|
||||
});
|
||||
} catch (error) {
|
||||
if (error instanceof InsufficientStockError) {
|
||||
res.status(400).json({ error: error.message });
|
||||
} else if (error instanceof PaymentFailedError) {
|
||||
res.status(402).json({ error: 'Payment failed' });
|
||||
} else {
|
||||
throw error; // Let error middleware handle
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Concern: Authentication (middleware)
|
||||
class AuthMiddleware {
|
||||
constructor(private authService: AuthService) {}
|
||||
|
||||
async handle(req: Request, res: Response, next: NextFunction): Promise<void> {
|
||||
const token = this.extractToken(req);
|
||||
if (!token) {
|
||||
res.status(401).json({ error: 'Unauthorized' });
|
||||
return;
|
||||
}
|
||||
|
||||
const user = await this.authService.validateToken(token);
|
||||
if (!user) {
|
||||
res.status(401).json({ error: 'Invalid token' });
|
||||
return;
|
||||
}
|
||||
|
||||
req.user = user;
|
||||
next();
|
||||
}
|
||||
|
||||
private extractToken(req: Request): string | null {
|
||||
const header = req.headers.authorization;
|
||||
return header?.startsWith('Bearer ') ? header.slice(7) : null;
|
||||
}
|
||||
}
|
||||
|
||||
// Concern: Validation (separate validator)
|
||||
class OrderValidator {
|
||||
validate(data: unknown): CreateOrderData {
|
||||
const schema = z.object({
|
||||
items: z.array(z.object({
|
||||
productId: z.string().uuid(),
|
||||
quantity: z.number().int().positive()
|
||||
})).min(1)
|
||||
});
|
||||
|
||||
return schema.parse(data);
|
||||
}
|
||||
}
|
||||
|
||||
// Concern: Business logic orchestration
|
||||
class OrderService {
|
||||
constructor(
|
||||
private orderRepository: OrderRepository,
|
||||
private inventoryService: InventoryService,
|
||||
private pricingService: PricingService,
|
||||
private paymentService: PaymentService,
|
||||
private notificationService: NotificationService,
|
||||
private logger: Logger
|
||||
) {}
|
||||
|
||||
async createOrder(userId: string, data: CreateOrderData): Promise<Order> {
|
||||
// Check inventory
|
||||
await this.inventoryService.checkAvailability(data.items);
|
||||
|
||||
// Calculate pricing
|
||||
const pricing = await this.pricingService.calculateTotal(data.items);
|
||||
|
||||
// Create order
|
||||
const order = await this.orderRepository.create({
|
||||
userId,
|
||||
items: data.items,
|
||||
total: pricing.total,
|
||||
status: 'pending'
|
||||
});
|
||||
|
||||
// Process payment
|
||||
await this.paymentService.charge(userId, pricing.total, order.id);
|
||||
|
||||
// Reserve inventory
|
||||
await this.inventoryService.reserve(data.items);
|
||||
|
||||
// Send confirmation
|
||||
await this.notificationService.sendOrderConfirmation(order);
|
||||
|
||||
// Log
|
||||
this.logger.info('Order created', { orderId: order.id, userId });
|
||||
|
||||
return order;
|
||||
}
|
||||
}
|
||||
|
||||
// Concern: Inventory management
|
||||
class InventoryService {
|
||||
constructor(private productRepository: ProductRepository) {}
|
||||
|
||||
async checkAvailability(items: OrderItem[]): Promise<void> {
|
||||
for (const item of items) {
|
||||
const product = await this.productRepository.findById(item.productId);
|
||||
if (product.stock < item.quantity) {
|
||||
throw new InsufficientStockError(item.productId, product.stock, item.quantity);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async reserve(items: OrderItem[]): Promise<void> {
|
||||
for (const item of items) {
|
||||
await this.productRepository.decrementStock(item.productId, item.quantity);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Concern: Pricing calculation
|
||||
class PricingService {
|
||||
constructor(private productRepository: ProductRepository) {}
|
||||
|
||||
async calculateTotal(items: OrderItem[]): Promise<PricingResult> {
|
||||
let subtotal = 0;
|
||||
|
||||
for (const item of items) {
|
||||
const product = await this.productRepository.findById(item.productId);
|
||||
subtotal += product.price * item.quantity;
|
||||
}
|
||||
|
||||
const tax = this.calculateTax(subtotal);
|
||||
const total = subtotal + tax;
|
||||
|
||||
return { subtotal, tax, total };
|
||||
}
|
||||
|
||||
private calculateTax(amount: number): number {
|
||||
return amount * 0.1; // 10% tax
|
||||
}
|
||||
}
|
||||
|
||||
// Concern: Payment processing
|
||||
class PaymentService {
|
||||
constructor(private stripeClient: Stripe) {}
|
||||
|
||||
async charge(userId: string, amount: number, orderId: string): Promise<Payment> {
|
||||
const intent = await this.stripeClient.paymentIntents.create({
|
||||
amount: Math.round(amount * 100),
|
||||
currency: 'usd',
|
||||
metadata: { orderId }
|
||||
});
|
||||
|
||||
return { id: intent.id, status: 'pending' };
|
||||
}
|
||||
}
|
||||
|
||||
// Concern: Notifications
|
||||
class NotificationService {
|
||||
constructor(private emailService: EmailService) {}
|
||||
|
||||
async sendOrderConfirmation(order: Order): Promise<void> {
|
||||
await this.emailService.send({
|
||||
to: order.userEmail,
|
||||
template: 'order-confirmation',
|
||||
data: { orderId: order.id, total: order.total }
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Concern: Data access
|
||||
class OrderRepository {
|
||||
constructor(private db: Database) {}
|
||||
|
||||
async create(data: CreateOrderInput): Promise<Order> {
|
||||
return this.db.orders.create({ data });
|
||||
}
|
||||
|
||||
async findById(id: string): Promise<Order | null> {
|
||||
return this.db.orders.findUnique({ where: { id } });
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Understandability**: Each module has one job. Easy to understand what it does.
|
||||
|
||||
2. **Testability**: Test inventory logic without payment, test pricing without database.
|
||||
|
||||
3. **Reusability**: `PricingService` can be used for quotes, carts, or invoices.
|
||||
|
||||
4. **Maintainability**: Change email provider? Only touch `EmailService`.
|
||||
|
||||
5. **Team Scaling**: Different developers can work on different concerns without conflicts.
|
||||
|
||||
6. **Flexibility**: Swap Stripe for another payment provider by changing only `PaymentService`.
|
||||
|
||||
7. **Debugging**: Error in pricing? Look at `PricingService`. Clear boundaries help locate issues.
|
||||
@@ -0,0 +1,252 @@
|
||||
---
|
||||
id: core-yagni-abstractions
|
||||
title: YAGNI - Abstractions
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [YAGNI, premature-abstraction, simplicity]
|
||||
related: [core-yagni-features, core-kiss-simplicity, solid-ocp-abstraction]
|
||||
---
|
||||
|
||||
# YAGNI Principle - Abstractions
|
||||
|
||||
Don't create abstractions until you have concrete evidence they're needed. Premature abstraction leads to wrong abstractions that are worse than no abstraction.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Creating abstractions before understanding the problem
|
||||
|
||||
// Task: Send an email notification
|
||||
// Over-abstracted solution based on imagined future needs
|
||||
|
||||
// "We might need different notification channels someday"
|
||||
interface NotificationChannel {
|
||||
send(notification: Notification): Promise<void>;
|
||||
getCapabilities(): ChannelCapabilities;
|
||||
isAvailable(): Promise<boolean>;
|
||||
}
|
||||
|
||||
// "We might need different notification types"
|
||||
interface Notification {
|
||||
id: string;
|
||||
type: NotificationType;
|
||||
priority: NotificationPriority;
|
||||
payload: NotificationPayload;
|
||||
metadata: NotificationMetadata;
|
||||
}
|
||||
|
||||
// "We might need complex routing logic"
|
||||
interface NotificationRouter {
|
||||
route(notification: Notification): Promise<NotificationChannel[]>;
|
||||
registerChannel(channel: NotificationChannel): void;
|
||||
setRoutingRules(rules: RoutingRule[]): void;
|
||||
}
|
||||
|
||||
// "We might need to transform notifications per channel"
|
||||
interface NotificationTransformer {
|
||||
transform(notification: Notification, channel: NotificationChannel): TransformedNotification;
|
||||
}
|
||||
|
||||
// "We might need retry logic"
|
||||
interface NotificationRetryPolicy {
|
||||
shouldRetry(attempt: number, error: Error): boolean;
|
||||
getDelay(attempt: number): number;
|
||||
}
|
||||
|
||||
// "We might need to track delivery"
|
||||
interface NotificationTracker {
|
||||
trackSent(notification: Notification, channel: NotificationChannel): Promise<void>;
|
||||
trackDelivered(notificationId: string): Promise<void>;
|
||||
trackFailed(notificationId: string, error: Error): Promise<void>;
|
||||
}
|
||||
|
||||
// "We might need a notification queue"
|
||||
interface NotificationQueue {
|
||||
enqueue(notification: Notification): Promise<void>;
|
||||
process(): Promise<void>;
|
||||
getStatus(notificationId: string): Promise<QueueStatus>;
|
||||
}
|
||||
|
||||
// Orchestrator that ties it all together
|
||||
class NotificationOrchestrator {
|
||||
constructor(
|
||||
private router: NotificationRouter,
|
||||
private transformer: NotificationTransformer,
|
||||
private retryPolicy: NotificationRetryPolicy,
|
||||
private tracker: NotificationTracker,
|
||||
private queue: NotificationQueue
|
||||
) {}
|
||||
|
||||
async notify(notification: Notification): Promise<void> {
|
||||
await this.queue.enqueue(notification);
|
||||
// 200+ lines of orchestration logic
|
||||
}
|
||||
}
|
||||
|
||||
// But all we actually needed was:
|
||||
// Send an email when a user registers
|
||||
|
||||
// Result: 1000+ lines of abstraction, weeks of work, for sending one email
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Start concrete, abstract when patterns emerge
|
||||
|
||||
// Task: Send an email notification
|
||||
// Simple, direct solution
|
||||
|
||||
interface EmailOptions {
|
||||
to: string;
|
||||
subject: string;
|
||||
body: string;
|
||||
}
|
||||
|
||||
class EmailService {
|
||||
constructor(private smtpClient: SmtpClient) {}
|
||||
|
||||
async send(options: EmailOptions): Promise<void> {
|
||||
await this.smtpClient.send({
|
||||
from: 'noreply@example.com',
|
||||
to: options.to,
|
||||
subject: options.subject,
|
||||
html: options.body
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Usage
|
||||
const emailService = new EmailService(smtpClient);
|
||||
await emailService.send({
|
||||
to: user.email,
|
||||
subject: 'Welcome!',
|
||||
body: '<h1>Welcome to our app!</h1>'
|
||||
});
|
||||
|
||||
// Later, when we actually need SMS (not "might need"):
|
||||
class SmsService {
|
||||
constructor(private twilioClient: TwilioClient) {}
|
||||
|
||||
async send(phone: string, message: string): Promise<void> {
|
||||
await this.twilioClient.messages.create({
|
||||
to: phone,
|
||||
from: process.env.TWILIO_NUMBER,
|
||||
body: message
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Now we have TWO concrete implementations
|
||||
// We can see what they have in common
|
||||
|
||||
// Abstract AFTER seeing the pattern (Rule of Three)
|
||||
// After email, SMS, and push notifications exist:
|
||||
|
||||
interface NotificationSender {
|
||||
send(recipient: string, message: NotificationMessage): Promise<void>;
|
||||
}
|
||||
|
||||
interface NotificationMessage {
|
||||
subject?: string;
|
||||
body: string;
|
||||
}
|
||||
|
||||
class EmailNotificationSender implements NotificationSender {
|
||||
constructor(private emailService: EmailService) {}
|
||||
|
||||
async send(recipient: string, message: NotificationMessage): Promise<void> {
|
||||
await this.emailService.send({
|
||||
to: recipient,
|
||||
subject: message.subject || 'Notification',
|
||||
body: message.body
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
class SmsNotificationSender implements NotificationSender {
|
||||
constructor(private smsService: SmsService) {}
|
||||
|
||||
async send(recipient: string, message: NotificationMessage): Promise<void> {
|
||||
// SMS doesn't support subject, so we just use body
|
||||
await this.smsService.send(recipient, message.body);
|
||||
}
|
||||
}
|
||||
|
||||
class PushNotificationSender implements NotificationSender {
|
||||
constructor(private pushService: PushService) {}
|
||||
|
||||
async send(recipient: string, message: NotificationMessage): Promise<void> {
|
||||
await this.pushService.send(recipient, {
|
||||
title: message.subject,
|
||||
body: message.body
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Simple notification service that uses the abstraction
|
||||
class NotificationService {
|
||||
constructor(private senders: Map<string, NotificationSender>) {}
|
||||
|
||||
async notify(
|
||||
channel: 'email' | 'sms' | 'push',
|
||||
recipient: string,
|
||||
message: NotificationMessage
|
||||
): Promise<void> {
|
||||
const sender = this.senders.get(channel);
|
||||
if (!sender) {
|
||||
throw new Error(`Unknown notification channel: ${channel}`);
|
||||
}
|
||||
await sender.send(recipient, message);
|
||||
}
|
||||
}
|
||||
|
||||
// The abstraction fits because it was derived from concrete implementations
|
||||
// It's minimal - just what's needed, nothing speculative
|
||||
|
||||
// If we later need retry logic, we add it when we have concrete requirements:
|
||||
class RetryingNotificationSender implements NotificationSender {
|
||||
constructor(
|
||||
private sender: NotificationSender,
|
||||
private maxAttempts: number = 3
|
||||
) {}
|
||||
|
||||
async send(recipient: string, message: NotificationMessage): Promise<void> {
|
||||
let lastError: Error | undefined;
|
||||
|
||||
for (let attempt = 1; attempt <= this.maxAttempts; attempt++) {
|
||||
try {
|
||||
await this.sender.send(recipient, message);
|
||||
return;
|
||||
} catch (error) {
|
||||
lastError = error as Error;
|
||||
if (attempt < this.maxAttempts) {
|
||||
await this.delay(attempt * 1000);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
throw lastError;
|
||||
}
|
||||
|
||||
private delay(ms: number): Promise<void> {
|
||||
return new Promise(resolve => setTimeout(resolve, ms));
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Wrong Abstractions Are Costly**: Premature abstractions are often wrong because you don't understand the problem yet. Wrong abstractions are harder to change than no abstractions.
|
||||
|
||||
2. **Rule of Three**: Wait until you have three concrete examples before abstracting. Two is often coincidence; three reveals the pattern.
|
||||
|
||||
3. **Duplication Is Cheaper Than Wrong Abstraction**: It's easier to extract a correct abstraction from duplicated code than to fix a wrong abstraction.
|
||||
|
||||
4. **Context Matters**: Abstractions made with real requirements fit better than those made speculatively.
|
||||
|
||||
5. **Simplicity**: Concrete code is simpler to understand, debug, and modify.
|
||||
|
||||
6. **Evolutionary Design**: Let the design emerge from actual needs rather than imagined ones.
|
||||
|
||||
7. **Time Value**: The time spent on speculative abstractions could be spent on real features.
|
||||
@@ -0,0 +1,205 @@
|
||||
---
|
||||
id: core-yagni-features
|
||||
title: YAGNI - Features
|
||||
category: core-principles
|
||||
priority: critical
|
||||
tags: [YAGNI, speculative-features, lean-development]
|
||||
related: [core-yagni-abstractions, core-kiss-simplicity]
|
||||
---
|
||||
|
||||
# YAGNI Principle - Features
|
||||
|
||||
You Aren't Gonna Need It. Don't implement features until they are actually required. Building speculative features wastes time and adds unnecessary complexity.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Building features "just in case"
|
||||
|
||||
// Task: Create a simple user registration
|
||||
// Over-built solution with features nobody asked for
|
||||
|
||||
interface User {
|
||||
id: string;
|
||||
email: string;
|
||||
password: string;
|
||||
// Speculative fields - nobody asked for these
|
||||
phone?: string;
|
||||
address?: Address;
|
||||
preferences?: UserPreferences;
|
||||
socialProfiles?: SocialProfile[];
|
||||
twoFactorEnabled?: boolean;
|
||||
twoFactorSecret?: string;
|
||||
backupCodes?: string[];
|
||||
apiKeys?: ApiKey[];
|
||||
loginHistory?: LoginAttempt[];
|
||||
sessions?: Session[];
|
||||
subscriptionTier?: 'free' | 'basic' | 'premium' | 'enterprise';
|
||||
billingInfo?: BillingInfo;
|
||||
teamMemberships?: TeamMembership[];
|
||||
referralCode?: string;
|
||||
referredBy?: string;
|
||||
loyaltyPoints?: number;
|
||||
}
|
||||
|
||||
class UserService {
|
||||
// Basic registration - actually needed
|
||||
async register(email: string, password: string): Promise<User> {
|
||||
// ...
|
||||
}
|
||||
|
||||
// These were built "just in case" - never used
|
||||
|
||||
async enableTwoFactor(userId: string): Promise<TwoFactorSetup> {
|
||||
// 200 lines of 2FA implementation
|
||||
// Product never asked for this feature
|
||||
}
|
||||
|
||||
async generateApiKey(userId: string, scopes: string[]): Promise<ApiKey> {
|
||||
// 150 lines of API key management
|
||||
// No API exists for external developers
|
||||
}
|
||||
|
||||
async trackLoginAttempt(userId: string, success: boolean, ip: string): Promise<void> {
|
||||
// 100 lines of login tracking
|
||||
// No dashboard to view this data
|
||||
}
|
||||
|
||||
async manageSessions(userId: string): Promise<Session[]> {
|
||||
// 180 lines of session management
|
||||
// Users can't actually see or manage sessions
|
||||
}
|
||||
|
||||
async upgradeSubscription(userId: string, tier: string): Promise<void> {
|
||||
// 300 lines of subscription logic
|
||||
// App is free, no paid tiers planned
|
||||
}
|
||||
|
||||
async processReferral(referrerCode: string, newUserId: string): Promise<void> {
|
||||
// 120 lines of referral logic
|
||||
// Marketing hasn't planned a referral program
|
||||
}
|
||||
|
||||
async awardLoyaltyPoints(userId: string, action: string): Promise<void> {
|
||||
// 100 lines of loyalty point logic
|
||||
// No loyalty program exists
|
||||
}
|
||||
|
||||
// 1500+ lines of code for features that may never be used
|
||||
}
|
||||
|
||||
// Result:
|
||||
// - 3 weeks spent building speculative features
|
||||
// - 1500+ lines to maintain
|
||||
// - Bugs introduced in code that's never executed
|
||||
// - Actual requirements delayed
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Build only what's needed now
|
||||
|
||||
// Task: Create a simple user registration
|
||||
// Focused solution that meets actual requirements
|
||||
|
||||
interface User {
|
||||
id: string;
|
||||
email: string;
|
||||
passwordHash: string;
|
||||
createdAt: Date;
|
||||
updatedAt: Date;
|
||||
}
|
||||
|
||||
interface CreateUserData {
|
||||
email: string;
|
||||
password: string;
|
||||
}
|
||||
|
||||
class UserService {
|
||||
constructor(
|
||||
private userRepository: UserRepository,
|
||||
private passwordHasher: PasswordHasher,
|
||||
private emailService: EmailService
|
||||
) {}
|
||||
|
||||
async register(data: CreateUserData): Promise<User> {
|
||||
// Validate email
|
||||
const existingUser = await this.userRepository.findByEmail(data.email);
|
||||
if (existingUser) {
|
||||
throw new UserAlreadyExistsError(data.email);
|
||||
}
|
||||
|
||||
// Create user
|
||||
const passwordHash = await this.passwordHasher.hash(data.password);
|
||||
const user = await this.userRepository.create({
|
||||
email: data.email,
|
||||
passwordHash,
|
||||
createdAt: new Date(),
|
||||
updatedAt: new Date()
|
||||
});
|
||||
|
||||
// Send welcome email
|
||||
await this.emailService.sendWelcome(user.email);
|
||||
|
||||
return user;
|
||||
}
|
||||
|
||||
async findById(id: string): Promise<User | null> {
|
||||
return this.userRepository.findById(id);
|
||||
}
|
||||
|
||||
async findByEmail(email: string): Promise<User | null> {
|
||||
return this.userRepository.findByEmail(email);
|
||||
}
|
||||
}
|
||||
|
||||
// Result:
|
||||
// - Built in 2 days
|
||||
// - ~100 lines of focused code
|
||||
// - Easy to understand and maintain
|
||||
// - Shipped to users quickly
|
||||
// - Can add features when actually needed
|
||||
|
||||
// When 2FA is actually requested (6 months later):
|
||||
// Add it then with full context of actual requirements
|
||||
|
||||
interface TwoFactorService {
|
||||
enable(userId: string): Promise<TwoFactorSetup>;
|
||||
verify(userId: string, code: string): Promise<boolean>;
|
||||
disable(userId: string): Promise<void>;
|
||||
}
|
||||
|
||||
// When subscription tiers are planned (1 year later):
|
||||
// Add with actual business requirements, pricing, and features defined
|
||||
|
||||
interface SubscriptionService {
|
||||
getCurrentPlan(userId: string): Promise<Plan>;
|
||||
upgrade(userId: string, planId: string): Promise<void>;
|
||||
downgrade(userId: string, planId: string): Promise<void>;
|
||||
cancel(userId: string): Promise<void>;
|
||||
}
|
||||
|
||||
// Benefits of waiting:
|
||||
// 1. Requirements are clearer
|
||||
// 2. You know the actual use cases
|
||||
// 3. Technology may have improved
|
||||
// 4. You didn't maintain unused code for months
|
||||
// 5. You might not need it at all (50%+ of speculative features are never used)
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Waste Prevention**: Speculative features consume development time that could be spent on actual needs.
|
||||
|
||||
2. **Maintenance Burden**: Every line of code must be maintained, tested, and understood - even unused code.
|
||||
|
||||
3. **Complexity Cost**: Unused features add cognitive load when reading and modifying the codebase.
|
||||
|
||||
4. **Requirements Clarity**: Future requirements are clearer when you're actually implementing them.
|
||||
|
||||
5. **Flexibility**: Code without speculative features is easier to refactor and evolve.
|
||||
|
||||
6. **Faster Delivery**: Ship what's needed now, iterate based on real feedback.
|
||||
|
||||
7. **Better Design**: Features designed with real requirements and user feedback are better than guessed features.
|
||||
359
.agents/skills/clean-code-principles/rules/pattern-repository.md
Normal file
359
.agents/skills/clean-code-principles/rules/pattern-repository.md
Normal file
@@ -0,0 +1,359 @@
|
||||
---
|
||||
id: pattern-repository
|
||||
title: Design Pattern - Repository
|
||||
category: design-patterns
|
||||
priority: high
|
||||
tags: [design-patterns, repository, data-access, separation-of-concerns]
|
||||
related: [solid-dip-abstractions, solid-srp-class, core-separation-concerns]
|
||||
---
|
||||
|
||||
# Repository Pattern
|
||||
|
||||
The Repository pattern abstracts data persistence, providing a collection-like interface for accessing domain objects. It separates business logic from data access, makes code testable, and allows swapping storage implementations without changing application code.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// ❌ Data access mixed with business logic
|
||||
class OrderService {
|
||||
async createOrder(userId: string, items: CartItem[]) {
|
||||
// Business logic
|
||||
const total = items.reduce((sum, i) => sum + i.price * i.quantity, 0);
|
||||
|
||||
// Direct database access - tightly coupled
|
||||
const result = await pool.query(
|
||||
`INSERT INTO orders (user_id, total, status, created_at)
|
||||
VALUES ($1, $2, $3, NOW()) RETURNING *`,
|
||||
[userId, total, 'pending']
|
||||
);
|
||||
|
||||
const orderId = result.rows[0].id;
|
||||
|
||||
// More SQL scattered in business logic
|
||||
for (const item of items) {
|
||||
await pool.query(
|
||||
`INSERT INTO order_items (order_id, product_id, quantity, price)
|
||||
VALUES ($1, $2, $3, $4)`,
|
||||
[orderId, item.productId, item.quantity, item.price]
|
||||
);
|
||||
}
|
||||
|
||||
return result.rows[0];
|
||||
}
|
||||
|
||||
async getOrdersByUser(userId: string) {
|
||||
// SQL everywhere
|
||||
const result = await pool.query(
|
||||
`SELECT o.*, json_agg(oi.*) as items
|
||||
FROM orders o
|
||||
LEFT JOIN order_items oi ON o.id = oi.order_id
|
||||
WHERE o.user_id = $1
|
||||
GROUP BY o.id`,
|
||||
[userId]
|
||||
);
|
||||
return result.rows;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Problems:**
|
||||
- Business logic mixed with SQL
|
||||
- Hard to test without real database
|
||||
- Changing database requires rewriting service
|
||||
- SQL scattered across codebase
|
||||
|
||||
## Good Example
|
||||
|
||||
### Define Repository Interface
|
||||
|
||||
```typescript
|
||||
// ✅ Repository interface - contract for data access
|
||||
interface OrderRepository {
|
||||
find(id: string): Promise<Order | null>;
|
||||
findByUser(userId: string): Promise<Order[]>;
|
||||
findByStatus(status: OrderStatus): Promise<Order[]>;
|
||||
save(order: Order): Promise<Order>;
|
||||
delete(id: string): Promise<void>;
|
||||
}
|
||||
|
||||
interface OrderItemRepository {
|
||||
findByOrder(orderId: string): Promise<OrderItem[]>;
|
||||
saveMany(items: OrderItem[]): Promise<OrderItem[]>;
|
||||
deleteByOrder(orderId: string): Promise<void>;
|
||||
}
|
||||
```
|
||||
|
||||
### Implement Repository
|
||||
|
||||
```typescript
|
||||
// ✅ PostgreSQL implementation
|
||||
class PostgresOrderRepository implements OrderRepository {
|
||||
constructor(private db: Pool) {}
|
||||
|
||||
async find(id: string): Promise<Order | null> {
|
||||
const result = await this.db.query(
|
||||
'SELECT * FROM orders WHERE id = $1',
|
||||
[id]
|
||||
);
|
||||
return result.rows[0] ? this.mapToOrder(result.rows[0]) : null;
|
||||
}
|
||||
|
||||
async findByUser(userId: string): Promise<Order[]> {
|
||||
const result = await this.db.query(
|
||||
'SELECT * FROM orders WHERE user_id = $1 ORDER BY created_at DESC',
|
||||
[userId]
|
||||
);
|
||||
return result.rows.map(this.mapToOrder);
|
||||
}
|
||||
|
||||
async findByStatus(status: OrderStatus): Promise<Order[]> {
|
||||
const result = await this.db.query(
|
||||
'SELECT * FROM orders WHERE status = $1',
|
||||
[status]
|
||||
);
|
||||
return result.rows.map(this.mapToOrder);
|
||||
}
|
||||
|
||||
async save(order: Order): Promise<Order> {
|
||||
if (order.id) {
|
||||
return this.update(order);
|
||||
}
|
||||
return this.insert(order);
|
||||
}
|
||||
|
||||
private async insert(order: Order): Promise<Order> {
|
||||
const result = await this.db.query(
|
||||
`INSERT INTO orders (user_id, total, status, created_at)
|
||||
VALUES ($1, $2, $3, NOW()) RETURNING *`,
|
||||
[order.userId, order.total, order.status]
|
||||
);
|
||||
return this.mapToOrder(result.rows[0]);
|
||||
}
|
||||
|
||||
private async update(order: Order): Promise<Order> {
|
||||
const result = await this.db.query(
|
||||
`UPDATE orders SET total = $1, status = $2, updated_at = NOW()
|
||||
WHERE id = $3 RETURNING *`,
|
||||
[order.total, order.status, order.id]
|
||||
);
|
||||
return this.mapToOrder(result.rows[0]);
|
||||
}
|
||||
|
||||
async delete(id: string): Promise<void> {
|
||||
await this.db.query('DELETE FROM orders WHERE id = $1', [id]);
|
||||
}
|
||||
|
||||
private mapToOrder(row: any): Order {
|
||||
return new Order({
|
||||
id: row.id,
|
||||
userId: row.user_id,
|
||||
total: parseFloat(row.total),
|
||||
status: row.status as OrderStatus,
|
||||
createdAt: row.created_at,
|
||||
updatedAt: row.updated_at,
|
||||
});
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Clean Service Layer
|
||||
|
||||
```typescript
|
||||
// ✅ Service focuses on business logic only
|
||||
class OrderService {
|
||||
constructor(
|
||||
private orderRepository: OrderRepository,
|
||||
private orderItemRepository: OrderItemRepository,
|
||||
private productRepository: ProductRepository,
|
||||
) {}
|
||||
|
||||
async createOrder(userId: string, items: CartItem[]): Promise<Order> {
|
||||
// Pure business logic
|
||||
const total = this.calculateTotal(items);
|
||||
|
||||
const order = new Order({
|
||||
userId,
|
||||
total,
|
||||
status: OrderStatus.Pending,
|
||||
});
|
||||
|
||||
// Repository handles persistence
|
||||
const savedOrder = await this.orderRepository.save(order);
|
||||
|
||||
const orderItems = items.map(item => new OrderItem({
|
||||
orderId: savedOrder.id,
|
||||
productId: item.productId,
|
||||
quantity: item.quantity,
|
||||
price: item.price,
|
||||
}));
|
||||
|
||||
await this.orderItemRepository.saveMany(orderItems);
|
||||
|
||||
return savedOrder;
|
||||
}
|
||||
|
||||
async cancelOrder(orderId: string): Promise<Order> {
|
||||
const order = await this.orderRepository.find(orderId);
|
||||
|
||||
if (!order) {
|
||||
throw new OrderNotFoundException(orderId);
|
||||
}
|
||||
|
||||
if (!order.canBeCancelled()) {
|
||||
throw new InvalidOrderStateException('Order cannot be cancelled');
|
||||
}
|
||||
|
||||
order.cancel();
|
||||
return this.orderRepository.save(order);
|
||||
}
|
||||
|
||||
private calculateTotal(items: CartItem[]): number {
|
||||
return items.reduce((sum, item) => sum + item.price * item.quantity, 0);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### In-Memory Repository for Testing
|
||||
|
||||
```typescript
|
||||
// ✅ In-memory implementation for tests
|
||||
class InMemoryOrderRepository implements OrderRepository {
|
||||
private orders: Map<string, Order> = new Map();
|
||||
private idCounter = 1;
|
||||
|
||||
async find(id: string): Promise<Order | null> {
|
||||
return this.orders.get(id) ?? null;
|
||||
}
|
||||
|
||||
async findByUser(userId: string): Promise<Order[]> {
|
||||
return Array.from(this.orders.values())
|
||||
.filter(order => order.userId === userId);
|
||||
}
|
||||
|
||||
async findByStatus(status: OrderStatus): Promise<Order[]> {
|
||||
return Array.from(this.orders.values())
|
||||
.filter(order => order.status === status);
|
||||
}
|
||||
|
||||
async save(order: Order): Promise<Order> {
|
||||
if (!order.id) {
|
||||
order.id = String(this.idCounter++);
|
||||
}
|
||||
this.orders.set(order.id, order);
|
||||
return order;
|
||||
}
|
||||
|
||||
async delete(id: string): Promise<void> {
|
||||
this.orders.delete(id);
|
||||
}
|
||||
|
||||
// Test helper methods
|
||||
clear(): void {
|
||||
this.orders.clear();
|
||||
}
|
||||
|
||||
seed(orders: Order[]): void {
|
||||
orders.forEach(order => this.orders.set(order.id, order));
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Easy Testing
|
||||
|
||||
```typescript
|
||||
// ✅ Unit tests without database
|
||||
describe('OrderService', () => {
|
||||
let orderService: OrderService;
|
||||
let orderRepository: InMemoryOrderRepository;
|
||||
let orderItemRepository: InMemoryOrderItemRepository;
|
||||
|
||||
beforeEach(() => {
|
||||
orderRepository = new InMemoryOrderRepository();
|
||||
orderItemRepository = new InMemoryOrderItemRepository();
|
||||
orderService = new OrderService(
|
||||
orderRepository,
|
||||
orderItemRepository,
|
||||
new InMemoryProductRepository(),
|
||||
);
|
||||
});
|
||||
|
||||
it('creates order with calculated total', async () => {
|
||||
const items = [
|
||||
{ productId: '1', quantity: 2, price: 10 },
|
||||
{ productId: '2', quantity: 1, price: 25 },
|
||||
];
|
||||
|
||||
const order = await orderService.createOrder('user-1', items);
|
||||
|
||||
expect(order.total).toBe(45);
|
||||
expect(order.status).toBe(OrderStatus.Pending);
|
||||
});
|
||||
|
||||
it('cancels pending order', async () => {
|
||||
orderRepository.seed([
|
||||
new Order({ id: '1', userId: 'user-1', status: OrderStatus.Pending }),
|
||||
]);
|
||||
|
||||
const order = await orderService.cancelOrder('1');
|
||||
|
||||
expect(order.status).toBe(OrderStatus.Cancelled);
|
||||
});
|
||||
|
||||
it('throws when cancelling shipped order', async () => {
|
||||
orderRepository.seed([
|
||||
new Order({ id: '1', userId: 'user-1', status: OrderStatus.Shipped }),
|
||||
]);
|
||||
|
||||
await expect(orderService.cancelOrder('1'))
|
||||
.rejects.toThrow(InvalidOrderStateException);
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
### Generic Repository Base
|
||||
|
||||
```typescript
|
||||
// ✅ Generic base for common operations
|
||||
interface Repository<T, ID> {
|
||||
find(id: ID): Promise<T | null>;
|
||||
findAll(): Promise<T[]>;
|
||||
save(entity: T): Promise<T>;
|
||||
delete(id: ID): Promise<void>;
|
||||
exists(id: ID): Promise<boolean>;
|
||||
}
|
||||
|
||||
abstract class BasePostgresRepository<T, ID> implements Repository<T, ID> {
|
||||
constructor(
|
||||
protected db: Pool,
|
||||
protected tableName: string,
|
||||
) {}
|
||||
|
||||
async find(id: ID): Promise<T | null> {
|
||||
const result = await this.db.query(
|
||||
`SELECT * FROM ${this.tableName} WHERE id = $1`,
|
||||
[id]
|
||||
);
|
||||
return result.rows[0] ? this.mapToEntity(result.rows[0]) : null;
|
||||
}
|
||||
|
||||
async exists(id: ID): Promise<boolean> {
|
||||
const result = await this.db.query(
|
||||
`SELECT 1 FROM ${this.tableName} WHERE id = $1`,
|
||||
[id]
|
||||
);
|
||||
return result.rows.length > 0;
|
||||
}
|
||||
|
||||
protected abstract mapToEntity(row: any): T;
|
||||
}
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
- Separation of concerns
|
||||
- Business logic free of persistence details
|
||||
- Easy to test with in-memory implementations
|
||||
- Swap databases without changing services
|
||||
- Centralized query logic
|
||||
- Consistent data access patterns
|
||||
- Single place to add caching, logging
|
||||
@@ -0,0 +1,336 @@
|
||||
---
|
||||
id: solid-dip-abstractions
|
||||
title: SOLID - Dependency Inversion (Abstractions)
|
||||
category: solid-principles
|
||||
priority: critical
|
||||
tags: [SOLID, DIP, dependency-inversion, abstractions]
|
||||
related: [solid-dip-injection, solid-ocp-abstraction, pattern-repository]
|
||||
---
|
||||
|
||||
# Dependency Inversion Principle - Depend on Abstractions
|
||||
|
||||
High-level modules should not depend on low-level modules. Both should depend on abstractions. Abstractions should not depend on details; details should depend on abstractions.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: High-level module depends on low-level implementation details
|
||||
|
||||
// Low-level modules (concrete implementations)
|
||||
class MySQLDatabase {
|
||||
connect(): void {
|
||||
console.log('Connecting to MySQL...');
|
||||
}
|
||||
|
||||
query(sql: string): any[] {
|
||||
console.log(`Executing MySQL query: ${sql}`);
|
||||
return [];
|
||||
}
|
||||
|
||||
close(): void {
|
||||
console.log('Closing MySQL connection');
|
||||
}
|
||||
}
|
||||
|
||||
class SmtpEmailSender {
|
||||
send(to: string, subject: string, body: string): void {
|
||||
console.log(`Sending SMTP email to ${to}`);
|
||||
}
|
||||
}
|
||||
|
||||
class StripePaymentGateway {
|
||||
charge(amount: number, cardToken: string): string {
|
||||
console.log(`Charging $${amount} via Stripe`);
|
||||
return 'stripe_txn_123';
|
||||
}
|
||||
}
|
||||
|
||||
// High-level module directly depends on low-level implementations
|
||||
class OrderService {
|
||||
private database: MySQLDatabase;
|
||||
private emailSender: SmtpEmailSender;
|
||||
private paymentGateway: StripePaymentGateway;
|
||||
|
||||
constructor() {
|
||||
// Direct instantiation creates tight coupling
|
||||
this.database = new MySQLDatabase();
|
||||
this.emailSender = new SmtpEmailSender();
|
||||
this.paymentGateway = new StripePaymentGateway();
|
||||
}
|
||||
|
||||
async createOrder(orderData: OrderData): Promise<Order> {
|
||||
this.database.connect();
|
||||
|
||||
// MySQL-specific query
|
||||
const result = this.database.query(
|
||||
`INSERT INTO orders (customer_id, total) VALUES (${orderData.customerId}, ${orderData.total})`
|
||||
);
|
||||
|
||||
// Stripe-specific charging
|
||||
const txnId = this.paymentGateway.charge(orderData.total, orderData.cardToken);
|
||||
|
||||
// SMTP-specific sending
|
||||
this.emailSender.send(
|
||||
orderData.customerEmail,
|
||||
'Order Confirmation',
|
||||
`Your order has been placed. Transaction: ${txnId}`
|
||||
);
|
||||
|
||||
this.database.close();
|
||||
|
||||
return { id: 'order_1', ...orderData };
|
||||
}
|
||||
}
|
||||
|
||||
// Problems:
|
||||
// 1. Cannot switch to PostgreSQL without modifying OrderService
|
||||
// 2. Cannot use SendGrid for emails without modifying OrderService
|
||||
// 3. Cannot test without real MySQL, SMTP, and Stripe
|
||||
// 4. OrderService knows implementation details it shouldn't
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Depend on abstractions, not concretions
|
||||
|
||||
// Abstractions (interfaces) - owned by the high-level module
|
||||
interface Database {
|
||||
connect(): Promise<void>;
|
||||
query<T>(query: QueryBuilder): Promise<T[]>;
|
||||
execute(command: Command): Promise<void>;
|
||||
disconnect(): Promise<void>;
|
||||
}
|
||||
|
||||
interface QueryBuilder {
|
||||
table: string;
|
||||
select?: string[];
|
||||
where?: Record<string, any>;
|
||||
orderBy?: string;
|
||||
limit?: number;
|
||||
}
|
||||
|
||||
interface Command {
|
||||
table: string;
|
||||
operation: 'insert' | 'update' | 'delete';
|
||||
data?: Record<string, any>;
|
||||
where?: Record<string, any>;
|
||||
}
|
||||
|
||||
interface EmailSender {
|
||||
send(options: EmailOptions): Promise<void>;
|
||||
}
|
||||
|
||||
interface EmailOptions {
|
||||
to: string;
|
||||
subject: string;
|
||||
body: string;
|
||||
html?: string;
|
||||
}
|
||||
|
||||
interface PaymentGateway {
|
||||
charge(request: ChargeRequest): Promise<ChargeResult>;
|
||||
refund(transactionId: string, amount?: number): Promise<RefundResult>;
|
||||
}
|
||||
|
||||
interface ChargeRequest {
|
||||
amount: number;
|
||||
currency: string;
|
||||
paymentMethodId: string;
|
||||
metadata?: Record<string, string>;
|
||||
}
|
||||
|
||||
interface ChargeResult {
|
||||
transactionId: string;
|
||||
status: 'success' | 'pending' | 'failed';
|
||||
amount: number;
|
||||
}
|
||||
|
||||
// High-level module depends only on abstractions
|
||||
class OrderService {
|
||||
constructor(
|
||||
private database: Database,
|
||||
private emailSender: EmailSender,
|
||||
private paymentGateway: PaymentGateway
|
||||
) {}
|
||||
|
||||
async createOrder(orderData: OrderData): Promise<Order> {
|
||||
await this.database.connect();
|
||||
|
||||
try {
|
||||
// Uses abstraction - no SQL dialect specifics
|
||||
await this.database.execute({
|
||||
table: 'orders',
|
||||
operation: 'insert',
|
||||
data: {
|
||||
customerId: orderData.customerId,
|
||||
total: orderData.total,
|
||||
status: 'pending'
|
||||
}
|
||||
});
|
||||
|
||||
// Uses abstraction - no payment provider specifics
|
||||
const chargeResult = await this.paymentGateway.charge({
|
||||
amount: orderData.total,
|
||||
currency: 'USD',
|
||||
paymentMethodId: orderData.paymentMethodId,
|
||||
metadata: { orderId: orderData.id }
|
||||
});
|
||||
|
||||
// Uses abstraction - no email provider specifics
|
||||
await this.emailSender.send({
|
||||
to: orderData.customerEmail,
|
||||
subject: 'Order Confirmation',
|
||||
body: `Your order has been placed. Transaction: ${chargeResult.transactionId}`
|
||||
});
|
||||
|
||||
return { id: orderData.id, ...orderData, status: 'completed' };
|
||||
} finally {
|
||||
await this.database.disconnect();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Low-level implementations depend on abstractions
|
||||
class MySQLDatabase implements Database {
|
||||
private connection: any;
|
||||
|
||||
async connect(): Promise<void> {
|
||||
this.connection = await mysql.createConnection(config);
|
||||
}
|
||||
|
||||
async query<T>(query: QueryBuilder): Promise<T[]> {
|
||||
const sql = this.buildSelectQuery(query);
|
||||
const [rows] = await this.connection.execute(sql);
|
||||
return rows as T[];
|
||||
}
|
||||
|
||||
async execute(command: Command): Promise<void> {
|
||||
const sql = this.buildCommandQuery(command);
|
||||
await this.connection.execute(sql);
|
||||
}
|
||||
|
||||
async disconnect(): Promise<void> {
|
||||
await this.connection.end();
|
||||
}
|
||||
|
||||
private buildSelectQuery(query: QueryBuilder): string {
|
||||
// MySQL-specific query building
|
||||
return `SELECT ${query.select?.join(', ') ?? '*'} FROM ${query.table}`;
|
||||
}
|
||||
|
||||
private buildCommandQuery(command: Command): string {
|
||||
// MySQL-specific command building
|
||||
return `INSERT INTO ${command.table} ...`;
|
||||
}
|
||||
}
|
||||
|
||||
class PostgreSQLDatabase implements Database {
|
||||
// PostgreSQL-specific implementation of the same interface
|
||||
async connect(): Promise<void> { /* PostgreSQL connection */ }
|
||||
async query<T>(query: QueryBuilder): Promise<T[]> { /* PostgreSQL query */ }
|
||||
async execute(command: Command): Promise<void> { /* PostgreSQL execute */ }
|
||||
async disconnect(): Promise<void> { /* PostgreSQL disconnect */ }
|
||||
}
|
||||
|
||||
class SendGridEmailSender implements EmailSender {
|
||||
constructor(private apiKey: string) {}
|
||||
|
||||
async send(options: EmailOptions): Promise<void> {
|
||||
await sendgrid.send({
|
||||
to: options.to,
|
||||
from: 'noreply@example.com',
|
||||
subject: options.subject,
|
||||
text: options.body,
|
||||
html: options.html
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
class StripePaymentGateway implements PaymentGateway {
|
||||
constructor(private stripe: Stripe) {}
|
||||
|
||||
async charge(request: ChargeRequest): Promise<ChargeResult> {
|
||||
const intent = await this.stripe.paymentIntents.create({
|
||||
amount: request.amount * 100,
|
||||
currency: request.currency,
|
||||
payment_method: request.paymentMethodId,
|
||||
confirm: true
|
||||
});
|
||||
|
||||
return {
|
||||
transactionId: intent.id,
|
||||
status: intent.status === 'succeeded' ? 'success' : 'pending',
|
||||
amount: request.amount
|
||||
};
|
||||
}
|
||||
|
||||
async refund(transactionId: string, amount?: number): Promise<RefundResult> {
|
||||
const refund = await this.stripe.refunds.create({
|
||||
payment_intent: transactionId,
|
||||
amount: amount ? amount * 100 : undefined
|
||||
});
|
||||
return { refundId: refund.id, status: 'success' };
|
||||
}
|
||||
}
|
||||
|
||||
// Composition root - where we wire everything together
|
||||
function createOrderService(): OrderService {
|
||||
const database = new PostgreSQLDatabase();
|
||||
const emailSender = new SendGridEmailSender(process.env.SENDGRID_API_KEY!);
|
||||
const paymentGateway = new StripePaymentGateway(new Stripe(process.env.STRIPE_KEY));
|
||||
|
||||
return new OrderService(database, emailSender, paymentGateway);
|
||||
}
|
||||
|
||||
// Testing with mock implementations
|
||||
class MockDatabase implements Database {
|
||||
public queries: QueryBuilder[] = [];
|
||||
public commands: Command[] = [];
|
||||
|
||||
async connect(): Promise<void> {}
|
||||
async query<T>(query: QueryBuilder): Promise<T[]> {
|
||||
this.queries.push(query);
|
||||
return [];
|
||||
}
|
||||
async execute(command: Command): Promise<void> {
|
||||
this.commands.push(command);
|
||||
}
|
||||
async disconnect(): Promise<void> {}
|
||||
}
|
||||
|
||||
// Test without real databases, email servers, or payment gateways
|
||||
describe('OrderService', () => {
|
||||
it('should create order', async () => {
|
||||
const mockDb = new MockDatabase();
|
||||
const mockEmail: EmailSender = { send: jest.fn() };
|
||||
const mockPayment: PaymentGateway = {
|
||||
charge: jest.fn().mockResolvedValue({ transactionId: 'test_txn', status: 'success', amount: 100 }),
|
||||
refund: jest.fn()
|
||||
};
|
||||
|
||||
const service = new OrderService(mockDb, mockEmail, mockPayment);
|
||||
await service.createOrder(testOrderData);
|
||||
|
||||
expect(mockDb.commands).toHaveLength(1);
|
||||
expect(mockPayment.charge).toHaveBeenCalled();
|
||||
expect(mockEmail.send).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Loose Coupling**: `OrderService` doesn't know or care about MySQL, Stripe, or SendGrid. It works with any implementation.
|
||||
|
||||
2. **Easy Swapping**: Switch from MySQL to PostgreSQL by providing a different implementation. No changes to `OrderService`.
|
||||
|
||||
3. **Testability**: Test with mock implementations - no real databases or external services needed.
|
||||
|
||||
4. **Parallel Development**: Teams can work on different implementations simultaneously against the same interface.
|
||||
|
||||
5. **Policy/Detail Separation**: Business rules (OrderService) are separate from infrastructure details (MySQLDatabase).
|
||||
|
||||
6. **Stable Architecture**: Abstractions change less frequently than implementations. The core is protected from change.
|
||||
|
||||
7. **Plugin Architecture**: New implementations can be added without modifying existing code.
|
||||
@@ -0,0 +1,349 @@
|
||||
---
|
||||
id: solid-dip-injection
|
||||
title: SOLID - Dependency Inversion (Injection)
|
||||
category: solid-principles
|
||||
priority: critical
|
||||
tags: [SOLID, DIP, dependency-injection, testability]
|
||||
related: [solid-dip-abstractions, solid-srp-class, core-composition]
|
||||
---
|
||||
|
||||
# Dependency Inversion Principle - Dependency Injection
|
||||
|
||||
Dependencies should be injected from outside rather than created inside a class. This enables loose coupling, testability, and flexibility in how dependencies are provided.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Creating dependencies inside the class
|
||||
|
||||
class UserRegistrationService {
|
||||
private userRepository: UserRepository;
|
||||
private passwordHasher: PasswordHasher;
|
||||
private emailService: EmailService;
|
||||
private logger: Logger;
|
||||
private analyticsService: AnalyticsService;
|
||||
|
||||
constructor() {
|
||||
// Dependencies created inside - tight coupling
|
||||
this.userRepository = new PostgresUserRepository(
|
||||
new PostgresConnection('localhost', 5432, 'mydb')
|
||||
);
|
||||
this.passwordHasher = new BcryptPasswordHasher(10);
|
||||
this.emailService = new SendGridEmailService(process.env.SENDGRID_KEY!);
|
||||
this.logger = new WinstonLogger('UserRegistration');
|
||||
this.analyticsService = new MixpanelAnalytics(process.env.MIXPANEL_TOKEN!);
|
||||
}
|
||||
|
||||
async register(email: string, password: string): Promise<User> {
|
||||
this.logger.info(`Registering user: ${email}`);
|
||||
|
||||
const existingUser = await this.userRepository.findByEmail(email);
|
||||
if (existingUser) {
|
||||
throw new Error('User already exists');
|
||||
}
|
||||
|
||||
const hashedPassword = await this.passwordHasher.hash(password);
|
||||
const user = await this.userRepository.create({ email, password: hashedPassword });
|
||||
|
||||
await this.emailService.send({
|
||||
to: email,
|
||||
subject: 'Welcome!',
|
||||
body: 'Thanks for registering'
|
||||
});
|
||||
|
||||
await this.analyticsService.track('user_registered', { userId: user.id });
|
||||
|
||||
return user;
|
||||
}
|
||||
}
|
||||
|
||||
// Problems:
|
||||
// 1. Cannot test without real Postgres, SendGrid, Mixpanel
|
||||
// 2. Cannot reuse with different implementations
|
||||
// 3. Hard-coded configuration scattered through constructors
|
||||
// 4. Class responsible for creating its dependencies
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Dependencies injected from outside
|
||||
|
||||
// Interfaces for all dependencies
|
||||
interface UserRepository {
|
||||
findByEmail(email: string): Promise<User | null>;
|
||||
findById(id: string): Promise<User | null>;
|
||||
create(data: CreateUserData): Promise<User>;
|
||||
update(id: string, data: Partial<User>): Promise<User>;
|
||||
}
|
||||
|
||||
interface PasswordHasher {
|
||||
hash(password: string): Promise<string>;
|
||||
verify(password: string, hash: string): Promise<boolean>;
|
||||
}
|
||||
|
||||
interface EmailService {
|
||||
send(options: EmailOptions): Promise<void>;
|
||||
}
|
||||
|
||||
interface Logger {
|
||||
info(message: string, meta?: Record<string, any>): void;
|
||||
error(message: string, error?: Error, meta?: Record<string, any>): void;
|
||||
warn(message: string, meta?: Record<string, any>): void;
|
||||
}
|
||||
|
||||
interface AnalyticsService {
|
||||
track(event: string, properties?: Record<string, any>): Promise<void>;
|
||||
identify(userId: string, traits?: Record<string, any>): Promise<void>;
|
||||
}
|
||||
|
||||
// Service with constructor injection
|
||||
class UserRegistrationService {
|
||||
constructor(
|
||||
private readonly userRepository: UserRepository,
|
||||
private readonly passwordHasher: PasswordHasher,
|
||||
private readonly emailService: EmailService,
|
||||
private readonly logger: Logger,
|
||||
private readonly analyticsService: AnalyticsService
|
||||
) {}
|
||||
|
||||
async register(email: string, password: string): Promise<User> {
|
||||
this.logger.info('Registering user', { email });
|
||||
|
||||
const existingUser = await this.userRepository.findByEmail(email);
|
||||
if (existingUser) {
|
||||
throw new UserAlreadyExistsError(email);
|
||||
}
|
||||
|
||||
const hashedPassword = await this.passwordHasher.hash(password);
|
||||
const user = await this.userRepository.create({
|
||||
email,
|
||||
password: hashedPassword,
|
||||
createdAt: new Date()
|
||||
});
|
||||
|
||||
await this.sendWelcomeEmail(user);
|
||||
await this.trackRegistration(user);
|
||||
|
||||
this.logger.info('User registered successfully', { userId: user.id });
|
||||
return user;
|
||||
}
|
||||
|
||||
private async sendWelcomeEmail(user: User): Promise<void> {
|
||||
try {
|
||||
await this.emailService.send({
|
||||
to: user.email,
|
||||
subject: 'Welcome!',
|
||||
body: 'Thanks for registering'
|
||||
});
|
||||
} catch (error) {
|
||||
this.logger.error('Failed to send welcome email', error as Error, { userId: user.id });
|
||||
// Don't fail registration if email fails
|
||||
}
|
||||
}
|
||||
|
||||
private async trackRegistration(user: User): Promise<void> {
|
||||
try {
|
||||
await this.analyticsService.identify(user.id, { email: user.email });
|
||||
await this.analyticsService.track('user_registered', { userId: user.id });
|
||||
} catch (error) {
|
||||
this.logger.error('Failed to track registration', error as Error, { userId: user.id });
|
||||
// Don't fail registration if analytics fails
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Concrete implementations
|
||||
class PostgresUserRepository implements UserRepository {
|
||||
constructor(private db: DatabaseConnection) {}
|
||||
|
||||
async findByEmail(email: string): Promise<User | null> {
|
||||
const result = await this.db.query('SELECT * FROM users WHERE email = $1', [email]);
|
||||
return result.rows[0] || null;
|
||||
}
|
||||
|
||||
async findById(id: string): Promise<User | null> {
|
||||
const result = await this.db.query('SELECT * FROM users WHERE id = $1', [id]);
|
||||
return result.rows[0] || null;
|
||||
}
|
||||
|
||||
async create(data: CreateUserData): Promise<User> {
|
||||
const result = await this.db.query(
|
||||
'INSERT INTO users (email, password, created_at) VALUES ($1, $2, $3) RETURNING *',
|
||||
[data.email, data.password, data.createdAt]
|
||||
);
|
||||
return result.rows[0];
|
||||
}
|
||||
|
||||
async update(id: string, data: Partial<User>): Promise<User> {
|
||||
// Implementation
|
||||
}
|
||||
}
|
||||
|
||||
class BcryptPasswordHasher implements PasswordHasher {
|
||||
constructor(private rounds: number = 10) {}
|
||||
|
||||
async hash(password: string): Promise<string> {
|
||||
return bcrypt.hash(password, this.rounds);
|
||||
}
|
||||
|
||||
async verify(password: string, hash: string): Promise<boolean> {
|
||||
return bcrypt.compare(password, hash);
|
||||
}
|
||||
}
|
||||
|
||||
// Dependency Injection Container (manual)
|
||||
class Container {
|
||||
private services: Map<string, any> = new Map();
|
||||
|
||||
register<T>(key: string, factory: () => T): void {
|
||||
this.services.set(key, factory);
|
||||
}
|
||||
|
||||
resolve<T>(key: string): T {
|
||||
const factory = this.services.get(key);
|
||||
if (!factory) {
|
||||
throw new Error(`Service not registered: ${key}`);
|
||||
}
|
||||
return factory();
|
||||
}
|
||||
}
|
||||
|
||||
// Composition root - wire up all dependencies
|
||||
function configureContainer(): Container {
|
||||
const container = new Container();
|
||||
|
||||
// Register infrastructure
|
||||
container.register('DatabaseConnection', () =>
|
||||
new PostgresConnection(process.env.DATABASE_URL!)
|
||||
);
|
||||
|
||||
container.register('Logger', () =>
|
||||
new WinstonLogger({ level: process.env.LOG_LEVEL || 'info' })
|
||||
);
|
||||
|
||||
// Register repositories
|
||||
container.register('UserRepository', () =>
|
||||
new PostgresUserRepository(container.resolve('DatabaseConnection'))
|
||||
);
|
||||
|
||||
// Register services
|
||||
container.register('PasswordHasher', () =>
|
||||
new BcryptPasswordHasher(12)
|
||||
);
|
||||
|
||||
container.register('EmailService', () =>
|
||||
new SendGridEmailService(process.env.SENDGRID_API_KEY!)
|
||||
);
|
||||
|
||||
container.register('AnalyticsService', () =>
|
||||
new MixpanelAnalytics(process.env.MIXPANEL_TOKEN!)
|
||||
);
|
||||
|
||||
// Register application services
|
||||
container.register('UserRegistrationService', () =>
|
||||
new UserRegistrationService(
|
||||
container.resolve('UserRepository'),
|
||||
container.resolve('PasswordHasher'),
|
||||
container.resolve('EmailService'),
|
||||
container.resolve('Logger'),
|
||||
container.resolve('AnalyticsService')
|
||||
)
|
||||
);
|
||||
|
||||
return container;
|
||||
}
|
||||
|
||||
// Application startup
|
||||
const container = configureContainer();
|
||||
const registrationService = container.resolve<UserRegistrationService>('UserRegistrationService');
|
||||
|
||||
// Testing is now trivial with mock implementations
|
||||
describe('UserRegistrationService', () => {
|
||||
let service: UserRegistrationService;
|
||||
let mockUserRepository: jest.Mocked<UserRepository>;
|
||||
let mockPasswordHasher: jest.Mocked<PasswordHasher>;
|
||||
let mockEmailService: jest.Mocked<EmailService>;
|
||||
let mockLogger: jest.Mocked<Logger>;
|
||||
let mockAnalytics: jest.Mocked<AnalyticsService>;
|
||||
|
||||
beforeEach(() => {
|
||||
mockUserRepository = {
|
||||
findByEmail: jest.fn(),
|
||||
findById: jest.fn(),
|
||||
create: jest.fn(),
|
||||
update: jest.fn()
|
||||
};
|
||||
mockPasswordHasher = {
|
||||
hash: jest.fn(),
|
||||
verify: jest.fn()
|
||||
};
|
||||
mockEmailService = { send: jest.fn() };
|
||||
mockLogger = { info: jest.fn(), error: jest.fn(), warn: jest.fn() };
|
||||
mockAnalytics = { track: jest.fn(), identify: jest.fn() };
|
||||
|
||||
service = new UserRegistrationService(
|
||||
mockUserRepository,
|
||||
mockPasswordHasher,
|
||||
mockEmailService,
|
||||
mockLogger,
|
||||
mockAnalytics
|
||||
);
|
||||
});
|
||||
|
||||
it('should register a new user', async () => {
|
||||
mockUserRepository.findByEmail.mockResolvedValue(null);
|
||||
mockPasswordHasher.hash.mockResolvedValue('hashed_password');
|
||||
mockUserRepository.create.mockResolvedValue({
|
||||
id: '1',
|
||||
email: 'test@example.com',
|
||||
password: 'hashed_password',
|
||||
createdAt: new Date()
|
||||
});
|
||||
|
||||
const user = await service.register('test@example.com', 'password123');
|
||||
|
||||
expect(user.id).toBe('1');
|
||||
expect(mockPasswordHasher.hash).toHaveBeenCalledWith('password123');
|
||||
expect(mockEmailService.send).toHaveBeenCalled();
|
||||
expect(mockAnalytics.track).toHaveBeenCalledWith('user_registered', { userId: '1' });
|
||||
});
|
||||
|
||||
it('should throw error if user already exists', async () => {
|
||||
mockUserRepository.findByEmail.mockResolvedValue({ id: '1', email: 'test@example.com' } as User);
|
||||
|
||||
await expect(service.register('test@example.com', 'password123'))
|
||||
.rejects.toThrow(UserAlreadyExistsError);
|
||||
|
||||
expect(mockUserRepository.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not fail registration if email sending fails', async () => {
|
||||
mockUserRepository.findByEmail.mockResolvedValue(null);
|
||||
mockPasswordHasher.hash.mockResolvedValue('hashed_password');
|
||||
mockUserRepository.create.mockResolvedValue({ id: '1', email: 'test@example.com' } as User);
|
||||
mockEmailService.send.mockRejectedValue(new Error('SMTP error'));
|
||||
|
||||
const user = await service.register('test@example.com', 'password123');
|
||||
|
||||
expect(user.id).toBe('1');
|
||||
expect(mockLogger.error).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Testability**: All dependencies can be mocked. Tests run fast without external services.
|
||||
|
||||
2. **Flexibility**: Switch implementations at configuration time, not code change time.
|
||||
|
||||
3. **Single Responsibility**: Classes focus on business logic, not on constructing dependencies.
|
||||
|
||||
4. **Configuration Centralization**: All wiring happens in one place (composition root), making it easy to see and change.
|
||||
|
||||
5. **Lifetime Management**: The container can manage singleton vs. transient instances.
|
||||
|
||||
6. **Environment Adaptation**: Easy to provide different implementations for dev, test, staging, and production.
|
||||
|
||||
7. **Explicit Dependencies**: Looking at the constructor tells you exactly what a class needs to function.
|
||||
246
.agents/skills/clean-code-principles/rules/solid-isp-clients.md
Normal file
246
.agents/skills/clean-code-principles/rules/solid-isp-clients.md
Normal file
@@ -0,0 +1,246 @@
|
||||
---
|
||||
id: solid-isp-clients
|
||||
title: SOLID - Interface Segregation (Client-Specific)
|
||||
category: solid-principles
|
||||
priority: critical
|
||||
tags: [SOLID, ISP, interface-segregation, client-design]
|
||||
related: [solid-isp-interfaces, solid-srp-class, solid-lsp-contracts]
|
||||
---
|
||||
|
||||
# Interface Segregation Principle - Client-Specific Interfaces
|
||||
|
||||
Clients should not be forced to depend on interfaces they do not use. Design interfaces from the client's perspective, not the implementation's.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Fat interface that forces clients to depend on methods they don't use
|
||||
|
||||
interface UserService {
|
||||
// Authentication
|
||||
login(email: string, password: string): Promise<AuthToken>;
|
||||
logout(userId: string): Promise<void>;
|
||||
refreshToken(token: string): Promise<AuthToken>;
|
||||
validateToken(token: string): Promise<boolean>;
|
||||
|
||||
// User management
|
||||
createUser(data: CreateUserData): Promise<User>;
|
||||
updateUser(id: string, data: UpdateUserData): Promise<User>;
|
||||
deleteUser(id: string): Promise<void>;
|
||||
getUser(id: string): Promise<User>;
|
||||
listUsers(filters: UserFilters): Promise<User[]>;
|
||||
|
||||
// Profile
|
||||
updateProfile(userId: string, profile: ProfileData): Promise<Profile>;
|
||||
uploadAvatar(userId: string, image: Buffer): Promise<string>;
|
||||
getProfile(userId: string): Promise<Profile>;
|
||||
|
||||
// Notifications
|
||||
sendNotification(userId: string, notification: Notification): Promise<void>;
|
||||
getNotificationPreferences(userId: string): Promise<NotificationPrefs>;
|
||||
updateNotificationPreferences(userId: string, prefs: NotificationPrefs): Promise<void>;
|
||||
|
||||
// Analytics
|
||||
trackUserEvent(userId: string, event: AnalyticsEvent): Promise<void>;
|
||||
getUserAnalytics(userId: string): Promise<UserAnalytics>;
|
||||
}
|
||||
|
||||
// Login page only needs authentication but must depend on entire interface
|
||||
class LoginPage {
|
||||
constructor(private userService: UserService) {} // Depends on 15+ methods!
|
||||
|
||||
async handleLogin(email: string, password: string): Promise<void> {
|
||||
// Only uses 1 method
|
||||
const token = await this.userService.login(email, password);
|
||||
this.storeToken(token);
|
||||
}
|
||||
}
|
||||
|
||||
// Profile component forced to depend on authentication and analytics
|
||||
class ProfileEditor {
|
||||
constructor(private userService: UserService) {} // Depends on 15+ methods!
|
||||
|
||||
async saveProfile(userId: string, data: ProfileData): Promise<void> {
|
||||
// Only uses 2 methods
|
||||
await this.userService.updateProfile(userId, data);
|
||||
const profile = await this.userService.getProfile(userId);
|
||||
}
|
||||
}
|
||||
|
||||
// Mock implementation nightmare for testing
|
||||
class MockUserService implements UserService {
|
||||
// Must implement ALL methods even for simple tests
|
||||
login = jest.fn();
|
||||
logout = jest.fn();
|
||||
refreshToken = jest.fn();
|
||||
validateToken = jest.fn();
|
||||
createUser = jest.fn();
|
||||
updateUser = jest.fn();
|
||||
deleteUser = jest.fn();
|
||||
getUser = jest.fn();
|
||||
listUsers = jest.fn();
|
||||
updateProfile = jest.fn();
|
||||
uploadAvatar = jest.fn();
|
||||
getProfile = jest.fn();
|
||||
sendNotification = jest.fn();
|
||||
getNotificationPreferences = jest.fn();
|
||||
updateNotificationPreferences = jest.fn();
|
||||
trackUserEvent = jest.fn();
|
||||
getUserAnalytics = jest.fn();
|
||||
}
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Client-specific interfaces
|
||||
|
||||
// Authentication client interface
|
||||
interface Authenticator {
|
||||
login(email: string, password: string): Promise<AuthToken>;
|
||||
logout(userId: string): Promise<void>;
|
||||
refreshToken(token: string): Promise<AuthToken>;
|
||||
validateToken(token: string): Promise<boolean>;
|
||||
}
|
||||
|
||||
// User management client interface
|
||||
interface UserManager {
|
||||
createUser(data: CreateUserData): Promise<User>;
|
||||
updateUser(id: string, data: UpdateUserData): Promise<User>;
|
||||
deleteUser(id: string): Promise<void>;
|
||||
getUser(id: string): Promise<User>;
|
||||
listUsers(filters: UserFilters): Promise<User[]>;
|
||||
}
|
||||
|
||||
// Profile client interface
|
||||
interface ProfileManager {
|
||||
updateProfile(userId: string, profile: ProfileData): Promise<Profile>;
|
||||
uploadAvatar(userId: string, image: Buffer): Promise<string>;
|
||||
getProfile(userId: string): Promise<Profile>;
|
||||
}
|
||||
|
||||
// Notification client interface
|
||||
interface NotificationManager {
|
||||
sendNotification(userId: string, notification: Notification): Promise<void>;
|
||||
getNotificationPreferences(userId: string): Promise<NotificationPrefs>;
|
||||
updateNotificationPreferences(userId: string, prefs: NotificationPrefs): Promise<void>;
|
||||
}
|
||||
|
||||
// Analytics client interface
|
||||
interface UserAnalyticsTracker {
|
||||
trackUserEvent(userId: string, event: AnalyticsEvent): Promise<void>;
|
||||
getUserAnalytics(userId: string): Promise<UserAnalytics>;
|
||||
}
|
||||
|
||||
// Read-only user lookup for components that only need to fetch
|
||||
interface UserLookup {
|
||||
getUser(id: string): Promise<User>;
|
||||
getProfile(userId: string): Promise<Profile>;
|
||||
}
|
||||
|
||||
// Login page depends only on what it needs
|
||||
class LoginPage {
|
||||
constructor(private auth: Authenticator) {} // Only 4 methods
|
||||
|
||||
async handleLogin(email: string, password: string): Promise<void> {
|
||||
const token = await this.auth.login(email, password);
|
||||
this.storeToken(token);
|
||||
}
|
||||
}
|
||||
|
||||
// Profile editor depends only on profile operations
|
||||
class ProfileEditor {
|
||||
constructor(private profileManager: ProfileManager) {} // Only 3 methods
|
||||
|
||||
async saveProfile(userId: string, data: ProfileData): Promise<void> {
|
||||
await this.profileManager.updateProfile(userId, data);
|
||||
}
|
||||
|
||||
async loadProfile(userId: string): Promise<Profile> {
|
||||
return this.profileManager.getProfile(userId);
|
||||
}
|
||||
}
|
||||
|
||||
// User display component only needs read access
|
||||
class UserCard {
|
||||
constructor(private userLookup: UserLookup) {} // Only 2 methods
|
||||
|
||||
async render(userId: string): Promise<void> {
|
||||
const user = await this.userLookup.getUser(userId);
|
||||
const profile = await this.userLookup.getProfile(userId);
|
||||
// Render user card
|
||||
}
|
||||
}
|
||||
|
||||
// Admin panel needs user management
|
||||
class AdminUserPanel {
|
||||
constructor(private userManager: UserManager) {} // Only 5 methods
|
||||
|
||||
async deleteUserAccount(id: string): Promise<void> {
|
||||
await this.userManager.deleteUser(id);
|
||||
}
|
||||
}
|
||||
|
||||
// Implementation can still implement multiple interfaces
|
||||
class UserServiceImpl implements
|
||||
Authenticator,
|
||||
UserManager,
|
||||
ProfileManager,
|
||||
NotificationManager,
|
||||
UserAnalyticsTracker,
|
||||
UserLookup
|
||||
{
|
||||
// Implement all methods...
|
||||
async login(email: string, password: string): Promise<AuthToken> { /* ... */ }
|
||||
async logout(userId: string): Promise<void> { /* ... */ }
|
||||
// ... rest of implementation
|
||||
}
|
||||
|
||||
// Testing is now simple - only mock what you need
|
||||
describe('LoginPage', () => {
|
||||
it('should login user', async () => {
|
||||
const mockAuth: Authenticator = {
|
||||
login: jest.fn().mockResolvedValue({ token: 'abc123' }),
|
||||
logout: jest.fn(),
|
||||
refreshToken: jest.fn(),
|
||||
validateToken: jest.fn()
|
||||
};
|
||||
|
||||
const loginPage = new LoginPage(mockAuth);
|
||||
await loginPage.handleLogin('test@example.com', 'password');
|
||||
|
||||
expect(mockAuth.login).toHaveBeenCalledWith('test@example.com', 'password');
|
||||
});
|
||||
});
|
||||
|
||||
describe('ProfileEditor', () => {
|
||||
it('should save profile', async () => {
|
||||
const mockProfileManager: ProfileManager = {
|
||||
updateProfile: jest.fn().mockResolvedValue({}),
|
||||
uploadAvatar: jest.fn(),
|
||||
getProfile: jest.fn()
|
||||
};
|
||||
|
||||
const editor = new ProfileEditor(mockProfileManager);
|
||||
await editor.saveProfile('user1', { name: 'John' });
|
||||
|
||||
expect(mockProfileManager.updateProfile).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Minimal Dependencies**: Each client depends only on the methods it actually uses, reducing coupling.
|
||||
|
||||
2. **Easier Testing**: Mocks are small and focused. Testing `LoginPage` requires mocking 4 methods, not 15+.
|
||||
|
||||
3. **Better Encapsulation**: Clients can't accidentally call methods they shouldn't have access to.
|
||||
|
||||
4. **Clearer Intent**: Interface names describe what clients need: `Authenticator`, `ProfileManager`, `UserLookup`.
|
||||
|
||||
5. **Independent Evolution**: Changes to analytics don't affect authentication clients. Interfaces can evolve separately.
|
||||
|
||||
6. **Flexible Composition**: Different implementations can provide different subsets of functionality.
|
||||
|
||||
7. **Single Responsibility**: Each interface has a focused responsibility, making the system easier to understand.
|
||||
@@ -0,0 +1,286 @@
|
||||
---
|
||||
id: solid-isp-interfaces
|
||||
title: SOLID - Interface Segregation (Small Interfaces)
|
||||
category: solid-principles
|
||||
priority: critical
|
||||
tags: [SOLID, ISP, interface-segregation, cohesion]
|
||||
related: [solid-isp-clients, solid-srp-class, core-separation-concerns]
|
||||
---
|
||||
|
||||
# Interface Segregation Principle - Small Cohesive Interfaces
|
||||
|
||||
Interfaces should be small and cohesive, grouping only closely related methods. Split large interfaces into smaller, more focused ones.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Large interface with unrelated methods grouped together
|
||||
|
||||
interface Document {
|
||||
// Content operations
|
||||
getContent(): string;
|
||||
setContent(content: string): void;
|
||||
appendContent(content: string): void;
|
||||
|
||||
// Persistence operations
|
||||
save(): Promise<void>;
|
||||
load(id: string): Promise<void>;
|
||||
delete(): Promise<void>;
|
||||
|
||||
// Export operations
|
||||
exportToPdf(): Promise<Buffer>;
|
||||
exportToWord(): Promise<Buffer>;
|
||||
exportToHtml(): Promise<string>;
|
||||
|
||||
// Collaboration operations
|
||||
share(userId: string): Promise<void>;
|
||||
getCollaborators(): Promise<User[]>;
|
||||
addComment(comment: Comment): Promise<void>;
|
||||
getComments(): Promise<Comment[]>;
|
||||
|
||||
// Version control
|
||||
createVersion(): Promise<Version>;
|
||||
getVersionHistory(): Promise<Version[]>;
|
||||
revertToVersion(versionId: string): Promise<void>;
|
||||
|
||||
// Permissions
|
||||
setPermissions(permissions: Permissions): Promise<void>;
|
||||
getPermissions(): Promise<Permissions>;
|
||||
checkPermission(userId: string, action: string): Promise<boolean>;
|
||||
}
|
||||
|
||||
// Simple note-taking app forced to implement everything
|
||||
class SimpleNote implements Document {
|
||||
private content: string = '';
|
||||
|
||||
getContent(): string { return this.content; }
|
||||
setContent(content: string): void { this.content = content; }
|
||||
appendContent(content: string): void { this.content += content; }
|
||||
|
||||
// Forced to implement methods it doesn't need
|
||||
async save(): Promise<void> { throw new Error('Not supported'); }
|
||||
async load(id: string): Promise<void> { throw new Error('Not supported'); }
|
||||
async delete(): Promise<void> { throw new Error('Not supported'); }
|
||||
|
||||
async exportToPdf(): Promise<Buffer> { throw new Error('Not supported'); }
|
||||
async exportToWord(): Promise<Buffer> { throw new Error('Not supported'); }
|
||||
async exportToHtml(): Promise<string> { throw new Error('Not supported'); }
|
||||
|
||||
async share(userId: string): Promise<void> { throw new Error('Not supported'); }
|
||||
async getCollaborators(): Promise<User[]> { throw new Error('Not supported'); }
|
||||
async addComment(comment: Comment): Promise<void> { throw new Error('Not supported'); }
|
||||
async getComments(): Promise<Comment[]> { throw new Error('Not supported'); }
|
||||
|
||||
async createVersion(): Promise<Version> { throw new Error('Not supported'); }
|
||||
async getVersionHistory(): Promise<Version[]> { throw new Error('Not supported'); }
|
||||
async revertToVersion(versionId: string): Promise<void> { throw new Error('Not supported'); }
|
||||
|
||||
async setPermissions(permissions: Permissions): Promise<void> { throw new Error('Not supported'); }
|
||||
async getPermissions(): Promise<Permissions> { throw new Error('Not supported'); }
|
||||
async checkPermission(userId: string, action: string): Promise<boolean> { throw new Error('Not supported'); }
|
||||
}
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Small, cohesive interfaces
|
||||
|
||||
// Core content interface - the essence of a document
|
||||
interface DocumentContent {
|
||||
getContent(): string;
|
||||
setContent(content: string): void;
|
||||
}
|
||||
|
||||
// Extended content operations
|
||||
interface EditableContent extends DocumentContent {
|
||||
appendContent(content: string): void;
|
||||
insertContent(position: number, content: string): void;
|
||||
deleteRange(start: number, end: number): void;
|
||||
}
|
||||
|
||||
// Persistence operations
|
||||
interface Persistable {
|
||||
save(): Promise<void>;
|
||||
load(id: string): Promise<void>;
|
||||
}
|
||||
|
||||
interface Deletable {
|
||||
delete(): Promise<void>;
|
||||
}
|
||||
|
||||
// Export capabilities
|
||||
interface PdfExportable {
|
||||
exportToPdf(): Promise<Buffer>;
|
||||
}
|
||||
|
||||
interface WordExportable {
|
||||
exportToWord(): Promise<Buffer>;
|
||||
}
|
||||
|
||||
interface HtmlExportable {
|
||||
exportToHtml(): Promise<string>;
|
||||
}
|
||||
|
||||
// Combine export interfaces when needed
|
||||
interface FullyExportable extends PdfExportable, WordExportable, HtmlExportable {}
|
||||
|
||||
// Collaboration interfaces
|
||||
interface Shareable {
|
||||
share(userId: string, permission: Permission): Promise<void>;
|
||||
unshare(userId: string): Promise<void>;
|
||||
getCollaborators(): Promise<Collaborator[]>;
|
||||
}
|
||||
|
||||
interface Commentable {
|
||||
addComment(comment: Comment): Promise<void>;
|
||||
removeComment(commentId: string): Promise<void>;
|
||||
getComments(): Promise<Comment[]>;
|
||||
}
|
||||
|
||||
// Version control
|
||||
interface Versionable {
|
||||
createVersion(message?: string): Promise<Version>;
|
||||
getVersionHistory(): Promise<Version[]>;
|
||||
revertToVersion(versionId: string): Promise<void>;
|
||||
}
|
||||
|
||||
// Permissions
|
||||
interface PermissionControlled {
|
||||
setPermissions(permissions: Permissions): Promise<void>;
|
||||
getPermissions(): Promise<Permissions>;
|
||||
checkPermission(userId: string, action: string): Promise<boolean>;
|
||||
}
|
||||
|
||||
// Simple note only implements what it needs
|
||||
class SimpleNote implements DocumentContent {
|
||||
private content: string = '';
|
||||
|
||||
getContent(): string {
|
||||
return this.content;
|
||||
}
|
||||
|
||||
setContent(content: string): void {
|
||||
this.content = content;
|
||||
}
|
||||
}
|
||||
|
||||
// Persistent note adds storage
|
||||
class PersistentNote implements EditableContent, Persistable {
|
||||
private content: string = '';
|
||||
private id: string | null = null;
|
||||
|
||||
constructor(private storage: StorageService) {}
|
||||
|
||||
getContent(): string { return this.content; }
|
||||
setContent(content: string): void { this.content = content; }
|
||||
appendContent(content: string): void { this.content += content; }
|
||||
insertContent(position: number, content: string): void {
|
||||
this.content = this.content.slice(0, position) + content + this.content.slice(position);
|
||||
}
|
||||
deleteRange(start: number, end: number): void {
|
||||
this.content = this.content.slice(0, start) + this.content.slice(end);
|
||||
}
|
||||
|
||||
async save(): Promise<void> {
|
||||
this.id = await this.storage.save(this.content);
|
||||
}
|
||||
|
||||
async load(id: string): Promise<void> {
|
||||
this.content = await this.storage.load(id);
|
||||
this.id = id;
|
||||
}
|
||||
}
|
||||
|
||||
// Full-featured collaborative document
|
||||
class CollaborativeDocument implements
|
||||
EditableContent,
|
||||
Persistable,
|
||||
Deletable,
|
||||
FullyExportable,
|
||||
Shareable,
|
||||
Commentable,
|
||||
Versionable,
|
||||
PermissionControlled
|
||||
{
|
||||
constructor(
|
||||
private storage: StorageService,
|
||||
private exportService: ExportService,
|
||||
private collaborationService: CollaborationService,
|
||||
private versionService: VersionService,
|
||||
private permissionService: PermissionService
|
||||
) {}
|
||||
|
||||
// Implement all methods with actual functionality
|
||||
// Each service handles its domain
|
||||
|
||||
getContent(): string { /* ... */ }
|
||||
setContent(content: string): void { /* ... */ }
|
||||
appendContent(content: string): void { /* ... */ }
|
||||
insertContent(position: number, content: string): void { /* ... */ }
|
||||
deleteRange(start: number, end: number): void { /* ... */ }
|
||||
|
||||
async save(): Promise<void> { /* delegate to storage */ }
|
||||
async load(id: string): Promise<void> { /* delegate to storage */ }
|
||||
async delete(): Promise<void> { /* delegate to storage */ }
|
||||
|
||||
async exportToPdf(): Promise<Buffer> { return this.exportService.toPdf(this); }
|
||||
async exportToWord(): Promise<Buffer> { return this.exportService.toWord(this); }
|
||||
async exportToHtml(): Promise<string> { return this.exportService.toHtml(this); }
|
||||
|
||||
async share(userId: string, permission: Permission): Promise<void> { /* ... */ }
|
||||
async unshare(userId: string): Promise<void> { /* ... */ }
|
||||
async getCollaborators(): Promise<Collaborator[]> { /* ... */ }
|
||||
|
||||
async addComment(comment: Comment): Promise<void> { /* ... */ }
|
||||
async removeComment(commentId: string): Promise<void> { /* ... */ }
|
||||
async getComments(): Promise<Comment[]> { /* ... */ }
|
||||
|
||||
async createVersion(message?: string): Promise<Version> { /* ... */ }
|
||||
async getVersionHistory(): Promise<Version[]> { /* ... */ }
|
||||
async revertToVersion(versionId: string): Promise<void> { /* ... */ }
|
||||
|
||||
async setPermissions(permissions: Permissions): Promise<void> { /* ... */ }
|
||||
async getPermissions(): Promise<Permissions> { /* ... */ }
|
||||
async checkPermission(userId: string, action: string): Promise<boolean> { /* ... */ }
|
||||
}
|
||||
|
||||
// Functions can accept only the interfaces they need
|
||||
function renderDocument(doc: DocumentContent): void {
|
||||
console.log(doc.getContent());
|
||||
}
|
||||
|
||||
async function exportToPdf(doc: PdfExportable): Promise<void> {
|
||||
const pdf = await doc.exportToPdf();
|
||||
// Send pdf...
|
||||
}
|
||||
|
||||
function canUserEdit(doc: PermissionControlled, userId: string): Promise<boolean> {
|
||||
return doc.checkPermission(userId, 'edit');
|
||||
}
|
||||
|
||||
// All document types work where their interfaces are expected
|
||||
const simpleNote = new SimpleNote();
|
||||
renderDocument(simpleNote); // Works!
|
||||
|
||||
const collabDoc = new CollaborativeDocument(/* ... */);
|
||||
renderDocument(collabDoc); // Works!
|
||||
await exportToPdf(collabDoc); // Works!
|
||||
await canUserEdit(collabDoc, 'user123'); // Works!
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Cohesion**: Each interface groups related methods. `Commentable` is about comments, `Versionable` about versions.
|
||||
|
||||
2. **Flexibility**: Classes implement only the interfaces that match their capabilities.
|
||||
|
||||
3. **Composability**: Complex types are built by combining simple interfaces: `EditableContent & Persistable & Shareable`.
|
||||
|
||||
4. **No Empty Implementations**: No need for `throw new Error('Not supported')` - just don't implement the interface.
|
||||
|
||||
5. **Clear Capabilities**: Looking at a class's implemented interfaces tells you exactly what it can do.
|
||||
|
||||
6. **Easier Evolution**: Add new capabilities by creating new interfaces without modifying existing ones.
|
||||
|
||||
7. **Better Typing**: Functions declare exactly what they need. `exportToPdf(doc: PdfExportable)` is self-documenting.
|
||||
@@ -0,0 +1,203 @@
|
||||
---
|
||||
id: solid-lsp-contracts
|
||||
title: SOLID - Liskov Substitution (Contracts)
|
||||
category: solid-principles
|
||||
priority: critical
|
||||
tags: [SOLID, LSP, liskov-substitution, contracts]
|
||||
related: [solid-lsp-preconditions, solid-ocp-abstraction, core-composition]
|
||||
---
|
||||
|
||||
# Liskov Substitution Principle - Contracts
|
||||
|
||||
Subtypes must be substitutable for their base types without altering the correctness of the program. Derived classes must honor the contracts established by their base classes.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Subclass violates the contract of the base class
|
||||
|
||||
class Rectangle {
|
||||
protected _width: number;
|
||||
protected _height: number;
|
||||
|
||||
constructor(width: number, height: number) {
|
||||
this._width = width;
|
||||
this._height = height;
|
||||
}
|
||||
|
||||
get width(): number {
|
||||
return this._width;
|
||||
}
|
||||
|
||||
set width(value: number) {
|
||||
this._width = value;
|
||||
}
|
||||
|
||||
get height(): number {
|
||||
return this._height;
|
||||
}
|
||||
|
||||
set height(value: number) {
|
||||
this._height = value;
|
||||
}
|
||||
|
||||
getArea(): number {
|
||||
return this._width * this._height;
|
||||
}
|
||||
}
|
||||
|
||||
// Square violates LSP - it changes the behavior of setters
|
||||
class Square extends Rectangle {
|
||||
constructor(side: number) {
|
||||
super(side, side);
|
||||
}
|
||||
|
||||
// Violates LSP: setter has different behavior than parent
|
||||
set width(value: number) {
|
||||
this._width = value;
|
||||
this._height = value; // Unexpected side effect!
|
||||
}
|
||||
|
||||
set height(value: number) {
|
||||
this._width = value; // Unexpected side effect!
|
||||
this._height = value;
|
||||
}
|
||||
}
|
||||
|
||||
// This code works with Rectangle but breaks with Square
|
||||
function resizeRectangle(rect: Rectangle): void {
|
||||
rect.width = 10;
|
||||
rect.height = 5;
|
||||
|
||||
// Expects area to be 50, but Square gives 25!
|
||||
console.assert(rect.getArea() === 50, 'Area should be 50');
|
||||
}
|
||||
|
||||
const rectangle = new Rectangle(4, 4);
|
||||
resizeRectangle(rectangle); // Works: area is 50
|
||||
|
||||
const square = new Square(4);
|
||||
resizeRectangle(square); // Fails: area is 25, not 50
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Use composition and proper abstractions
|
||||
|
||||
// Define what shapes can do
|
||||
interface Shape {
|
||||
getArea(): number;
|
||||
getPerimeter(): number;
|
||||
}
|
||||
|
||||
// Define what resizable shapes can do
|
||||
interface ResizableShape extends Shape {
|
||||
scale(factor: number): void;
|
||||
}
|
||||
|
||||
// Immutable rectangle - no setters that could be violated
|
||||
class Rectangle implements Shape {
|
||||
constructor(
|
||||
readonly width: number,
|
||||
readonly height: number
|
||||
) {
|
||||
if (width <= 0 || height <= 0) {
|
||||
throw new Error('Dimensions must be positive');
|
||||
}
|
||||
}
|
||||
|
||||
getArea(): number {
|
||||
return this.width * this.height;
|
||||
}
|
||||
|
||||
getPerimeter(): number {
|
||||
return 2 * (this.width + this.height);
|
||||
}
|
||||
|
||||
// Return new instance instead of mutating
|
||||
withWidth(width: number): Rectangle {
|
||||
return new Rectangle(width, this.height);
|
||||
}
|
||||
|
||||
withHeight(height: number): Rectangle {
|
||||
return new Rectangle(this.width, height);
|
||||
}
|
||||
|
||||
scale(factor: number): Rectangle {
|
||||
return new Rectangle(this.width * factor, this.height * factor);
|
||||
}
|
||||
}
|
||||
|
||||
// Square is its own shape, not a subtype of Rectangle
|
||||
class Square implements Shape {
|
||||
constructor(readonly side: number) {
|
||||
if (side <= 0) {
|
||||
throw new Error('Side must be positive');
|
||||
}
|
||||
}
|
||||
|
||||
getArea(): number {
|
||||
return this.side * this.side;
|
||||
}
|
||||
|
||||
getPerimeter(): number {
|
||||
return 4 * this.side;
|
||||
}
|
||||
|
||||
withSide(side: number): Square {
|
||||
return new Square(side);
|
||||
}
|
||||
|
||||
scale(factor: number): Square {
|
||||
return new Square(this.side * factor);
|
||||
}
|
||||
}
|
||||
|
||||
// Functions work with the Shape interface
|
||||
function printShapeInfo(shape: Shape): void {
|
||||
console.log(`Area: ${shape.getArea()}`);
|
||||
console.log(`Perimeter: ${shape.getPerimeter()}`);
|
||||
}
|
||||
|
||||
// This works correctly with both Rectangle and Square
|
||||
const rect = new Rectangle(10, 5);
|
||||
printShapeInfo(rect); // Area: 50, Perimeter: 30
|
||||
|
||||
const square = new Square(5);
|
||||
printShapeInfo(square); // Area: 25, Perimeter: 20
|
||||
|
||||
// For operations specific to rectangles, use Rectangle type
|
||||
function createBanner(width: number, height: number): Rectangle {
|
||||
return new Rectangle(width, height);
|
||||
}
|
||||
|
||||
// For operations that work with any shape, use Shape interface
|
||||
function calculateTotalArea(shapes: Shape[]): number {
|
||||
return shapes.reduce((total, shape) => total + shape.getArea(), 0);
|
||||
}
|
||||
|
||||
const shapes: Shape[] = [
|
||||
new Rectangle(10, 5),
|
||||
new Square(4),
|
||||
new Rectangle(3, 7)
|
||||
];
|
||||
|
||||
console.log(calculateTotalArea(shapes)); // 50 + 16 + 21 = 87
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Predictable Behavior**: Code using the base type works correctly with any subtype. No surprises.
|
||||
|
||||
2. **Safe Polymorphism**: You can pass any `Shape` to functions expecting `Shape` without checking the concrete type.
|
||||
|
||||
3. **Contract Honoring**: Each class fully honors its interface contract - `getArea()` always returns the correct area.
|
||||
|
||||
4. **Immutability Benefits**: By making shapes immutable, we avoid the setter problem entirely.
|
||||
|
||||
5. **Proper Modeling**: Square and Rectangle are separate concepts that happen to share behavior (Shape), not a parent-child relationship.
|
||||
|
||||
6. **Easier Testing**: Tests for `Shape` work for all implementations. No special cases needed.
|
||||
|
||||
7. **Design Clarity**: The inheritance hierarchy reflects true "is-a" relationships, not just code reuse.
|
||||
@@ -0,0 +1,256 @@
|
||||
---
|
||||
id: solid-lsp-preconditions
|
||||
title: SOLID - Liskov Substitution (Preconditions)
|
||||
category: solid-principles
|
||||
priority: critical
|
||||
tags: [SOLID, LSP, liskov-substitution, preconditions, postconditions]
|
||||
related: [solid-lsp-contracts, core-fail-fast, solid-ocp-abstraction]
|
||||
---
|
||||
|
||||
# Liskov Substitution Principle - Preconditions and Postconditions
|
||||
|
||||
Subtypes cannot strengthen preconditions (require more) or weaken postconditions (guarantee less) compared to their base types.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Subclass strengthens preconditions and weakens postconditions
|
||||
|
||||
interface PaymentProcessor {
|
||||
// Contract: accepts any amount > 0, returns transaction ID
|
||||
processPayment(amount: number): Promise<string>;
|
||||
}
|
||||
|
||||
class StandardPaymentProcessor implements PaymentProcessor {
|
||||
async processPayment(amount: number): Promise<string> {
|
||||
if (amount <= 0) {
|
||||
throw new Error('Amount must be positive');
|
||||
}
|
||||
// Process payment and return transaction ID
|
||||
return `TXN-${Date.now()}`;
|
||||
}
|
||||
}
|
||||
|
||||
// Violates LSP: Strengthens preconditions
|
||||
class PremiumPaymentProcessor implements PaymentProcessor {
|
||||
private readonly minimumAmount = 100; // Stronger precondition!
|
||||
private readonly maximumAmount = 10000; // Stronger precondition!
|
||||
|
||||
async processPayment(amount: number): Promise<string> {
|
||||
if (amount <= 0) {
|
||||
throw new Error('Amount must be positive');
|
||||
}
|
||||
// Additional restrictions not in base contract
|
||||
if (amount < this.minimumAmount) {
|
||||
throw new Error(`Minimum amount is ${this.minimumAmount}`); // Violates LSP!
|
||||
}
|
||||
if (amount > this.maximumAmount) {
|
||||
throw new Error(`Maximum amount is ${this.maximumAmount}`); // Violates LSP!
|
||||
}
|
||||
return `PREM-TXN-${Date.now()}`;
|
||||
}
|
||||
}
|
||||
|
||||
// Violates LSP: Weakens postconditions
|
||||
class UnreliablePaymentProcessor implements PaymentProcessor {
|
||||
async processPayment(amount: number): Promise<string> {
|
||||
if (amount <= 0) {
|
||||
throw new Error('Amount must be positive');
|
||||
}
|
||||
|
||||
// Sometimes returns null instead of transaction ID - weaker postcondition!
|
||||
if (Math.random() > 0.5) {
|
||||
return null as any; // Violates the guarantee of returning a string
|
||||
}
|
||||
|
||||
return `TXN-${Date.now()}`;
|
||||
}
|
||||
}
|
||||
|
||||
// Client code breaks when substituting implementations
|
||||
async function checkout(processor: PaymentProcessor, amount: number): Promise<void> {
|
||||
const txnId = await processor.processPayment(amount);
|
||||
// Assumes txnId is always a valid string (as per contract)
|
||||
console.log(`Payment complete: ${txnId.toUpperCase()}`); // Crashes with null!
|
||||
}
|
||||
|
||||
// Works with StandardPaymentProcessor
|
||||
await checkout(new StandardPaymentProcessor(), 50);
|
||||
|
||||
// Fails with PremiumPaymentProcessor - amount too low
|
||||
await checkout(new PremiumPaymentProcessor(), 50); // Error: Minimum amount is 100
|
||||
|
||||
// Fails with UnreliablePaymentProcessor - null txnId
|
||||
await checkout(new UnreliablePaymentProcessor(), 50); // Error: Cannot read toUpperCase of null
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Subtypes honor preconditions and postconditions
|
||||
|
||||
// Clear contract with documented preconditions and postconditions
|
||||
interface PaymentProcessor {
|
||||
/**
|
||||
* Process a payment transaction.
|
||||
*
|
||||
* @precondition amount > 0
|
||||
* @postcondition returns a non-empty transaction ID string
|
||||
* @throws PaymentError if payment fails (not for invalid preconditions)
|
||||
*/
|
||||
processPayment(amount: number): Promise<PaymentResult>;
|
||||
|
||||
/**
|
||||
* Check if this processor can handle the given amount.
|
||||
* Allows clients to check before attempting payment.
|
||||
*/
|
||||
canProcess(amount: number): boolean;
|
||||
|
||||
/**
|
||||
* Get the constraints of this processor.
|
||||
*/
|
||||
getConstraints(): ProcessorConstraints;
|
||||
}
|
||||
|
||||
interface PaymentResult {
|
||||
readonly transactionId: string;
|
||||
readonly processedAt: Date;
|
||||
readonly amount: number;
|
||||
readonly status: 'success' | 'pending';
|
||||
}
|
||||
|
||||
interface ProcessorConstraints {
|
||||
readonly minAmount: number;
|
||||
readonly maxAmount: number;
|
||||
readonly supportedCurrencies: string[];
|
||||
}
|
||||
|
||||
// Base implementation with standard constraints
|
||||
class StandardPaymentProcessor implements PaymentProcessor {
|
||||
private readonly constraints: ProcessorConstraints = {
|
||||
minAmount: 0.01,
|
||||
maxAmount: Infinity,
|
||||
supportedCurrencies: ['USD', 'EUR', 'GBP']
|
||||
};
|
||||
|
||||
getConstraints(): ProcessorConstraints {
|
||||
return this.constraints;
|
||||
}
|
||||
|
||||
canProcess(amount: number): boolean {
|
||||
return amount >= this.constraints.minAmount &&
|
||||
amount <= this.constraints.maxAmount;
|
||||
}
|
||||
|
||||
async processPayment(amount: number): Promise<PaymentResult> {
|
||||
// Precondition check (same as interface contract)
|
||||
if (amount <= 0) {
|
||||
throw new InvalidAmountError('Amount must be positive');
|
||||
}
|
||||
|
||||
// Implementation
|
||||
const transactionId = `TXN-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`;
|
||||
|
||||
// Postcondition: always returns valid PaymentResult
|
||||
return {
|
||||
transactionId,
|
||||
processedAt: new Date(),
|
||||
amount,
|
||||
status: 'success'
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// Premium processor with different constraints but same contract behavior
|
||||
class PremiumPaymentProcessor implements PaymentProcessor {
|
||||
private readonly constraints: ProcessorConstraints = {
|
||||
minAmount: 100,
|
||||
maxAmount: 100000,
|
||||
supportedCurrencies: ['USD', 'EUR', 'GBP', 'JPY', 'CHF']
|
||||
};
|
||||
|
||||
getConstraints(): ProcessorConstraints {
|
||||
return this.constraints;
|
||||
}
|
||||
|
||||
// Clients can check constraints before calling processPayment
|
||||
canProcess(amount: number): boolean {
|
||||
return amount >= this.constraints.minAmount &&
|
||||
amount <= this.constraints.maxAmount;
|
||||
}
|
||||
|
||||
async processPayment(amount: number): Promise<PaymentResult> {
|
||||
// Same precondition as interface - not strengthened
|
||||
if (amount <= 0) {
|
||||
throw new InvalidAmountError('Amount must be positive');
|
||||
}
|
||||
|
||||
// Business logic can still reject, but with proper error handling
|
||||
if (!this.canProcess(amount)) {
|
||||
throw new PaymentError(
|
||||
`Amount ${amount} outside processor range [${this.constraints.minAmount}, ${this.constraints.maxAmount}]`
|
||||
);
|
||||
}
|
||||
|
||||
// Premium processing logic
|
||||
const transactionId = `PREM-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`;
|
||||
|
||||
// Postcondition: always returns valid PaymentResult (same guarantee)
|
||||
return {
|
||||
transactionId,
|
||||
processedAt: new Date(),
|
||||
amount,
|
||||
status: 'success'
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// Factory that selects appropriate processor
|
||||
class PaymentProcessorFactory {
|
||||
private processors: PaymentProcessor[] = [
|
||||
new StandardPaymentProcessor(),
|
||||
new PremiumPaymentProcessor()
|
||||
];
|
||||
|
||||
getProcessorFor(amount: number): PaymentProcessor {
|
||||
const suitable = this.processors.find(p => p.canProcess(amount));
|
||||
if (!suitable) {
|
||||
throw new NoSuitableProcessorError(amount);
|
||||
}
|
||||
return suitable;
|
||||
}
|
||||
}
|
||||
|
||||
// Client code works correctly with any processor
|
||||
async function checkout(amount: number, factory: PaymentProcessorFactory): Promise<void> {
|
||||
const processor = factory.getProcessorFor(amount);
|
||||
const result = await processor.processPayment(amount);
|
||||
|
||||
// Postcondition guarantees result is valid
|
||||
console.log(`Payment complete: ${result.transactionId}`);
|
||||
console.log(`Processed at: ${result.processedAt.toISOString()}`);
|
||||
}
|
||||
|
||||
// Usage
|
||||
const factory = new PaymentProcessorFactory();
|
||||
|
||||
await checkout(50, factory); // Uses StandardPaymentProcessor
|
||||
await checkout(500, factory); // Uses PremiumPaymentProcessor
|
||||
await checkout(5000, factory); // Uses PremiumPaymentProcessor
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Substitutability**: Any `PaymentProcessor` can be used interchangeably where `PaymentProcessor` is expected.
|
||||
|
||||
2. **Client Safety**: Clients can rely on the contract. If preconditions are met, postconditions are guaranteed.
|
||||
|
||||
3. **Explicit Constraints**: Instead of silently strengthening preconditions, processors expose their constraints through `getConstraints()` and `canProcess()`.
|
||||
|
||||
4. **Proper Error Handling**: Constraint violations throw appropriate errors that clients can handle, rather than unexpected failures.
|
||||
|
||||
5. **Factory Pattern**: The factory selects the appropriate processor, keeping constraint logic out of client code.
|
||||
|
||||
6. **Design by Contract**: Clear documentation of preconditions and postconditions makes the contract explicit.
|
||||
|
||||
7. **Defensive Programming**: Clients can check `canProcess()` before attempting payment, avoiding errors entirely.
|
||||
@@ -0,0 +1,265 @@
|
||||
---
|
||||
id: solid-ocp-abstraction
|
||||
title: SOLID - Open/Closed (Abstraction)
|
||||
category: solid-principles
|
||||
priority: critical
|
||||
tags: [SOLID, OCP, open-closed, abstraction]
|
||||
related: [solid-ocp-extension, solid-dip-abstractions, pattern-repository]
|
||||
---
|
||||
|
||||
# Open/Closed Principle - Abstraction
|
||||
|
||||
Use abstractions (interfaces and abstract classes) to define stable contracts that allow new implementations without modifying existing code.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Concrete dependencies that require modification for new requirements
|
||||
class ReportGenerator {
|
||||
generateReport(data: SalesData[], format: string): string {
|
||||
let report = '';
|
||||
|
||||
// Calculate totals
|
||||
const totalSales = data.reduce((sum, d) => sum + d.amount, 0);
|
||||
const averageSale = totalSales / data.length;
|
||||
|
||||
if (format === 'html') {
|
||||
report = `
|
||||
<html>
|
||||
<body>
|
||||
<h1>Sales Report</h1>
|
||||
<p>Total Sales: $${totalSales}</p>
|
||||
<p>Average Sale: $${averageSale.toFixed(2)}</p>
|
||||
<table>
|
||||
${data.map(d => `<tr><td>${d.date}</td><td>$${d.amount}</td></tr>`).join('')}
|
||||
</table>
|
||||
</body>
|
||||
</html>
|
||||
`;
|
||||
} else if (format === 'pdf') {
|
||||
// PDF generation logic mixed in
|
||||
const pdf = new PDFDocument();
|
||||
pdf.text('Sales Report');
|
||||
pdf.text(`Total: $${totalSales}`);
|
||||
// ... more PDF logic
|
||||
report = pdf.output();
|
||||
} else if (format === 'csv') {
|
||||
report = 'Date,Amount\n';
|
||||
report += data.map(d => `${d.date},${d.amount}`).join('\n');
|
||||
} else if (format === 'excel') {
|
||||
// Excel generation logic
|
||||
// Adding new format requires modifying this class
|
||||
}
|
||||
|
||||
return report;
|
||||
}
|
||||
}
|
||||
|
||||
// Problems:
|
||||
// 1. Adding JSON format requires modifying ReportGenerator
|
||||
// 2. All format logic is tightly coupled
|
||||
// 3. Testing is difficult
|
||||
// 4. No way to extend without modification
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Abstractions enable extension without modification
|
||||
|
||||
// Stable abstraction for report data
|
||||
interface ReportData {
|
||||
readonly title: string;
|
||||
readonly generatedAt: Date;
|
||||
readonly summary: ReportSummary;
|
||||
readonly items: ReportItem[];
|
||||
}
|
||||
|
||||
interface ReportSummary {
|
||||
readonly totalSales: number;
|
||||
readonly averageSale: number;
|
||||
readonly itemCount: number;
|
||||
}
|
||||
|
||||
interface ReportItem {
|
||||
readonly date: string;
|
||||
readonly amount: number;
|
||||
readonly description?: string;
|
||||
}
|
||||
|
||||
// Stable abstraction for formatters
|
||||
interface ReportFormatter {
|
||||
readonly format: string;
|
||||
readonly mimeType: string;
|
||||
render(data: ReportData): string | Buffer | Promise<string | Buffer>;
|
||||
}
|
||||
|
||||
// Stable abstraction for data sources
|
||||
interface ReportDataSource<T> {
|
||||
fetch(criteria: ReportCriteria): Promise<T[]>;
|
||||
transform(rawData: T[]): ReportData;
|
||||
}
|
||||
|
||||
// Report generator depends only on abstractions
|
||||
class ReportGenerator {
|
||||
constructor(
|
||||
private formatters: Map<string, ReportFormatter> = new Map()
|
||||
) {}
|
||||
|
||||
registerFormatter(formatter: ReportFormatter): void {
|
||||
this.formatters.set(formatter.format, formatter);
|
||||
}
|
||||
|
||||
async generate<T>(
|
||||
dataSource: ReportDataSource<T>,
|
||||
criteria: ReportCriteria,
|
||||
format: string
|
||||
): Promise<GeneratedReport> {
|
||||
const formatter = this.formatters.get(format);
|
||||
if (!formatter) {
|
||||
throw new UnsupportedFormatError(format, this.getSupportedFormats());
|
||||
}
|
||||
|
||||
const rawData = await dataSource.fetch(criteria);
|
||||
const reportData = dataSource.transform(rawData);
|
||||
const content = await Promise.resolve(formatter.render(reportData));
|
||||
|
||||
return {
|
||||
content,
|
||||
mimeType: formatter.mimeType,
|
||||
format,
|
||||
generatedAt: new Date()
|
||||
};
|
||||
}
|
||||
|
||||
getSupportedFormats(): string[] {
|
||||
return Array.from(this.formatters.keys());
|
||||
}
|
||||
}
|
||||
|
||||
// Concrete formatters - each can be added without modifying ReportGenerator
|
||||
|
||||
class HtmlReportFormatter implements ReportFormatter {
|
||||
readonly format = 'html';
|
||||
readonly mimeType = 'text/html';
|
||||
|
||||
render(data: ReportData): string {
|
||||
return `
|
||||
<!DOCTYPE html>
|
||||
<html>
|
||||
<head><title>${data.title}</title></head>
|
||||
<body>
|
||||
<h1>${data.title}</h1>
|
||||
<p>Generated: ${data.generatedAt.toISOString()}</p>
|
||||
<section class="summary">
|
||||
<p>Total Sales: $${data.summary.totalSales.toFixed(2)}</p>
|
||||
<p>Average Sale: $${data.summary.averageSale.toFixed(2)}</p>
|
||||
</section>
|
||||
<table>
|
||||
<thead><tr><th>Date</th><th>Amount</th></tr></thead>
|
||||
<tbody>
|
||||
${data.items.map(item =>
|
||||
`<tr><td>${item.date}</td><td>$${item.amount}</td></tr>`
|
||||
).join('')}
|
||||
</tbody>
|
||||
</table>
|
||||
</body>
|
||||
</html>
|
||||
`;
|
||||
}
|
||||
}
|
||||
|
||||
class CsvReportFormatter implements ReportFormatter {
|
||||
readonly format = 'csv';
|
||||
readonly mimeType = 'text/csv';
|
||||
|
||||
render(data: ReportData): string {
|
||||
const header = 'Date,Amount,Description';
|
||||
const rows = data.items.map(item =>
|
||||
`${item.date},${item.amount},"${item.description || ''}"`
|
||||
);
|
||||
return [header, ...rows].join('\n');
|
||||
}
|
||||
}
|
||||
|
||||
class JsonReportFormatter implements ReportFormatter {
|
||||
readonly format = 'json';
|
||||
readonly mimeType = 'application/json';
|
||||
|
||||
render(data: ReportData): string {
|
||||
return JSON.stringify(data, null, 2);
|
||||
}
|
||||
}
|
||||
|
||||
// Adding Excel format - no modification to existing code
|
||||
class ExcelReportFormatter implements ReportFormatter {
|
||||
readonly format = 'xlsx';
|
||||
readonly mimeType = 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet';
|
||||
|
||||
render(data: ReportData): Buffer {
|
||||
const workbook = new ExcelJS.Workbook();
|
||||
const sheet = workbook.addWorksheet(data.title);
|
||||
|
||||
sheet.addRow(['Date', 'Amount', 'Description']);
|
||||
data.items.forEach(item => {
|
||||
sheet.addRow([item.date, item.amount, item.description || '']);
|
||||
});
|
||||
|
||||
return workbook.xlsx.writeBuffer();
|
||||
}
|
||||
}
|
||||
|
||||
// Concrete data source - can create new sources without modification
|
||||
class SalesReportDataSource implements ReportDataSource<SalesData> {
|
||||
constructor(private salesRepository: SalesRepository) {}
|
||||
|
||||
async fetch(criteria: ReportCriteria): Promise<SalesData[]> {
|
||||
return this.salesRepository.findByCriteria(criteria);
|
||||
}
|
||||
|
||||
transform(rawData: SalesData[]): ReportData {
|
||||
const totalSales = rawData.reduce((sum, d) => sum + d.amount, 0);
|
||||
|
||||
return {
|
||||
title: 'Sales Report',
|
||||
generatedAt: new Date(),
|
||||
summary: {
|
||||
totalSales,
|
||||
averageSale: rawData.length > 0 ? totalSales / rawData.length : 0,
|
||||
itemCount: rawData.length
|
||||
},
|
||||
items: rawData.map(d => ({
|
||||
date: d.date,
|
||||
amount: d.amount,
|
||||
description: d.productName
|
||||
}))
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// Usage
|
||||
const generator = new ReportGenerator();
|
||||
generator.registerFormatter(new HtmlReportFormatter());
|
||||
generator.registerFormatter(new CsvReportFormatter());
|
||||
generator.registerFormatter(new JsonReportFormatter());
|
||||
generator.registerFormatter(new ExcelReportFormatter());
|
||||
|
||||
const salesDataSource = new SalesReportDataSource(salesRepository);
|
||||
const report = await generator.generate(salesDataSource, criteria, 'xlsx');
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Stable Core**: The `ReportGenerator` class and interfaces form a stable core that rarely changes.
|
||||
|
||||
2. **Independent Development**: Teams can develop new formatters or data sources independently.
|
||||
|
||||
3. **Composition Over Inheritance**: New functionality is added through composition (registering new implementations) rather than inheritance hierarchies.
|
||||
|
||||
4. **Testing**: Each component can be tested in isolation with mock implementations of interfaces.
|
||||
|
||||
5. **Flexibility**: The same generator works with any combination of data sources and formatters.
|
||||
|
||||
6. **Dependency Inversion**: High-level modules (ReportGenerator) don't depend on low-level modules (specific formatters), both depend on abstractions.
|
||||
|
||||
7. **Plugin System**: Natural foundation for a plugin architecture where third parties can add formatters.
|
||||
@@ -0,0 +1,205 @@
|
||||
---
|
||||
id: solid-ocp-extension
|
||||
title: SOLID - Open/Closed Principle (Extension)
|
||||
category: solid-principles
|
||||
priority: critical
|
||||
tags: [SOLID, OCP, open-closed, extensibility, design-patterns]
|
||||
related: [solid-ocp-abstraction, pattern-repository, solid-dip-abstractions]
|
||||
---
|
||||
|
||||
# Open/Closed Principle - Extension
|
||||
|
||||
Software entities should be open for extension but closed for modification. Add new functionality by adding new code, not by changing existing code.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Modifying existing code to add new functionality
|
||||
class PaymentProcessor {
|
||||
processPayment(payment: Payment): PaymentResult {
|
||||
switch (payment.method) {
|
||||
case 'credit_card':
|
||||
return this.processCreditCard(payment);
|
||||
case 'paypal':
|
||||
return this.processPayPal(payment);
|
||||
case 'stripe':
|
||||
return this.processStripe(payment);
|
||||
// Every new payment method requires modifying this class
|
||||
case 'apple_pay':
|
||||
return this.processApplePay(payment);
|
||||
case 'google_pay':
|
||||
return this.processGooglePay(payment);
|
||||
case 'crypto':
|
||||
return this.processCrypto(payment);
|
||||
default:
|
||||
throw new Error(`Unknown payment method: ${payment.method}`);
|
||||
}
|
||||
}
|
||||
|
||||
private processCreditCard(payment: Payment): PaymentResult {
|
||||
// Credit card logic
|
||||
}
|
||||
|
||||
private processPayPal(payment: Payment): PaymentResult {
|
||||
// PayPal logic
|
||||
}
|
||||
|
||||
private processStripe(payment: Payment): PaymentResult {
|
||||
// Stripe logic
|
||||
}
|
||||
|
||||
private processApplePay(payment: Payment): PaymentResult {
|
||||
// Apple Pay logic
|
||||
}
|
||||
|
||||
private processGooglePay(payment: Payment): PaymentResult {
|
||||
// Google Pay logic
|
||||
}
|
||||
|
||||
private processCrypto(payment: Payment): PaymentResult {
|
||||
// Crypto logic
|
||||
}
|
||||
}
|
||||
|
||||
// Adding a new payment method requires:
|
||||
// 1. Adding a new case to the switch statement
|
||||
// 2. Adding a new private method
|
||||
// 3. Testing the entire class again
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Open for extension, closed for modification
|
||||
|
||||
// Define the contract for payment handlers
|
||||
interface PaymentHandler {
|
||||
readonly methodType: string;
|
||||
canHandle(payment: Payment): boolean;
|
||||
process(payment: Payment): Promise<PaymentResult>;
|
||||
validate(payment: Payment): ValidationResult;
|
||||
}
|
||||
|
||||
// Payment processor that never needs modification
|
||||
class PaymentProcessor {
|
||||
private handlers: Map<string, PaymentHandler> = new Map();
|
||||
|
||||
registerHandler(handler: PaymentHandler): void {
|
||||
this.handlers.set(handler.methodType, handler);
|
||||
}
|
||||
|
||||
async processPayment(payment: Payment): Promise<PaymentResult> {
|
||||
const handler = this.handlers.get(payment.method);
|
||||
|
||||
if (!handler) {
|
||||
throw new UnsupportedPaymentMethodError(payment.method);
|
||||
}
|
||||
|
||||
const validation = handler.validate(payment);
|
||||
if (!validation.isValid) {
|
||||
throw new PaymentValidationError(validation.errors);
|
||||
}
|
||||
|
||||
return handler.process(payment);
|
||||
}
|
||||
|
||||
getSupportedMethods(): string[] {
|
||||
return Array.from(this.handlers.keys());
|
||||
}
|
||||
}
|
||||
|
||||
// Each payment method is a separate class that can be added without modifying PaymentProcessor
|
||||
|
||||
class CreditCardPaymentHandler implements PaymentHandler {
|
||||
readonly methodType = 'credit_card';
|
||||
|
||||
canHandle(payment: Payment): boolean {
|
||||
return payment.method === this.methodType;
|
||||
}
|
||||
|
||||
validate(payment: Payment): ValidationResult {
|
||||
const errors: string[] = [];
|
||||
if (!payment.cardNumber || payment.cardNumber.length !== 16) {
|
||||
errors.push('Invalid card number');
|
||||
}
|
||||
if (!payment.cvv || payment.cvv.length !== 3) {
|
||||
errors.push('Invalid CVV');
|
||||
}
|
||||
return { isValid: errors.length === 0, errors };
|
||||
}
|
||||
|
||||
async process(payment: Payment): Promise<PaymentResult> {
|
||||
// Credit card processing logic
|
||||
return { success: true, transactionId: generateId() };
|
||||
}
|
||||
}
|
||||
|
||||
class PayPalPaymentHandler implements PaymentHandler {
|
||||
readonly methodType = 'paypal';
|
||||
|
||||
canHandle(payment: Payment): boolean {
|
||||
return payment.method === this.methodType;
|
||||
}
|
||||
|
||||
validate(payment: Payment): ValidationResult {
|
||||
const errors: string[] = [];
|
||||
if (!payment.paypalEmail) {
|
||||
errors.push('PayPal email required');
|
||||
}
|
||||
return { isValid: errors.length === 0, errors };
|
||||
}
|
||||
|
||||
async process(payment: Payment): Promise<PaymentResult> {
|
||||
// PayPal processing logic
|
||||
return { success: true, transactionId: generateId() };
|
||||
}
|
||||
}
|
||||
|
||||
// Adding a new payment method - no modification to existing code!
|
||||
class CryptoPaymentHandler implements PaymentHandler {
|
||||
readonly methodType = 'crypto';
|
||||
|
||||
canHandle(payment: Payment): boolean {
|
||||
return payment.method === this.methodType;
|
||||
}
|
||||
|
||||
validate(payment: Payment): ValidationResult {
|
||||
const errors: string[] = [];
|
||||
if (!payment.walletAddress) {
|
||||
errors.push('Wallet address required');
|
||||
}
|
||||
if (!payment.cryptoCurrency) {
|
||||
errors.push('Cryptocurrency type required');
|
||||
}
|
||||
return { isValid: errors.length === 0, errors };
|
||||
}
|
||||
|
||||
async process(payment: Payment): Promise<PaymentResult> {
|
||||
// Crypto processing logic
|
||||
return { success: true, transactionId: generateId() };
|
||||
}
|
||||
}
|
||||
|
||||
// Application setup - register handlers
|
||||
const processor = new PaymentProcessor();
|
||||
processor.registerHandler(new CreditCardPaymentHandler());
|
||||
processor.registerHandler(new PayPalPaymentHandler());
|
||||
processor.registerHandler(new CryptoPaymentHandler());
|
||||
// Add new payment methods by simply registering new handlers
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Stability**: The `PaymentProcessor` class is stable and tested. Adding new payment methods doesn't risk breaking existing functionality.
|
||||
|
||||
2. **Team Scalability**: Different developers can work on different payment handlers simultaneously without merge conflicts.
|
||||
|
||||
3. **Plugin Architecture**: Payment handlers can be loaded dynamically, even from external packages or plugins.
|
||||
|
||||
4. **Testing Isolation**: Each handler is tested independently. New handlers don't require retesting old ones.
|
||||
|
||||
5. **Feature Flags**: New payment methods can be enabled/disabled by simply registering or not registering the handler.
|
||||
|
||||
6. **Compliance**: When regulations change for one payment method, only that handler needs updating.
|
||||
|
||||
7. **Rollback Safety**: If a new payment handler has issues, remove it without touching proven code.
|
||||
149
.agents/skills/clean-code-principles/rules/solid-srp-class.md
Normal file
149
.agents/skills/clean-code-principles/rules/solid-srp-class.md
Normal file
@@ -0,0 +1,149 @@
|
||||
---
|
||||
id: solid-srp-class
|
||||
title: SOLID - Single Responsibility Principle (Class Level)
|
||||
category: solid-principles
|
||||
priority: critical
|
||||
tags: [SOLID, SRP, single-responsibility, class-design]
|
||||
related: [solid-srp-function, core-separation-concerns, solid-isp-interfaces]
|
||||
---
|
||||
|
||||
# Single Responsibility Principle - Class Level
|
||||
|
||||
A class should have only one reason to change, meaning it should have only one job or responsibility.
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Class handles multiple responsibilities
|
||||
class UserManager {
|
||||
private db: Database;
|
||||
private mailer: EmailService;
|
||||
|
||||
constructor() {
|
||||
this.db = new Database();
|
||||
this.mailer = new EmailService();
|
||||
}
|
||||
|
||||
createUser(userData: UserData): User {
|
||||
// Validation logic
|
||||
if (!userData.email.includes('@')) {
|
||||
throw new Error('Invalid email');
|
||||
}
|
||||
if (userData.password.length < 8) {
|
||||
throw new Error('Password too short');
|
||||
}
|
||||
|
||||
// Database logic
|
||||
const user = this.db.insert('users', userData);
|
||||
|
||||
// Email logic
|
||||
const htmlContent = `<h1>Welcome ${user.name}!</h1>`;
|
||||
this.mailer.send(user.email, 'Welcome', htmlContent);
|
||||
|
||||
// Logging logic
|
||||
console.log(`User created: ${user.id} at ${new Date()}`);
|
||||
this.writeToFile(`/logs/users.log`, `Created: ${user.id}`);
|
||||
|
||||
return user;
|
||||
}
|
||||
|
||||
private writeToFile(path: string, content: string): void {
|
||||
// File writing implementation
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Each class has a single responsibility
|
||||
|
||||
// Handles user validation only
|
||||
class UserValidator {
|
||||
validate(userData: UserData): ValidationResult {
|
||||
const errors: string[] = [];
|
||||
|
||||
if (!userData.email.includes('@')) {
|
||||
errors.push('Invalid email format');
|
||||
}
|
||||
if (userData.password.length < 8) {
|
||||
errors.push('Password must be at least 8 characters');
|
||||
}
|
||||
|
||||
return { isValid: errors.length === 0, errors };
|
||||
}
|
||||
}
|
||||
|
||||
// Handles user persistence only
|
||||
class UserRepository {
|
||||
constructor(private db: Database) {}
|
||||
|
||||
save(userData: UserData): User {
|
||||
return this.db.insert('users', userData);
|
||||
}
|
||||
|
||||
findById(id: string): User | null {
|
||||
return this.db.findOne('users', { id });
|
||||
}
|
||||
}
|
||||
|
||||
// Handles welcome email only
|
||||
class WelcomeEmailService {
|
||||
constructor(private mailer: EmailService) {}
|
||||
|
||||
sendWelcomeEmail(user: User): void {
|
||||
const htmlContent = this.buildTemplate(user);
|
||||
this.mailer.send(user.email, 'Welcome', htmlContent);
|
||||
}
|
||||
|
||||
private buildTemplate(user: User): string {
|
||||
return `<h1>Welcome ${user.name}!</h1>`;
|
||||
}
|
||||
}
|
||||
|
||||
// Handles user activity logging only
|
||||
class UserActivityLogger {
|
||||
constructor(private logger: Logger) {}
|
||||
|
||||
logCreation(user: User): void {
|
||||
this.logger.info(`User created: ${user.id}`, { userId: user.id });
|
||||
}
|
||||
}
|
||||
|
||||
// Orchestrates the user creation process
|
||||
class UserService {
|
||||
constructor(
|
||||
private validator: UserValidator,
|
||||
private repository: UserRepository,
|
||||
private welcomeEmail: WelcomeEmailService,
|
||||
private activityLogger: UserActivityLogger
|
||||
) {}
|
||||
|
||||
createUser(userData: UserData): User {
|
||||
const validation = this.validator.validate(userData);
|
||||
if (!validation.isValid) {
|
||||
throw new ValidationError(validation.errors);
|
||||
}
|
||||
|
||||
const user = this.repository.save(userData);
|
||||
this.welcomeEmail.sendWelcomeEmail(user);
|
||||
this.activityLogger.logCreation(user);
|
||||
|
||||
return user;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Easier Testing**: Each class can be unit tested in isolation without mocking unrelated dependencies.
|
||||
|
||||
2. **Reduced Coupling**: Changes to email templates don't affect database logic or validation rules.
|
||||
|
||||
3. **Better Reusability**: The `UserValidator` can be reused for profile updates, the `WelcomeEmailService` can be triggered from different flows.
|
||||
|
||||
4. **Clearer Ownership**: Teams can own specific classes without stepping on each other's work.
|
||||
|
||||
5. **Simpler Maintenance**: Bug in email formatting? Look only at `WelcomeEmailService`. Validation issue? Check `UserValidator`.
|
||||
|
||||
6. **Flexible Composition**: Easy to add features like async email sending or different logging strategies without touching core logic.
|
||||
208
.agents/skills/clean-code-principles/rules/solid-srp-function.md
Normal file
208
.agents/skills/clean-code-principles/rules/solid-srp-function.md
Normal file
@@ -0,0 +1,208 @@
|
||||
---
|
||||
id: solid-srp-function
|
||||
title: SOLID - Single Responsibility Principle (Function Level)
|
||||
category: solid-principles
|
||||
priority: critical
|
||||
tags: [SOLID, SRP, single-responsibility, function-design]
|
||||
related: [solid-srp-class, core-dry-extraction, core-kiss-simplicity]
|
||||
---
|
||||
|
||||
# Single Responsibility Principle - Function Level
|
||||
|
||||
A function should do one thing, do it well, and do it only. It should have a single, clear purpose that can be described without using conjunctions like "and" or "or".
|
||||
|
||||
## Bad Example
|
||||
|
||||
```typescript
|
||||
// Anti-pattern: Function does multiple things
|
||||
async function processOrder(orderId: string): Promise<void> {
|
||||
// Fetch order
|
||||
const order = await db.query('SELECT * FROM orders WHERE id = ?', [orderId]);
|
||||
|
||||
// Validate order
|
||||
if (!order) throw new Error('Order not found');
|
||||
if (order.status !== 'pending') throw new Error('Order already processed');
|
||||
if (order.items.length === 0) throw new Error('Order has no items');
|
||||
|
||||
// Calculate totals
|
||||
let subtotal = 0;
|
||||
for (const item of order.items) {
|
||||
const product = await db.query('SELECT price FROM products WHERE id = ?', [item.productId]);
|
||||
subtotal += product.price * item.quantity;
|
||||
}
|
||||
const tax = subtotal * 0.1;
|
||||
const shipping = subtotal > 100 ? 0 : 10;
|
||||
const total = subtotal + tax + shipping;
|
||||
|
||||
// Update inventory
|
||||
for (const item of order.items) {
|
||||
await db.query('UPDATE products SET stock = stock - ? WHERE id = ?',
|
||||
[item.quantity, item.productId]);
|
||||
}
|
||||
|
||||
// Process payment
|
||||
const paymentResult = await stripe.charges.create({
|
||||
amount: Math.round(total * 100),
|
||||
currency: 'usd',
|
||||
customer: order.customerId
|
||||
});
|
||||
|
||||
// Update order status
|
||||
await db.query('UPDATE orders SET status = ?, total = ?, payment_id = ? WHERE id = ?',
|
||||
['completed', total, paymentResult.id, orderId]);
|
||||
|
||||
// Send confirmation email
|
||||
const emailHtml = `<h1>Order Confirmed</h1><p>Total: $${total}</p>`;
|
||||
await sendgrid.send({
|
||||
to: order.customerEmail,
|
||||
subject: 'Order Confirmation',
|
||||
html: emailHtml
|
||||
});
|
||||
|
||||
// Log analytics
|
||||
await analytics.track('order_completed', { orderId, total, itemCount: order.items.length });
|
||||
}
|
||||
```
|
||||
|
||||
## Good Example
|
||||
|
||||
```typescript
|
||||
// Correct approach: Each function has a single purpose
|
||||
|
||||
async function processOrder(orderId: string): Promise<ProcessedOrder> {
|
||||
const order = await fetchAndValidateOrder(orderId);
|
||||
const pricing = await calculateOrderPricing(order);
|
||||
|
||||
await reserveInventory(order.items);
|
||||
|
||||
try {
|
||||
const payment = await processPayment(order.customerId, pricing.total);
|
||||
const completedOrder = await finalizeOrder(order, pricing, payment);
|
||||
|
||||
await sendOrderConfirmation(completedOrder);
|
||||
await trackOrderCompletion(completedOrder);
|
||||
|
||||
return completedOrder;
|
||||
} catch (error) {
|
||||
await releaseInventory(order.items);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
async function fetchAndValidateOrder(orderId: string): Promise<Order> {
|
||||
const order = await orderRepository.findById(orderId);
|
||||
|
||||
if (!order) {
|
||||
throw new OrderNotFoundError(orderId);
|
||||
}
|
||||
|
||||
validateOrderStatus(order);
|
||||
validateOrderItems(order);
|
||||
|
||||
return order;
|
||||
}
|
||||
|
||||
function validateOrderStatus(order: Order): void {
|
||||
if (order.status !== 'pending') {
|
||||
throw new InvalidOrderStatusError(order.id, order.status);
|
||||
}
|
||||
}
|
||||
|
||||
function validateOrderItems(order: Order): void {
|
||||
if (order.items.length === 0) {
|
||||
throw new EmptyOrderError(order.id);
|
||||
}
|
||||
}
|
||||
|
||||
async function calculateOrderPricing(order: Order): Promise<OrderPricing> {
|
||||
const subtotal = await calculateSubtotal(order.items);
|
||||
const tax = calculateTax(subtotal);
|
||||
const shipping = calculateShipping(subtotal);
|
||||
|
||||
return {
|
||||
subtotal,
|
||||
tax,
|
||||
shipping,
|
||||
total: subtotal + tax + shipping
|
||||
};
|
||||
}
|
||||
|
||||
async function calculateSubtotal(items: OrderItem[]): Promise<number> {
|
||||
const itemTotals = await Promise.all(
|
||||
items.map(item => calculateItemTotal(item))
|
||||
);
|
||||
return itemTotals.reduce((sum, total) => sum + total, 0);
|
||||
}
|
||||
|
||||
async function calculateItemTotal(item: OrderItem): Promise<number> {
|
||||
const product = await productRepository.findById(item.productId);
|
||||
return product.price * item.quantity;
|
||||
}
|
||||
|
||||
function calculateTax(subtotal: number): number {
|
||||
const TAX_RATE = 0.1;
|
||||
return subtotal * TAX_RATE;
|
||||
}
|
||||
|
||||
function calculateShipping(subtotal: number): number {
|
||||
const FREE_SHIPPING_THRESHOLD = 100;
|
||||
const STANDARD_SHIPPING = 10;
|
||||
return subtotal > FREE_SHIPPING_THRESHOLD ? 0 : STANDARD_SHIPPING;
|
||||
}
|
||||
|
||||
async function reserveInventory(items: OrderItem[]): Promise<void> {
|
||||
await Promise.all(
|
||||
items.map(item => inventoryService.reserve(item.productId, item.quantity))
|
||||
);
|
||||
}
|
||||
|
||||
async function releaseInventory(items: OrderItem[]): Promise<void> {
|
||||
await Promise.all(
|
||||
items.map(item => inventoryService.release(item.productId, item.quantity))
|
||||
);
|
||||
}
|
||||
|
||||
async function processPayment(customerId: string, amount: number): Promise<Payment> {
|
||||
return paymentService.charge(customerId, amount);
|
||||
}
|
||||
|
||||
async function finalizeOrder(
|
||||
order: Order,
|
||||
pricing: OrderPricing,
|
||||
payment: Payment
|
||||
): Promise<ProcessedOrder> {
|
||||
return orderRepository.updateStatus(order.id, {
|
||||
status: 'completed',
|
||||
total: pricing.total,
|
||||
paymentId: payment.id
|
||||
});
|
||||
}
|
||||
|
||||
async function sendOrderConfirmation(order: ProcessedOrder): Promise<void> {
|
||||
await emailService.sendOrderConfirmation(order);
|
||||
}
|
||||
|
||||
async function trackOrderCompletion(order: ProcessedOrder): Promise<void> {
|
||||
await analyticsService.track('order_completed', {
|
||||
orderId: order.id,
|
||||
total: order.total,
|
||||
itemCount: order.items.length
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
## Why
|
||||
|
||||
1. **Readability**: The main function reads like a story - fetch, validate, calculate, reserve, charge, finalize, notify.
|
||||
|
||||
2. **Testability**: Each function can be unit tested independently. Testing `calculateTax` doesn't require mocking a database.
|
||||
|
||||
3. **Reusability**: `calculateShipping` can be reused in a shipping estimate feature. `validateOrderStatus` can be used in other order operations.
|
||||
|
||||
4. **Debugging**: Stack traces point to specific functions. "Error in calculateSubtotal" is more helpful than "Error in processOrder at line 47".
|
||||
|
||||
5. **Modification**: Changing tax calculation only touches `calculateTax`. Adding a discount feature can be inserted cleanly between subtotal and tax.
|
||||
|
||||
6. **Error Handling**: Each function can have appropriate error handling. The main function can orchestrate rollback on failure.
|
||||
|
||||
7. **Documentation**: Function names serve as documentation. The code is self-explanatory without comments.
|
||||
Reference in New Issue
Block a user