--- name: code-quality-standards description: Code quality standards including SOLID principles, design patterns, code smells, refactoring techniques, naming conventions, and technical debt management. Use when reviewing code, refactoring, ensuring quality, or detecting code smells. --- # Code Quality Standards This skill provides comprehensive guidance for writing clean, maintainable, and high-quality code. ## SOLID Principles ### S - Single Responsibility Principle **Definition**: A class should have only one reason to change. ```typescript // ❌ BAD - Multiple responsibilities class UserManager { createUser(data: UserData) { // Validation logic if (!data.email.includes('@')) throw new Error('Invalid email'); // Database logic const user = database.insert('users', data); // Email logic emailService.send(data.email, 'Welcome!'); // Logging logic logger.info(`User created: ${data.email}`); return user; } } // ✅ GOOD - Single responsibility per class class UserValidator { validate(data: UserData): void { if (!data.email.includes('@')) { throw new Error('Invalid email'); } } } class UserRepository { create(data: UserData): User { return database.insert('users', data); } } class UserNotificationService { sendWelcomeEmail(email: string): void { emailService.send(email, 'Welcome!'); } } class UserService { constructor( private validator: UserValidator, private repository: UserRepository, private notificationService: UserNotificationService, private logger: Logger ) {} async createUser(data: UserData): Promise { this.validator.validate(data); const user = await this.repository.create(data); await this.notificationService.sendWelcomeEmail(user.email); this.logger.info(`User created: ${user.email}`); return user; } } ``` ### O - Open/Closed Principle **Definition**: Classes should be open for extension but closed for modification. ```typescript // ❌ BAD - Must modify class to add new payment methods class PaymentProcessor { process(type: string, amount: number) { if (type === 'credit_card') { // Process credit card } else if (type === 'paypal') { // Process PayPal } else if (type === 'bitcoin') { // Process Bitcoin } } } // ✅ GOOD - Can extend without modifying interface PaymentMethod { process(amount: number): Promise; } class CreditCardPayment implements PaymentMethod { async process(amount: number): Promise { // Process credit card return { success: true }; } } class PayPalPayment implements PaymentMethod { async process(amount: number): Promise { // Process PayPal return { success: true }; } } class PaymentProcessor { async process(method: PaymentMethod, amount: number): Promise { return await method.process(amount); } } // Add new payment method without modifying existing code class BitcoinPayment implements PaymentMethod { async process(amount: number): Promise { // Process Bitcoin return { success: true }; } } ``` ### L - Liskov Substitution Principle **Definition**: Subtypes must be substitutable for their base types. ```typescript // ❌ BAD - Violates LSP class Rectangle { constructor(protected width: number, protected height: number) {} setWidth(width: number) { this.width = width; } setHeight(height: number) { this.height = height; } getArea(): number { return this.width * this.height; } } class Square extends Rectangle { setWidth(width: number) { this.width = width; this.height = width; // Violates expectation } setHeight(height: number) { this.width = height; // Violates expectation this.height = height; } } // ✅ GOOD - Separate abstractions interface Shape { getArea(): number; } class Rectangle implements Shape { constructor(private width: number, private height: number) {} getArea(): number { return this.width * this.height; } } class Square implements Shape { constructor(private side: number) {} getArea(): number { return this.side * this.side; } } ``` ### I - Interface Segregation Principle **Definition**: Clients shouldn't be forced to depend on interfaces they don't use. ```typescript // ❌ BAD - Fat interface interface Worker { work(): void; eat(): void; sleep(): void; getPaid(): void; } class HumanWorker implements Worker { work() { /* ... */ } eat() { /* ... */ } sleep() { /* ... */ } getPaid() { /* ... */ } } class RobotWorker implements Worker { work() { /* ... */ } eat() { /* Not applicable */ } sleep() { /* Not applicable */ } getPaid() { /* Not applicable */ } } // ✅ GOOD - Segregated interfaces interface Workable { work(): void; } interface Eatable { eat(): void; } interface Sleepable { sleep(): void; } interface Payable { getPaid(): void; } class HumanWorker implements Workable, Eatable, Sleepable, Payable { work() { /* ... */ } eat() { /* ... */ } sleep() { /* ... */ } getPaid() { /* ... */ } } class RobotWorker implements Workable { work() { /* ... */ } } ``` ### D - Dependency Inversion Principle **Definition**: Depend on abstractions, not concretions. ```typescript // ❌ BAD - Depends on concrete implementation class UserService { private database = new MySQLDatabase(); // Tight coupling async getUser(id: string) { return this.database.query(`SELECT * FROM users WHERE id = ${id}`); } } // ✅ GOOD - Depends on abstraction interface Database { query(sql: string): Promise; } class MySQLDatabase implements Database { async query(sql: string): Promise { // MySQL implementation } } class PostgreSQLDatabase implements Database { async query(sql: string): Promise { // PostgreSQL implementation } } class UserService { constructor(private database: Database) {} // Dependency injection async getUser(id: string) { return this.database.query(`SELECT * FROM users WHERE id = ${id}`); } } // Can easily swap database implementations const userService = new UserService(new PostgreSQLDatabase()); ``` ## DRY (Don't Repeat Yourself) ### Identifying Duplication ```typescript // ❌ BAD - Repeated validation logic function createUser(data: UserData) { 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 } function updateUser(id: string, data: UserData) { if (!data.email || !data.email.includes('@')) { throw new Error('Invalid email'); } if (!data.password || data.password.length < 8) { throw new Error('Password too short'); } // Update user } // ✅ GOOD - Extract common logic function validateUserData(data: UserData): void { if (!data.email || !data.email.includes('@')) { throw new Error('Invalid email'); } if (!data.password || data.password.length < 8) { throw new Error('Password too short'); } } function createUser(data: UserData) { validateUserData(data); // Create user } function updateUser(id: string, data: UserData) { validateUserData(data); // Update user } ``` ## KISS (Keep It Simple, Stupid) ```typescript // ❌ BAD - Over-engineered class NumberProcessor { private strategy: ProcessingStrategy; constructor(strategy: ProcessingStrategy) { this.strategy = strategy; } process(numbers: number[]): number[] { return this.strategy.execute(numbers); } } interface ProcessingStrategy { execute(numbers: number[]): number[]; } class MultiplyByTwoStrategy implements ProcessingStrategy { execute(numbers: number[]): number[] { return numbers.map(n => n * 2); } } // ✅ GOOD - Simple and clear function multiplyByTwo(numbers: number[]): number[] { return numbers.map(n => n * 2); } ``` ## YAGNI (You Aren't Gonna Need It) ```typescript // ❌ BAD - Building features you might need class User { id: string; email: string; name: string; // Future features that aren't needed yet preferences?: UserPreferences; badges?: Badge[]; followers?: User[]; following?: User[]; achievements?: Achievement[]; notifications?: Notification[]; } // ✅ GOOD - Only what you need now class User { id: string; email: string; name: string; } // Add features when actually needed ``` ## Design Patterns ### Factory Pattern ```typescript interface Animal { speak(): string; } class Dog implements Animal { speak(): string { return 'Woof!'; } } class Cat implements Animal { speak(): string { return 'Meow!'; } } class AnimalFactory { static create(type: 'dog' | 'cat'): Animal { switch (type) { case 'dog': return new Dog(); case 'cat': return new Cat(); default: throw new Error('Unknown animal type'); } } } // Usage const dog = AnimalFactory.create('dog'); console.log(dog.speak()); // "Woof!" ``` ### Strategy Pattern ```typescript interface SortStrategy { sort(data: number[]): number[]; } class QuickSort implements SortStrategy { sort(data: number[]): number[] { // Quick sort implementation return data.sort((a, b) => a - b); } } class MergeSort implements SortStrategy { sort(data: number[]): number[] { // Merge sort implementation return data.sort((a, b) => a - b); } } class Sorter { constructor(private strategy: SortStrategy) {} setStrategy(strategy: SortStrategy) { this.strategy = strategy; } sort(data: number[]): number[] { return this.strategy.sort(data); } } // Usage const sorter = new Sorter(new QuickSort()); sorter.sort([3, 1, 4, 1, 5]); sorter.setStrategy(new MergeSort()); sorter.sort([3, 1, 4, 1, 5]); ``` ### Observer Pattern ```typescript interface Observer { update(data: any): void; } class Subject { private observers: Observer[] = []; attach(observer: Observer): void { this.observers.push(observer); } detach(observer: Observer): void { const index = this.observers.indexOf(observer); if (index > -1) { this.observers.splice(index, 1); } } notify(data: any): void { for (const observer of this.observers) { observer.update(data); } } } class EmailNotifier implements Observer { update(data: any): void { console.log(`Sending email: ${data}`); } } class SMSNotifier implements Observer { update(data: any): void { console.log(`Sending SMS: ${data}`); } } // Usage const subject = new Subject(); subject.attach(new EmailNotifier()); subject.attach(new SMSNotifier()); subject.notify('New user registered!'); ``` ### Singleton Pattern ```typescript class Database { private static instance: Database; private connection: any; private constructor() { // Private constructor prevents direct instantiation this.connection = this.createConnection(); } static getInstance(): Database { if (!Database.instance) { Database.instance = new Database(); } return Database.instance; } private createConnection() { // Create database connection return {}; } query(sql: string) { // Execute query } } // Usage - Always returns same instance const db1 = Database.getInstance(); const db2 = Database.getInstance(); console.log(db1 === db2); // true ``` ## Code Smells and Detection ### Long Method ```typescript // ❌ BAD - Method too long (>20 lines) function processOrder(order: Order) { // Validate order (10 lines) // Calculate totals (15 lines) // Apply discounts (20 lines) // Process payment (25 lines) // Send confirmation (10 lines) // Update inventory (15 lines) // Log transaction (5 lines) } // ✅ GOOD - Extract methods function processOrder(order: Order) { validateOrder(order); const total = calculateTotal(order); const discounted = applyDiscounts(total, order.promoCode); processPayment(discounted); sendConfirmation(order.email); updateInventory(order.items); logTransaction(order.id); } ``` ### Large Class ```typescript // ❌ BAD - Too many responsibilities class User { // User properties (20+ fields) // User validation (10 methods) // User persistence (10 methods) // User authentication (5 methods) // User notifications (5 methods) // User reporting (5 methods) } // ✅ GOOD - Split into focused classes class User { id: string; email: string; name: string; } class UserValidator { validate(user: User): boolean { /* ... */ } } class UserRepository { save(user: User): Promise { /* ... */ } find(id: string): Promise { /* ... */ } } class UserAuthService { authenticate(credentials: Credentials): Promise { /* ... */ } } ``` ### Duplicate Code ```typescript // ❌ BAD - Duplicated logic function calculateEmployeeSalary(employee: Employee) { let salary = employee.baseSalary; salary += employee.baseSalary * 0.1; // 10% bonus salary += employee.baseSalary * 0.05; // 5% tax deduction return salary; } function calculateContractorSalary(contractor: Contractor) { let salary = contractor.baseSalary; salary += contractor.baseSalary * 0.1; // 10% bonus salary += contractor.baseSalary * 0.05; // 5% tax deduction return salary; } // ✅ GOOD - Extract common logic function calculateSalary(baseSalary: number): number { let salary = baseSalary; salary += baseSalary * 0.1; // 10% bonus salary += baseSalary * 0.05; // 5% tax deduction return salary; } function calculateEmployeeSalary(employee: Employee) { return calculateSalary(employee.baseSalary); } function calculateContractorSalary(contractor: Contractor) { return calculateSalary(contractor.baseSalary); } ``` ### God Object ```typescript // ❌ BAD - Knows/does too much class Application { database: Database; emailService: EmailService; paymentProcessor: PaymentProcessor; createUser() { /* ... */ } sendEmail() { /* ... */ } processPayment() { /* ... */ } generateReport() { /* ... */ } validateInput() { /* ... */ } // ... 50 more methods } // ✅ GOOD - Focused classes class UserService { createUser() { /* ... */ } } class NotificationService { sendEmail() { /* ... */ } } class PaymentService { processPayment() { /* ... */ } } ``` ## Refactoring Patterns ### Extract Method ```typescript // Before function printOwing(invoice: Invoice) { console.log('***********************'); console.log('**** Customer Owes ****'); console.log('***********************'); let outstanding = 0; for (const order of invoice.orders) { outstanding += order.amount; } console.log(`Name: ${invoice.customer}`); console.log(`Amount: ${outstanding}`); } // After function printOwing(invoice: Invoice) { printBanner(); const outstanding = calculateOutstanding(invoice); printDetails(invoice.customer, outstanding); } function printBanner() { console.log('***********************'); console.log('**** Customer Owes ****'); console.log('***********************'); } function calculateOutstanding(invoice: Invoice): number { return invoice.orders.reduce((sum, order) => sum + order.amount, 0); } function printDetails(customer: string, outstanding: number) { console.log(`Name: ${customer}`); console.log(`Amount: ${outstanding}`); } ``` ### Introduce Parameter Object ```typescript // Before function createUser( firstName: string, lastName: string, email: string, phone: string, address: string, city: string, state: string, zip: string ) { // Too many parameters } // After interface UserDetails { firstName: string; lastName: string; email: string; phone: string; address: Address; } interface Address { street: string; city: string; state: string; zip: string; } function createUser(details: UserDetails) { // Much cleaner } ``` ### Replace Conditional with Polymorphism ```typescript // Before class Bird { type: 'european' | 'african' | 'norwegian'; getSpeed(): number { switch (this.type) { case 'european': return 35; case 'african': return 40; case 'norwegian': return 24; } } } // After abstract class Bird { abstract getSpeed(): number; } class EuropeanBird extends Bird { getSpeed(): number { return 35; } } class AfricanBird extends Bird { getSpeed(): number { return 40; } } class NorwegianBird extends Bird { getSpeed(): number { return 24; } } ``` ## Naming Conventions ### Variables ```typescript // ✅ Descriptive, clear names const userEmail = 'user@example.com'; const totalPrice = 100; const isActive = true; const hasPermission = false; // ❌ Vague, unclear names const e = 'user@example.com'; const temp = 100; const flag = true; const data = {}; ``` ### Functions ```typescript // ✅ Verb + Noun for actions function getUserById(id: string): User { /* ... */ } function calculateTotalPrice(items: Item[]): number { /* ... */ } function isValidEmail(email: string): boolean { /* ... */ } function hasPermission(user: User, resource: string): boolean { /* ... */ } // ❌ Unclear names function user(id: string): User { /* ... */ } function price(items: Item[]): number { /* ... */ } function email(email: string): boolean { /* ... */ } ``` ### Classes ```typescript // ✅ Noun or noun phrase class UserRepository { /* ... */ } class EmailValidator { /* ... */ } class PaymentProcessor { /* ... */ } class DatabaseConnection { /* ... */ } // ❌ Vague or verb names class Manager { /* ... */ } class Handler { /* ... */ } class Process { /* ... */ } ``` ### Constants ```typescript // ✅ SCREAMING_SNAKE_CASE for true constants const MAX_RETRY_ATTEMPTS = 3; const API_BASE_URL = 'https://api.example.com'; const DEFAULT_TIMEOUT_MS = 5000; // ✅ camelCase for config objects const databaseConfig = { host: 'localhost', port: 5432, }; ``` ## Function/Method Size Guidelines ### Keep Functions Under 20 Lines ```typescript // ❌ BAD - Too long (40+ lines) function processOrder(order: Order) { // 40+ lines of logic } // ✅ GOOD - Split into smaller functions function processOrder(order: Order) { validateOrder(order); const total = calculateTotal(order); const payment = processPayment(order, total); sendConfirmation(order, payment); updateInventory(order); } function validateOrder(order: Order) { // 5-10 lines } function calculateTotal(order: Order): number { // 5-10 lines } ``` ### Maximum 3-4 Parameters ```typescript // ❌ BAD - Too many parameters function createUser( firstName: string, lastName: string, email: string, phone: string, role: string, department: string ) { /* ... */ } // ✅ GOOD - Use object parameter interface CreateUserParams { firstName: string; lastName: string; email: string; phone: string; role: string; department: string; } function createUser(params: CreateUserParams) { /* ... */ } ``` ## Cyclomatic Complexity ### Keep Complexity Under 10 ```typescript // ❌ BAD - Complexity > 10 function calculatePrice(item: Item, user: User): number { let price = item.basePrice; if (user.isPremium) { price *= 0.9; } if (item.category === 'electronics') { if (item.brand === 'Apple') { price *= 1.2; } else if (item.brand === 'Samsung') { price *= 1.1; } } if (user.location === 'CA') { price *= 1.08; } else if (user.location === 'NY') { price *= 1.09; } else if (user.location === 'TX') { price *= 1.06; } return price; } // ✅ GOOD - Split into focused functions function calculatePrice(item: Item, user: User): number { let price = item.basePrice; price = applyUserDiscount(price, user); price = applyBrandMarkup(price, item); price = applyLocationTax(price, user.location); return price; } function applyUserDiscount(price: number, user: User): number { return user.isPremium ? price * 0.9 : price; } function applyBrandMarkup(price: number, item: Item): number { const markups = { 'Apple': 1.2, 'Samsung': 1.1, }; return item.category === 'electronics' ? price * (markups[item.brand] || 1) : price; } function applyLocationTax(price: number, location: string): number { const taxRates = { 'CA': 1.08, 'NY': 1.09, 'TX': 1.06, }; return price * (taxRates[location] || 1); } ``` ## Technical Debt Management ### Document Technical Debt ```typescript /** * TODO: Refactor this function - it's too complex * DEBT: Using deprecated API, need to migrate to v2 * HACK: Workaround for bug in third-party library * FIXME: Race condition occurs under heavy load * OPTIMIZE: Query is slow, add database index */ ``` ### Track in Issue Tracker ```markdown ## Technical Debt Items ### High Priority - [ ] Refactor UserService - violates SRP (Est: 8h) - [ ] Replace deprecated payment API (Est: 16h) - [ ] Fix race condition in order processing (Est: 4h) ### Medium Priority - [ ] Optimize slow database queries (Est: 8h) - [ ] Add missing unit tests for AuthService (Est: 6h) ### Low Priority - [ ] Improve error messages (Est: 2h) - [ ] Update documentation (Est: 4h) ``` ### Allocate Time for Refactoring **20% Rule**: Spend 20% of sprint capacity on technical debt - 4 out of 10 story points - 1 out of 5 days per sprint - Prevents debt from accumulating ## Code Review Checklist **Functionality**: - [ ] Code does what it's supposed to do - [ ] Edge cases handled - [ ] Error handling in place **Design**: - [ ] Follows SOLID principles - [ ] No code smells - [ ] Appropriate design patterns used **Readability**: - [ ] Clear naming conventions - [ ] Functions under 20 lines - [ ] Cyclomatic complexity under 10 - [ ] Comments explain "why" not "what" **Tests**: - [ ] Unit tests added/updated - [ ] Test coverage adequate - [ ] Tests follow AAA pattern **Performance**: - [ ] No obvious performance issues - [ ] Database queries optimized - [ ] No N+1 query problems **Security**: - [ ] Input validation in place - [ ] No SQL injection vulnerabilities - [ ] Secrets not hardcoded ## When to Use This Skill Use this skill when: - Reviewing code in pull requests - Refactoring existing code - Setting code standards for team - Onboarding new developers - Conducting code quality audits - Planning technical debt reduction - Designing new features - Improving codebase maintainability - Training team on best practices - Establishing coding guidelines --- **Remember**: Code quality is not about perfection, but about maintainability. Write code that your future self and team members will thank you for.