Code Quality and Refactoring: SOLID Principles and Clean Code

Code Quality and Refactoring: SOLID Principles and Clean Code

Introduction

High-quality code is maintainable, testable, and extensible. This guide covers SOLID principles for object-oriented design, common code smells and their remedies, practical refactoring techniques, and static analysis tools for JavaScript, Python, and C# that help maintain code quality.

SOLID Principles

Single Responsibility Principle (SRP)

Definition: A class should have only one reason to change.

❌ Violation:

// User class has too many responsibilities
class User {
    constructor(
        public name: string,
        public email: string
    ) {}
    
    // Database responsibility
    save(): void {
        const db = new Database();
        db.execute(`INSERT INTO users VALUES ('${this.name}', '${this.email}')`);
    }
    
    // Email responsibility
    sendWelcomeEmail(): void {
        const smtp = new SMTPClient();
        smtp.send(this.email, 'Welcome!', 'Thanks for joining...');
    }
    
    // Validation responsibility
    validate(): boolean {
        return this.email.includes('@') && this.name.length > 0;
    }
}

✅ Following SRP:

// Each class has single responsibility
class User {
    constructor(
        public name: string,
        public email: string
    ) {}
}

class UserValidator {
    validate(user: User): boolean {
        return user.email.includes('@') && user.name.length > 0;
    }
}

class UserRepository {
    async save(user: User): Promise<void> {
        await this.db.execute(
            'INSERT INTO users (name, email) VALUES (?, ?)',
            [user.name, user.email]
        );
    }
}

class EmailService {
    async sendWelcomeEmail(user: User): Promise<void> {
        await this.smtp.send({
            to: user.email,
            subject: 'Welcome!',
            body: 'Thanks for joining...'
        });
    }
}

Open/Closed Principle (OCP)

Definition: Software entities should be open for extension but closed for modification.

❌ Violation:

class PaymentProcessor:
    def process_payment(self, payment_type, amount):
        if payment_type == "credit_card":
            print(f"Processing credit card payment: ${amount}")
        elif payment_type == "paypal":
            print(f"Processing PayPal payment: ${amount}")
        elif payment_type == "bitcoin":  # Adding new type requires modifying class
            print(f"Processing Bitcoin payment: ${amount}")

✅ Following OCP:

from abc import ABC, abstractmethod

# Open for extension
class PaymentMethod(ABC):
    @abstractmethod
    def process(self, amount: float) -> None:
        pass

class CreditCardPayment(PaymentMethod):
    def process(self, amount: float) -> None:
        print(f"Processing credit card: ${amount}")

class PayPalPayment(PaymentMethod):
    def process(self, amount: float) -> None:
        print(f"Processing PayPal: ${amount}")

class BitcoinPayment(PaymentMethod):  # Extension doesn't modify existing code
    def process(self, amount: float) -> None:
        print(f"Processing Bitcoin: ${amount}")

class PaymentProcessor:
    def process_payment(self, method: PaymentMethod, amount: float) -> None:
        method.process(amount)

# Usage
processor = PaymentProcessor()
processor.process_payment(CreditCardPayment(), 99.99)
processor.process_payment(BitcoinPayment(), 0.002)

Liskov Substitution Principle (LSP)

Definition: Derived classes must be substitutable for their base classes.

❌ Violation:

public class Rectangle
{
    public virtual int Width { get; set; }
    public virtual int Height { get; set; }
    
    public int GetArea() => Width * Height;
}

public class Square : Rectangle
{
    // Violates LSP - changes behavior of base class
    public override int Width
    {
        get => base.Width;
        set
        {
            base.Width = value;
            base.Height = value;  // Side effect!
        }
    }
    
    public override int Height
    {
        get => base.Height;
        set
        {
            base.Width = value;   // Side effect!
            base.Height = value;
        }
    }
}

// This breaks when using Square
void ResizeRectangle(Rectangle rect)
{
    rect.Width = 5;
    rect.Height = 10;
    // Expects area to be 50, but Square gives 100!
    Console.WriteLine(rect.GetArea());
}

✅ Following LSP:

public interface IShape
{
    int GetArea();
}

public class Rectangle : IShape
{
    public int Width { get; set; }
    public int Height { get; set; }
    
    public int GetArea() => Width * Height;
}

public class Square : IShape
{
    public int Side { get; set; }
    
    public int GetArea() => Side * Side;
}

// Now works correctly with both shapes
void PrintArea(IShape shape)
{
    Console.WriteLine($"Area: {shape.GetArea()}");
}

Interface Segregation Principle (ISP)

Definition: Clients shouldn't be forced to depend on interfaces they don't use.

❌ Violation:

// Fat interface
interface Worker {
    work(): void;
    eat(): void;
    sleep(): void;
}

// Robot forced to implement eat() and sleep()
class RobotWorker implements Worker {
    work(): void {
        console.log("Working...");
    }
    
    eat(): void {
        throw new Error("Robots don't eat!");
    }
    
    sleep(): void {
        throw new Error("Robots don't sleep!");
    }
}

✅ Following ISP:

// Segregated interfaces
interface Workable {
    work(): void;
}

interface Eatable {
    eat(): void;
}

interface Sleepable {
    sleep(): void;
}

class HumanWorker implements Workable, Eatable, Sleepable {
    work(): void {
        console.log("Working...");
    }
    
    eat(): void {
        console.log("Eating lunch...");
    }
    
    sleep(): void {
        console.log("Sleeping...");
    }
}

class RobotWorker implements Workable {
    work(): void {
        console.log("Working 24/7...");
    }
}

Dependency Inversion Principle (DIP)

Definition: High-level modules shouldn't depend on low-level modules. Both should depend on abstractions.

❌ Violation:

# High-level module depends on low-level implementation
class MySQLDatabase:
    def save(self, data):
        print(f"Saving to MySQL: {data}")

class UserService:
    def __init__(self):
        self.db = MySQLDatabase()  # Tight coupling!
    
    def create_user(self, name):
        self.db.save(name)

✅ Following DIP:

from abc import ABC, abstractmethod

# Abstraction
class Database(ABC):
    @abstractmethod
    def save(self, data: str) -> None:
        pass

# Low-level implementations
class MySQLDatabase(Database):
    def save(self, data: str) -> None:
        print(f"Saving to MySQL: {data}")

class MongoDatabase(Database):
    def save(self, data: str) -> None:
        print(f"Saving to MongoDB: {data}")

# High-level module depends on abstraction
class UserService:
    def __init__(self, db: Database):
        self.db = db
    
    def create_user(self, name: str) -> None:
        self.db.save(name)

# Dependency injection
mysql_db = MySQLDatabase()
user_service = UserService(mysql_db)  # Easy to swap implementations

Common Code Smells

Long Method

Smell:

public void ProcessOrder(Order order)
{
    // 150 lines of code doing everything...
    // Validation
    if (order.Items.Count == 0) throw new Exception("Empty order");
    if (!order.Customer.IsActive) throw new Exception("Inactive customer");
    
    // Calculate totals
    decimal subtotal = 0;
    foreach (var item in order.Items)
    {
        subtotal += item.Price * item.Quantity;
    }
    decimal tax = subtotal * 0.08m;
    decimal shipping = CalculateShipping(order);
    decimal total = subtotal + tax + shipping;
    
    // Apply discounts
    if (order.Customer.IsPremium && total > 100)
    {
        total *= 0.9m;
    }
    
    // Process payment
    // ...
    
    // Send notifications
    // ...
}

Refactored:

public void ProcessOrder(Order order)
{
    ValidateOrder(order);
    var totals = CalculateOrderTotals(order);
    ApplyDiscounts(order, totals);
    ProcessPayment(order, totals.Final);
    SendNotifications(order);
}

private void ValidateOrder(Order order)
{
    if (order.Items.Count == 0)
        throw new ValidationException("Empty order");
    if (!order.Customer.IsActive)
        throw new ValidationException("Inactive customer");
}

private OrderTotals CalculateOrderTotals(Order order)
{
    var subtotal = order.Items.Sum(i => i.Price * i.Quantity);
    var tax = subtotal * 0.08m;
    var shipping = CalculateShipping(order);
    return new OrderTotals(subtotal, tax, shipping);
}

Large Class (God Object)

Smell:

// 2000-line class doing everything
class OrderManager {
    // Order creation
    createOrder(items: Item[]): Order { }
    
    // Payment processing
    processPayment(order: Order): void { }
    
    // Inventory management
    updateInventory(items: Item[]): void { }
    
    // Email notifications
    sendOrderConfirmation(order: Order): void { }
    
    // Reporting
    generateInvoice(order: Order): void { }
    generateReport(): Report { }
    
    // Shipping
    calculateShipping(order: Order): number { }
    scheduleShipment(order: Order): void { }
}

Refactored:

class OrderService {
    constructor(
        private orderRepository: OrderRepository,
        private paymentService: PaymentService,
        private inventoryService: InventoryService,
        private notificationService: NotificationService
    ) {}
    
    async createOrder(items: Item[]): Promise<Order> {
        const order = await this.orderRepository.create(items);
        await this.inventoryService.reserve(items);
        await this.notificationService.sendOrderConfirmation(order);
        return order;
    }
}

class PaymentService {
    processPayment(order: Order): Promise<PaymentResult> { }
}

class InventoryService {
    reserve(items: Item[]): Promise<void> { }
    release(items: Item[]): Promise<void> { }
}

class NotificationService {
    sendOrderConfirmation(order: Order): Promise<void> { }
}

Duplicate Code

Smell:

def calculate_employee_salary(employee):
    base = employee.base_salary
    bonus = base * 0.1
    tax = (base + bonus) * 0.2
    return base + bonus - tax

def calculate_contractor_payment(contractor):
    base = contractor.hourly_rate * contractor.hours
    bonus = base * 0.05
    tax = (base + bonus) * 0.3
    return base + bonus - tax

Refactored:

from dataclasses import dataclass

@dataclass
class CompensationRules:
    bonus_rate: float
    tax_rate: float

def calculate_compensation(base_amount: float, rules: CompensationRules) -> float:
    bonus = base_amount * rules.bonus_rate
    gross = base_amount + bonus
    tax = gross * rules.tax_rate
    return gross - tax

# Usage
employee_rules = CompensationRules(bonus_rate=0.1, tax_rate=0.2)
contractor_rules = CompensationRules(bonus_rate=0.05, tax_rate=0.3)

employee_salary = calculate_compensation(
    employee.base_salary,
    employee_rules
)

contractor_payment = calculate_compensation(
    contractor.hourly_rate * contractor.hours,
    contractor_rules
)

Feature Envy

Smell:

// OrderProcessor is too interested in Order internals
public class OrderProcessor
{
    public decimal CalculateTotal(Order order)
    {
        decimal total = 0;
        foreach (var item in order.Items)
        {
            total += item.Price * item.Quantity;
        }
        
        if (order.DiscountCode != null)
        {
            total *= (1 - order.DiscountRate);
        }
        
        return total;
    }
}

Refactored:

// Move logic to where data lives
public class Order
{
    public List<OrderItem> Items { get; set; }
    public string DiscountCode { get; set; }
    public decimal DiscountRate { get; set; }
    
    public decimal CalculateTotal()
    {
        var subtotal = Items.Sum(i => i.Price * i.Quantity);
        return ApplyDiscount(subtotal);
    }
    
    private decimal ApplyDiscount(decimal amount)
    {
        return DiscountCode != null
            ? amount * (1 - DiscountRate)
            : amount;
    }
}

public class OrderProcessor
{
    public void ProcessOrder(Order order)
    {
        var total = order.CalculateTotal();  // Delegate to Order
        // Process payment...
    }
}

Refactoring Techniques

Extract Method

// Before
function printOwing(invoice: Invoice): void {
    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): void {
    printBanner();
    const outstanding = calculateOutstanding(invoice);
    printDetails(invoice, outstanding);
}

function printBanner(): void {
    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(invoice: Invoice, outstanding: number): void {
    console.log(`Name: ${invoice.customer}`);
    console.log(`Amount: ${outstanding}`);
}

Replace Conditional with Polymorphism

# Before
class Bird:
    def __init__(self, bird_type):
        self.type = bird_type
    
    def fly(self):
        if self.type == "eagle":
            print("Soaring high")
        elif self.type == "penguin":
            print("Can't fly")
        elif self.type == "parrot":
            print("Flying in circles")

# After
from abc import ABC, abstractmethod

class Bird(ABC):
    @abstractmethod
    def fly(self):
        pass

class Eagle(Bird):
    def fly(self):
        print("Soaring high")

class Penguin(Bird):
    def fly(self):
        print("Can't fly")

class Parrot(Bird):
    def fly(self):
        print("Flying in circles")

Static Analysis Tools

ESLint (JavaScript/TypeScript)

// .eslintrc.json
{
  "extends": [
    "eslint:recommended",
    "plugin:@typescript-eslint/recommended"
  ],
  "rules": {
    "no-console": "warn",
    "no-unused-vars": "error",
    "complexity": ["warn", 10],
    "max-lines-per-function": ["warn", 50],
    "@typescript-eslint/no-explicit-any": "error",
    "@typescript-eslint/explicit-function-return-type": "warn"
  }
}

Pylint (Python)

# .pylintrc
[MASTER]
max-line-length=100

[MESSAGES CONTROL]
disable=
    missing-docstring,
    too-few-public-methods

[DESIGN]
max-args=5
max-locals=15
max-returns=6
max-branches=12
max-statements=50

[SIMILARITIES]
min-similarity-lines=4

Roslyn Analyzers (C#)

<!-- .editorconfig -->
[*.cs]
dotnet_diagnostic.CA1062.severity = warning  # Validate arguments
dotnet_diagnostic.CA1031.severity = error    # Don't catch generic Exception
dotnet_diagnostic.CA1303.severity = none     # String literals (localization)

# Code style
csharp_prefer_braces = true:warning
csharp_style_var_elsewhere = true:suggestion
csharp_style_expression_bodied_methods = true:suggestion

Best Practices

1. Keep methods small - Aim for 20 lines or less
2. Limit class responsibilities - Single Responsibility Principle
3. Use meaningful names - Avoid abbreviations and single letters
4. Write tests first - TDD helps design better APIs
5. Refactor continuously - Boy Scout Rule: leave code cleaner than you found it
6. Use static analysis - Catch issues before code review
7. Code reviews - Multiple eyes catch more issues
8. Measure complexity - Track cyclomatic complexity metrics

Key Takeaways

• SOLID principles guide maintainable object-oriented design
• Code smells indicate areas needing refactoring
• Small, focused methods and classes improve readability
• Static analysis tools enforce consistency and catch bugs
• Continuous refactoring prevents technical debt accumulation

Next Steps

• Learn Test-Driven Development for better design
• Explore Domain-Driven Design patterns
• Study Clean Architecture principles
• Practice code katas for refactoring skills

Additional Resources

• Refactoring: Improving the Design of Existing Code
• Clean Code by Robert C. Martin
• SOLID Principles
• Code Smells Catalog

---

Quality is not an act, it's a habit.