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.

// ❌ 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<User> {
    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.

// ❌ 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<PaymentResult>;
}

class CreditCardPayment implements PaymentMethod {
  async process(amount: number): Promise<PaymentResult> {
    // Process credit card
    return { success: true };
  }
}

class PayPalPayment implements PaymentMethod {
  async process(amount: number): Promise<PaymentResult> {
    // Process PayPal
    return { success: true };
  }
}

class PaymentProcessor {
  async process(method: PaymentMethod, amount: number): Promise<PaymentResult> {
    return await method.process(amount);
  }
}

// Add new payment method without modifying existing code
class BitcoinPayment implements PaymentMethod {
  async process(amount: number): Promise<PaymentResult> {
    // Process Bitcoin
    return { success: true };
  }
}

L - Liskov Substitution Principle

Definition: Subtypes must be substitutable for their base types.

// ❌ 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.

// ❌ 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.

// ❌ 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<any>;
}

class MySQLDatabase implements Database {
  async query(sql: string): Promise<any> {
    // MySQL implementation
  }
}

class PostgreSQLDatabase implements Database {
  async query(sql: string): Promise<any> {
    // 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

// ❌ 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)

// ❌ 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)

// ❌ 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

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

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

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

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

// ❌ 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

// ❌ 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<void> { /* ... */ }
  find(id: string): Promise<User> { /* ... */ }
}

class UserAuthService {
  authenticate(credentials: Credentials): Promise<Token> { /* ... */ }
}

Duplicate Code

// ❌ 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

// ❌ 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

// 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

// 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

// 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

// ✅ 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

// ✅ 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

// ✅ Noun or noun phrase
class UserRepository { /* ... */ }
class EmailValidator { /* ... */ }
class PaymentProcessor { /* ... */ }
class DatabaseConnection { /* ... */ }

// ❌ Vague or verb names
class Manager { /* ... */ }
class Handler { /* ... */ }
class Process { /* ... */ }

Constants

// ✅ 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

// ❌ 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

// ❌ 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

// ❌ 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

/**
 * 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

## 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.