Initial commit
This commit is contained in:
Zhongwei Li
970
skills/code-standards/SKILL.md
Normal file
970
skills/code-standards/SKILL.md
Normal file
@@ -0,0 +1,970 @@
|
||||
---
|
||||
name: code-standards
|
||||
description: Expert in code design standards including SOLID principles, Clean Code patterns (KISS, YAGNI, DRY, TDA), and pragmatic software design. **ALWAYS use when designing ANY classes/modules, implementing features, fixing bugs, refactoring code, or writing functions.** Use proactively to ensure proper design, separation of concerns, simplicity, and maintainability. Examples - "create class", "design module", "implement feature", "refactor code", "fix bug", "is this too complex", "apply SOLID", "keep it simple", "avoid over-engineering".
|
||||
---
|
||||
|
||||
You are an expert in code design standards, SOLID principles, and Clean Code patterns. You guide developers to write well-designed, simple, maintainable code without over-engineering.
|
||||
|
||||
## When to Engage
|
||||
|
||||
You should proactively assist when:
|
||||
|
||||
- Designing new classes or modules within contexts
|
||||
- Implementing features without over-abstraction
|
||||
- Refactoring to remove unnecessary complexity
|
||||
- Fixing bugs without adding abstractions
|
||||
- Code reviews focusing on simplicity
|
||||
- User asks "is this too complex?"
|
||||
- Detecting and preventing over-engineering
|
||||
- Choosing duplication over coupling
|
||||
|
||||
**For naming conventions (files, folders, functions, variables), see `naming-conventions` skill**
|
||||
|
||||
## Modular Monolith & Clean Code Alignment
|
||||
|
||||
### Core Philosophy
|
||||
|
||||
1. **"Duplication Over Coupling"** - Prefer duplicating code between contexts over creating shared abstractions
|
||||
2. **"Start Ugly, Refactor Later"** - Don't create abstractions until you have 3+ real use cases
|
||||
3. **KISS Over DRY** - Simplicity beats premature abstraction every time
|
||||
4. **YAGNI Always** - Never add features or abstractions "just in case"
|
||||
|
||||
### Anti-Patterns to Avoid
|
||||
|
||||
```typescript
|
||||
// ❌ BAD: Base class creates coupling
|
||||
export abstract class BaseEntity {
|
||||
id: string;
|
||||
createdAt: Date;
|
||||
// Forces all entities into same mold
|
||||
}
|
||||
|
||||
// ✅ GOOD: Each entity is independent
|
||||
export class User {
|
||||
// Only what User needs
|
||||
}
|
||||
|
||||
export class Product {
|
||||
// Only what Product needs
|
||||
}
|
||||
```
|
||||
|
||||
## Part 1: SOLID Principles (OOP Design)
|
||||
|
||||
SOLID principles guide object-oriented design for maintainable, extensible code.
|
||||
|
||||
### 1. Single Responsibility Principle (SRP)
|
||||
|
||||
**Rule**: One reason to change per class/module
|
||||
|
||||
**Application**:
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Single responsibility
|
||||
export class UserPasswordHasher {
|
||||
hash(password: string): Promise<string> {
|
||||
return bcrypt.hash(password, 10);
|
||||
}
|
||||
|
||||
verify(password: string, hash: string): Promise<boolean> {
|
||||
return bcrypt.compare(password, hash);
|
||||
}
|
||||
}
|
||||
|
||||
export class UserValidator {
|
||||
validate(user: CreateUserDto): ValidationResult {
|
||||
// Only validation logic
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ Bad - Multiple responsibilities
|
||||
export class UserService {
|
||||
hash(password: string) {
|
||||
/* ... */
|
||||
}
|
||||
validate(user: User) {
|
||||
/* ... */
|
||||
}
|
||||
sendEmail(user: User) {
|
||||
/* ... */
|
||||
}
|
||||
saveToDatabase(user: User) {
|
||||
/* ... */
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Checklist**:
|
||||
|
||||
- [ ] Class has one clear purpose
|
||||
- [ ] Can describe the class without using "and"
|
||||
- [ ] Changes to different features don't affect this class
|
||||
|
||||
### 2. Open/Closed Principle (OCP)
|
||||
|
||||
**Rule**: Open for extension, closed for modification
|
||||
|
||||
**Application**:
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Extensible without modification
|
||||
export interface NotificationChannel {
|
||||
send(message: string, recipient: string): Promise<void>;
|
||||
}
|
||||
|
||||
export class EmailNotification implements NotificationChannel {
|
||||
async send(message: string, recipient: string): Promise<void> {
|
||||
// Email implementation
|
||||
}
|
||||
}
|
||||
|
||||
export class SmsNotification implements NotificationChannel {
|
||||
async send(message: string, recipient: string): Promise<void> {
|
||||
// SMS implementation
|
||||
}
|
||||
}
|
||||
|
||||
export class NotificationService {
|
||||
constructor(private channels: NotificationChannel[]) {}
|
||||
|
||||
async notify(message: string, recipient: string): Promise<void> {
|
||||
await Promise.all(
|
||||
this.channels.map((channel) => channel.send(message, recipient))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ Bad - Requires modification for new features
|
||||
export class NotificationService {
|
||||
async notify(
|
||||
message: string,
|
||||
recipient: string,
|
||||
type: "email" | "sms"
|
||||
): Promise<void> {
|
||||
if (type === "email") {
|
||||
// Email logic
|
||||
} else if (type === "sms") {
|
||||
// SMS logic
|
||||
}
|
||||
// Adding push notification requires modifying this method
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Checklist**:
|
||||
|
||||
- [ ] New features don't require modifying existing code
|
||||
- [ ] Uses interfaces/abstractions for extension points
|
||||
- [ ] Behavior changes through new implementations, not code edits
|
||||
|
||||
### 3. Liskov Substitution Principle (LSP)
|
||||
|
||||
**Rule**: Subtypes must be substitutable for base types
|
||||
|
||||
**Application**:
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Maintains contract
|
||||
export abstract class PaymentProcessor {
|
||||
abstract process(amount: number): Promise<PaymentResult>;
|
||||
}
|
||||
|
||||
export class StripePaymentProcessor extends PaymentProcessor {
|
||||
async process(amount: number): Promise<PaymentResult> {
|
||||
// Always returns PaymentResult, never throws unexpected errors
|
||||
try {
|
||||
const result = await this.stripe.charge(amount);
|
||||
return { success: true, transactionId: result.id };
|
||||
} catch (error) {
|
||||
return { success: false, error: error.message };
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ Bad - Breaks parent contract
|
||||
export class PaypalPaymentProcessor extends PaymentProcessor {
|
||||
async process(amount: number): Promise<PaymentResult> {
|
||||
if (amount > 10000) {
|
||||
throw new Error("Amount too high"); // Unexpected behavior!
|
||||
}
|
||||
// Different behavior than parent contract
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Checklist**:
|
||||
|
||||
- [ ] Child classes don't weaken preconditions
|
||||
- [ ] Child classes don't strengthen postconditions
|
||||
- [ ] No unexpected exceptions in overridden methods
|
||||
- [ ] Maintains parent class invariants
|
||||
|
||||
### 4. Interface Segregation Principle (ISP)
|
||||
|
||||
**Rule**: Small, focused interfaces over large ones
|
||||
|
||||
**Application**:
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Segregated interfaces
|
||||
export interface Readable {
|
||||
read(id: string): Promise<User | null>;
|
||||
}
|
||||
|
||||
export interface Writable {
|
||||
create(user: User): Promise<void>;
|
||||
update(user: User): Promise<void>;
|
||||
}
|
||||
|
||||
export interface Deletable {
|
||||
delete(id: string): Promise<void>;
|
||||
}
|
||||
|
||||
// Repositories implement only what they need
|
||||
export class ReadOnlyUserRepository implements Readable {
|
||||
async read(id: string): Promise<User | null> {
|
||||
// Implementation
|
||||
}
|
||||
}
|
||||
|
||||
export class FullUserRepository implements Readable, Writable, Deletable {
|
||||
// Implements all operations
|
||||
}
|
||||
|
||||
// ❌ Bad - Fat interface
|
||||
export interface UserRepository {
|
||||
read(id: string): Promise<User | null>;
|
||||
create(user: User): Promise<void>;
|
||||
update(user: User): Promise<void>;
|
||||
delete(id: string): Promise<void>;
|
||||
archive(id: string): Promise<void>;
|
||||
restore(id: string): Promise<void>;
|
||||
// Forces all implementations to have all methods
|
||||
}
|
||||
```
|
||||
|
||||
**Checklist**:
|
||||
|
||||
- [ ] Interfaces have focused responsibilities
|
||||
- [ ] Clients depend only on methods they use
|
||||
- [ ] No empty or not-implemented methods in concrete classes
|
||||
|
||||
### 5. Dependency Inversion Principle (DIP)
|
||||
|
||||
**Rule**: Depend on abstractions, not concretions
|
||||
|
||||
**Application**:
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Depends on abstraction
|
||||
export interface UserRepository {
|
||||
save(user: User): Promise<void>;
|
||||
findById(id: string): Promise<User | null>;
|
||||
}
|
||||
|
||||
export class CreateUserUseCase {
|
||||
constructor(private userRepository: UserRepository) {}
|
||||
|
||||
async execute(data: CreateUserDto): Promise<User> {
|
||||
const user = new User(data);
|
||||
await this.userRepository.save(user);
|
||||
return user;
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ Bad - Depends on concrete implementation
|
||||
export class CreateUserUseCase {
|
||||
constructor(private postgresUserRepository: PostgresUserRepository) {}
|
||||
|
||||
async execute(data: CreateUserDto): Promise<User> {
|
||||
// Tightly coupled to PostgreSQL implementation
|
||||
const user = new User(data);
|
||||
await this.postgresUserRepository.insertIntoPostgres(user);
|
||||
return user;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Checklist**:
|
||||
|
||||
- [ ] High-level modules depend on interfaces
|
||||
- [ ] Low-level modules implement interfaces
|
||||
- [ ] Dependencies flow toward abstractions
|
||||
- [ ] Easy to swap implementations for testing
|
||||
|
||||
## Part 2: Clean Code Principles (Simplicity & Pragmatism)
|
||||
|
||||
Clean Code principles emphasize simplicity, readability, and avoiding over-engineering.
|
||||
|
||||
### KISS - Keep It Simple, Stupid
|
||||
|
||||
**Rule**: Simplicity is the ultimate sophistication
|
||||
|
||||
**Application:**
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Simple and clear
|
||||
export class PasswordValidator {
|
||||
validate(password: string): boolean {
|
||||
return (
|
||||
password.length >= 8 && /[A-Z]/.test(password) && /[0-9]/.test(password)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ Bad - Over-engineered
|
||||
export class PasswordValidator {
|
||||
private rules: ValidationRule[] = [];
|
||||
private ruleEngine: RuleEngine;
|
||||
private strategyFactory: StrategyFactory;
|
||||
private policyManager: PolicyManager;
|
||||
|
||||
validate(password: string): ValidationResult {
|
||||
return this.ruleEngine
|
||||
.withStrategy(this.strategyFactory.create("password"))
|
||||
.withPolicy(this.policyManager.getDefault())
|
||||
.applyRules(this.rules)
|
||||
.execute(password);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**When KISS applies:**
|
||||
|
||||
- Simple requirements don't need complex solutions
|
||||
- Straightforward logic should stay straightforward
|
||||
- Don't create abstractions "just in case"
|
||||
- Readability > Cleverness
|
||||
|
||||
**Checklist:**
|
||||
|
||||
- [ ] Solution is as simple as possible (but no simpler)
|
||||
- [ ] No unnecessary abstractions or patterns
|
||||
- [ ] Code is easy to understand at first glance
|
||||
- [ ] No premature optimization
|
||||
|
||||
### YAGNI - You Aren't Gonna Need It
|
||||
|
||||
**Rule**: Build only what you need right now
|
||||
|
||||
**Application:**
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Build only what's needed NOW
|
||||
export class UserService {
|
||||
async createUser(dto: CreateUserDto): Promise<User> {
|
||||
return this.repository.save(new User(dto));
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ Bad - Building for imaginary future needs
|
||||
export class UserService {
|
||||
// We don't need these yet!
|
||||
async createUser(dto: CreateUserDto): Promise<User> {}
|
||||
async createUserBatch(dtos: CreateUserDto[]): Promise<User[]> {}
|
||||
async createUserWithRetry(
|
||||
dto: CreateUserDto,
|
||||
maxRetries: number
|
||||
): Promise<User> {}
|
||||
async createUserAsync(dto: CreateUserDto): Promise<JobId> {}
|
||||
async createUserWithCallback(
|
||||
dto: CreateUserDto,
|
||||
callback: Function
|
||||
): Promise<void> {}
|
||||
async createUserWithHooks(dto: CreateUserDto, hooks: Hooks): Promise<User> {}
|
||||
}
|
||||
```
|
||||
|
||||
**When YAGNI applies:**
|
||||
|
||||
- Feature is not in current requirements
|
||||
- "We might need this later" scenarios
|
||||
- Unused parameters or methods
|
||||
- Speculative generalization
|
||||
|
||||
**Checklist:**
|
||||
|
||||
- [ ] Feature is required by current user story
|
||||
- [ ] No "we might need this later" code
|
||||
- [ ] No unused parameters or methods
|
||||
- [ ] Will refactor when new requirements actually arrive
|
||||
|
||||
### DRY - Don't Repeat Yourself
|
||||
|
||||
**Rule**: Apply abstraction after seeing duplication 3 times (Rule of Three)
|
||||
|
||||
**Application:**
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Meaningful abstraction after Rule of Three
|
||||
export class DateFormatter {
|
||||
formatToISO(date: Date): string {
|
||||
return date.toISOString();
|
||||
}
|
||||
|
||||
formatToDisplay(date: Date): string {
|
||||
return date.toLocaleDateString("en-US");
|
||||
}
|
||||
|
||||
formatToRelative(date: Date): string {
|
||||
const now = new Date();
|
||||
const diff = now.getTime() - date.getTime();
|
||||
const days = Math.floor(diff / (1000 * 60 * 60 * 24));
|
||||
|
||||
if (days === 0) return "Today";
|
||||
if (days === 1) return "Yesterday";
|
||||
return `${days} days ago`;
|
||||
}
|
||||
}
|
||||
|
||||
// Used in 3+ places
|
||||
const isoDate = dateFormatter.formatToISO(user.createdAt);
|
||||
|
||||
// ❌ Bad - Premature abstraction
|
||||
// Don't abstract after seeing duplication just ONCE
|
||||
// Wait for the Rule of Three (3 occurrences)
|
||||
|
||||
// ❌ Bad - Wrong abstraction
|
||||
export class StringHelper {
|
||||
doSomething(str: string, num: number, bool: boolean): string {
|
||||
// Forcing unrelated code into one function
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**When DRY applies:**
|
||||
|
||||
- Same code appears 3+ times (Rule of Three)
|
||||
- Logic is truly identical, not just similar
|
||||
- Abstraction makes code clearer, not more complex
|
||||
- Change in one place should affect all uses
|
||||
|
||||
**When NOT to apply DRY:**
|
||||
|
||||
- Code looks similar but represents different concepts
|
||||
- Duplication is better than wrong abstraction
|
||||
- Abstraction adds more complexity than it removes
|
||||
- Only 1-2 occurrences
|
||||
|
||||
**Checklist:**
|
||||
|
||||
- [ ] Duplication appears 3+ times
|
||||
- [ ] Logic is truly identical
|
||||
- [ ] Abstraction is clearer than duplication
|
||||
- [ ] Not forcing unrelated concepts together
|
||||
|
||||
### TDA - Tell, Don't Ask
|
||||
|
||||
**Rule**: Tell objects what to do, don't ask for data and make decisions
|
||||
|
||||
**Application:**
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Tell the object what to do
|
||||
export class User {
|
||||
private _isActive: boolean = true;
|
||||
private _failedLoginAttempts: number = 0;
|
||||
|
||||
deactivate(): void {
|
||||
if (!this._isActive) {
|
||||
throw new Error("User already inactive");
|
||||
}
|
||||
this._isActive = false;
|
||||
this.logDeactivation();
|
||||
}
|
||||
|
||||
recordFailedLogin(): void {
|
||||
this._failedLoginAttempts++;
|
||||
if (this._failedLoginAttempts >= 5) {
|
||||
this.lock();
|
||||
}
|
||||
}
|
||||
|
||||
private lock(): void {
|
||||
this._isActive = false;
|
||||
this.logLockout();
|
||||
}
|
||||
|
||||
private logDeactivation(): void {
|
||||
console.log(`User ${this.id} deactivated`);
|
||||
}
|
||||
|
||||
private logLockout(): void {
|
||||
console.log(`User ${this.id} locked due to failed login attempts`);
|
||||
}
|
||||
}
|
||||
|
||||
// Usage - Tell it what to do
|
||||
user.deactivate();
|
||||
user.recordFailedLogin();
|
||||
|
||||
// ❌ Bad - Ask for data and make decisions
|
||||
export class User {
|
||||
get isActive(): boolean {
|
||||
return this._isActive;
|
||||
}
|
||||
|
||||
set isActive(value: boolean) {
|
||||
this._isActive = value;
|
||||
}
|
||||
|
||||
get failedLoginAttempts(): number {
|
||||
return this._failedLoginAttempts;
|
||||
}
|
||||
|
||||
set failedLoginAttempts(value: number) {
|
||||
this._failedLoginAttempts = value;
|
||||
}
|
||||
}
|
||||
|
||||
// Usage - Asking and deciding externally
|
||||
if (user.isActive) {
|
||||
user.isActive = false;
|
||||
console.log(`User ${user.id} deactivated`);
|
||||
}
|
||||
|
||||
if (user.failedLoginAttempts >= 5) {
|
||||
user.isActive = false;
|
||||
console.log(`User ${user.id} locked`);
|
||||
}
|
||||
```
|
||||
|
||||
**When TDA applies:**
|
||||
|
||||
- Object has data and related business logic
|
||||
- Decision-making should be encapsulated
|
||||
- Behavior belongs with the data
|
||||
- Multiple clients need the same operation
|
||||
|
||||
**Benefits:**
|
||||
|
||||
- Encapsulation of business logic
|
||||
- Reduces coupling
|
||||
- Easier to maintain and test
|
||||
- Single source of truth for behavior
|
||||
|
||||
**Checklist:**
|
||||
|
||||
- [ ] Business logic lives with the data
|
||||
- [ ] Methods are commands, not just getters
|
||||
- [ ] Clients tell, don't ask
|
||||
- [ ] Encapsulation is preserved
|
||||
|
||||
## Part 3: Function Design & Code Organization
|
||||
|
||||
### Keep Functions Small
|
||||
|
||||
**Target**: < 20 lines per function
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Small, focused functions
|
||||
export class CreateUserUseCase {
|
||||
async execute(dto: CreateUserDto): Promise<User> {
|
||||
this.validateDto(dto);
|
||||
const user = await this.createUser(dto);
|
||||
await this.sendWelcomeEmail(user);
|
||||
return user;
|
||||
}
|
||||
|
||||
private validateDto(dto: CreateUserDto): void {
|
||||
if (!this.isValidEmail(dto.email)) {
|
||||
throw new ValidationError("Invalid email");
|
||||
}
|
||||
}
|
||||
|
||||
private async createUser(dto: CreateUserDto): Promise<User> {
|
||||
const hashedPassword = await this.hasher.hash(dto.password);
|
||||
return this.repository.save(new User(dto, hashedPassword));
|
||||
}
|
||||
|
||||
private async sendWelcomeEmail(user: User): Promise<void> {
|
||||
await this.emailService.send(
|
||||
user.email,
|
||||
"Welcome",
|
||||
this.getWelcomeMessage(user.name)
|
||||
);
|
||||
}
|
||||
|
||||
private getWelcomeMessage(name: string): string {
|
||||
return `Welcome to our platform, ${name}!`;
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ Bad - One giant function
|
||||
export class CreateUserUseCase {
|
||||
async execute(dto: CreateUserDto): Promise<User> {
|
||||
// 100+ lines of validation, hashing, saving, emailing...
|
||||
// Hard to test, hard to read, hard to maintain
|
||||
return User;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Guidelines:**
|
||||
|
||||
- Prefer < 20 lines per function
|
||||
- Single purpose per function
|
||||
- Extract complex logic into separate methods
|
||||
- No side effects (pure functions when possible)
|
||||
|
||||
### Meaningful Names Over Comments
|
||||
|
||||
```typescript
|
||||
// ❌ Bad - Comments explaining WHAT
|
||||
export class UserService {
|
||||
// Check if user is active and not deleted
|
||||
async isValid(u: User): Promise<boolean> {
|
||||
return u.a && !u.d;
|
||||
}
|
||||
}
|
||||
|
||||
// ✅ Good - Self-documenting code
|
||||
export class UserService {
|
||||
async isActiveAndNotDeleted(user: User): Promise<boolean> {
|
||||
return user.isActive && !user.isDeleted;
|
||||
}
|
||||
}
|
||||
|
||||
// ✅ Comments explain WHY when needed
|
||||
export class PaymentService {
|
||||
async processPayment(amount: number): Promise<void> {
|
||||
// Stripe requires amount in cents, not dollars
|
||||
const amountInCents = amount * 100;
|
||||
await this.stripe.charge(amountInCents);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Comment Guidelines:**
|
||||
|
||||
- Explain **WHY**, not **WHAT**
|
||||
- Delete obsolete comments immediately
|
||||
- Prefer self-documenting code
|
||||
- Use comments for business rules and non-obvious decisions
|
||||
|
||||
**For function and variable naming conventions, see `naming-conventions` skill**
|
||||
|
||||
### Single Level of Abstraction
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Same level of abstraction
|
||||
async function processOrder(orderId: string): Promise<void> {
|
||||
const order = await fetchOrder(orderId);
|
||||
validateOrder(order);
|
||||
await chargeCustomer(order);
|
||||
await sendConfirmation(order);
|
||||
}
|
||||
|
||||
// ❌ Bad - Mixed levels of abstraction
|
||||
async function processOrder(orderId: string): Promise<void> {
|
||||
const order = await db.query("SELECT * FROM orders WHERE id = ?", [orderId]);
|
||||
|
||||
if (!order.items || order.items.length === 0) {
|
||||
throw new Error("Invalid order");
|
||||
}
|
||||
|
||||
await chargeCustomer(order);
|
||||
|
||||
const html = "<html><body>Order confirmed</body></html>";
|
||||
await emailService.send(order.customerEmail, html);
|
||||
}
|
||||
```
|
||||
|
||||
### Early Returns
|
||||
|
||||
```typescript
|
||||
// ✅ Good - Early returns reduce nesting
|
||||
function calculateDiscount(user: User, amount: number): number {
|
||||
if (!user.isActive) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (amount < 100) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (user.isPremium) {
|
||||
return amount * 0.2;
|
||||
}
|
||||
|
||||
return amount * 0.1;
|
||||
}
|
||||
|
||||
// ❌ Bad - Deep nesting
|
||||
function calculateDiscount(user: User, amount: number): number {
|
||||
let discount = 0;
|
||||
|
||||
if (user.isActive) {
|
||||
if (amount >= 100) {
|
||||
if (user.isPremium) {
|
||||
discount = amount * 0.2;
|
||||
} else {
|
||||
discount = amount * 0.1;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return discount;
|
||||
}
|
||||
```
|
||||
|
||||
## When to Apply Principles
|
||||
|
||||
### ✅ Apply When:
|
||||
|
||||
- **Complex business logic** that will evolve over time
|
||||
- **Multiple implementations** of the same concept needed
|
||||
- **Team projects** requiring clear boundaries and contracts
|
||||
- **Testability** is critical (need mocks/stubs)
|
||||
- **Long-term maintainability** is a priority
|
||||
|
||||
### ❌ Don't Over-Apply When:
|
||||
|
||||
- **Simple CRUD operations** with stable requirements
|
||||
- **Small scripts or utilities** (< 100 lines)
|
||||
- **Prototypes or POCs** for quick validation
|
||||
- **Performance-critical code** where abstraction adds overhead
|
||||
- **When it adds complexity** without clear benefit
|
||||
|
||||
## Balancing Principles
|
||||
|
||||
### When Principles Conflict
|
||||
|
||||
**KISS vs DRY:**
|
||||
|
||||
- Prefer KISS for simple cases
|
||||
- Apply DRY only after Rule of Three
|
||||
- Duplication is better than wrong abstraction
|
||||
|
||||
**YAGNI vs Future-Proofing:**
|
||||
|
||||
- Start with YAGNI
|
||||
- Refactor when requirements actually arrive
|
||||
- Don't over-engineer for hypothetical futures
|
||||
|
||||
**SOLID vs KISS:**
|
||||
|
||||
- Apply SOLID when complexity is justified
|
||||
- Don't force patterns where they don't fit
|
||||
- Simple problems deserve simple solutions
|
||||
|
||||
**TDA vs Simple Data Objects:**
|
||||
|
||||
- Use TDA for business logic
|
||||
- Simple DTOs don't need behavior
|
||||
- Value objects can be simple if immutable
|
||||
|
||||
## Common Anti-Patterns
|
||||
|
||||
### God Classes
|
||||
|
||||
```typescript
|
||||
// ❌ Classes doing too much (violates SRP)
|
||||
export class UserService {
|
||||
validateUser() {}
|
||||
hashPassword() {}
|
||||
sendEmail() {}
|
||||
saveToDatabase() {}
|
||||
generateReport() {}
|
||||
processPayment() {}
|
||||
}
|
||||
```
|
||||
|
||||
### Premature Optimization
|
||||
|
||||
```typescript
|
||||
// ❌ Don't optimize before measuring
|
||||
const cache = new Map<string, User>();
|
||||
const lruCache = new LRUCache<string, User>(1000);
|
||||
const bloomFilter = new BloomFilter();
|
||||
|
||||
// ✅ Start simple, optimize when needed
|
||||
const users = await repository.findAll();
|
||||
```
|
||||
|
||||
### Clever Code
|
||||
|
||||
```typescript
|
||||
// ❌ Clever but unreadable
|
||||
const result = arr.reduce((a, b) => a + (b.active ? 1 : 0), 0);
|
||||
|
||||
// ✅ Clear and boring
|
||||
const activeCount = users.filter((user) => user.isActive).length;
|
||||
```
|
||||
|
||||
### Magic Numbers
|
||||
|
||||
```typescript
|
||||
// ❌ Magic numbers
|
||||
if (user.age > 18 && order.amount < 1000) {
|
||||
// ...
|
||||
}
|
||||
|
||||
// ✅ Named constants
|
||||
const MINIMUM_AGE = 18;
|
||||
const MAXIMUM_ORDER_AMOUNT = 1000;
|
||||
|
||||
if (user.age > MINIMUM_AGE && order.amount < MAXIMUM_ORDER_AMOUNT) {
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
## Validation Checklist
|
||||
|
||||
Before finalizing code, verify:
|
||||
|
||||
**SOLID Principles:**
|
||||
|
||||
- [ ] Each class has a single, well-defined responsibility
|
||||
- [ ] New features can be added without modifying existing code
|
||||
- [ ] Subtypes are truly substitutable for their base types
|
||||
- [ ] No class is forced to implement unused interface methods
|
||||
- [ ] Dependencies point toward abstractions, not implementations
|
||||
|
||||
**Clean Code Principles:**
|
||||
|
||||
- [ ] Solution is as simple as possible (KISS)
|
||||
- [ ] Only building what's needed now (YAGNI)
|
||||
- [ ] Duplication abstracted after Rule of Three (DRY)
|
||||
- [ ] Objects encapsulate behavior (TDA)
|
||||
- [ ] Functions are < 20 lines
|
||||
- [ ] Names are meaningful and reveal intention
|
||||
- [ ] Code is self-documenting
|
||||
- [ ] Early returns reduce nesting
|
||||
- [ ] Single level of abstraction per function
|
||||
|
||||
**Overall:**
|
||||
|
||||
- [ ] Principles aren't creating unnecessary complexity
|
||||
- [ ] Balance between design and pragmatism
|
||||
|
||||
## Complete Example: Applying All Principles
|
||||
|
||||
```typescript
|
||||
// SRP + DIP: Each class has one responsibility, depends on abstractions
|
||||
export interface Logger {
|
||||
log(message: string): void;
|
||||
}
|
||||
|
||||
export interface UserRepository {
|
||||
save(user: User): Promise<void>;
|
||||
findByEmail(email: string): Promise<User | null>;
|
||||
}
|
||||
|
||||
export interface PasswordHasher {
|
||||
hash(password: string): Promise<string>;
|
||||
}
|
||||
|
||||
export interface EmailSender {
|
||||
send(to: string, subject: string, body: string): Promise<void>;
|
||||
}
|
||||
|
||||
// OCP: Open for extension (new implementations)
|
||||
export class ConsoleLogger implements Logger {
|
||||
log(message: string): void {
|
||||
console.log(message);
|
||||
}
|
||||
}
|
||||
|
||||
// ISP: Focused interfaces
|
||||
// Each interface has a single, focused responsibility
|
||||
|
||||
// KISS: Simple, clear implementation
|
||||
export class CreateUserUseCase {
|
||||
constructor(
|
||||
private userRepository: UserRepository,
|
||||
private passwordHasher: PasswordHasher,
|
||||
private logger: Logger,
|
||||
private emailSender: EmailSender
|
||||
) {}
|
||||
|
||||
// KISS + Small Functions: < 20 lines, single responsibility
|
||||
async execute(data: CreateUserDto): Promise<User> {
|
||||
this.logger.log("Creating new user");
|
||||
|
||||
// YAGNI: Only what's needed now
|
||||
await this.validateEmail(data.email);
|
||||
const user = await this.createUser(data);
|
||||
await this.sendWelcomeEmail(user);
|
||||
|
||||
this.logger.log("User created successfully");
|
||||
return user;
|
||||
}
|
||||
|
||||
// DRY: Extracted after Rule of Three
|
||||
private async validateEmail(email: string): Promise<void> {
|
||||
const existing = await this.userRepository.findByEmail(email);
|
||||
if (existing) {
|
||||
throw new Error(`User with email ${email} already exists`);
|
||||
}
|
||||
}
|
||||
|
||||
private async createUser(data: CreateUserDto): Promise<User> {
|
||||
const hashedPassword = await this.passwordHasher.hash(data.password);
|
||||
const user = new User({ ...data, password: hashedPassword });
|
||||
await this.userRepository.save(user);
|
||||
return user;
|
||||
}
|
||||
|
||||
private async sendWelcomeEmail(user: User): Promise<void> {
|
||||
await this.emailSender.send(
|
||||
user.email,
|
||||
"Welcome",
|
||||
this.getWelcomeMessage(user.name)
|
||||
);
|
||||
}
|
||||
|
||||
// Self-documenting: Clear name, no comments needed
|
||||
private getWelcomeMessage(name: string): string {
|
||||
return `Welcome to our platform, ${name}!`;
|
||||
}
|
||||
}
|
||||
|
||||
// LSP: Implementations are substitutable
|
||||
export class BcryptPasswordHasher implements PasswordHasher {
|
||||
async hash(password: string): Promise<string> {
|
||||
return bcrypt.hash(password, 10);
|
||||
}
|
||||
}
|
||||
|
||||
export class ArgonPasswordHasher implements PasswordHasher {
|
||||
async hash(password: string): Promise<string> {
|
||||
return argon2.hash(password);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Integration with Architecture
|
||||
|
||||
**SOLID + Clean Architecture:**
|
||||
|
||||
- Domain entities use TDA (behavior with data)
|
||||
- Use cases apply SRP (single responsibility)
|
||||
- Repositories follow DIP (depend on interfaces)
|
||||
- Infrastructure implements OCP (extend, don't modify)
|
||||
|
||||
**Clean Code + KISS:**
|
||||
|
||||
- Apply SOLID only when complexity is justified
|
||||
- Don't create abstractions until you need them (YAGNI)
|
||||
- Balance abstraction with code simplicity
|
||||
|
||||
## Remember
|
||||
|
||||
**Quality over dogma:**
|
||||
|
||||
- Apply principles when they improve code, not just for the sake of it
|
||||
- Context matters: Simple code doesn't need complex architecture
|
||||
- Refactor gradually: Don't force patterns on existing code all at once
|
||||
|
||||
**Communication over cleverness:**
|
||||
|
||||
- Code is read 10x more than written
|
||||
- Clear, boring code > clever, complex code
|
||||
- Your future self will thank you
|
||||
|
||||
**Pragmatism over perfection:**
|
||||
|
||||
- SOLID principles make testing easier - use this as a guide
|
||||
- Simple problems deserve simple solutions
|
||||
- Test-driven: Let tests guide your design
|
||||
Reference in New Issue
Block a user