code-quality-standards

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.

// ❌ 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 = '[e-mail]';
const totalPrice = 100;
const isActive = true;
const hasPermission = false;

// ❌ Vague, unclear names
const e = '[e-mail]';
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.