Refactor Legacy Code

Refactor Legacy Code

Table of Contents

• Overview
• When to Use
• Quick Start
• Reference Guides
• Best Practices

Overview

This skill helps you systematically refactor legacy code to improve maintainability, readability, and performance while preserving existing functionality. It follows industry best practices for safe refactoring with comprehensive testing.

When to Use

• Modernizing outdated code patterns or deprecated APIs
• Reducing technical debt in existing codebases
• Improving code readability and maintainability
• Extracting reusable components from monolithic code
• Upgrading to newer language features or frameworks
• Preparing code for new feature development

Quick Start

First, analyze the legacy code to understand:

# Review the codebase structure
tree -L 3 -I 'node_modules|dist|build'

# Check for outdated dependencies
npm outdated  # or pip list --outdated, composer outdated, etc.

# Identify code complexity hotspots
# Use tools like:
# - SonarQube for code smells
# - eslint for JavaScript
# - pylint for Python
# - RuboCop for Ruby

Reference Guides

Detailed implementations in the references/ directory:

Best Practices

✅ DO

• Refactor incrementally: Small, testable changes
• Run tests frequently: After each refactoring step
• Commit often: Create logical, atomic commits
• Keep existing tests passing: Don't break functionality
• Use IDE refactoring tools: Safer than manual edits
• Review code coverage: Ensure tests cover refactored code
• Document decisions: Why, not just what
• Seek peer review: Fresh eyes catch issues

❌ DON'T

• Mix refactoring with new features: Separate concerns
• Refactor without tests: Recipe for breaking changes
• Change behavior: Refactoring should preserve functionality
• Refactor large chunks: Increases risk and review difficulty
• Ignore code smells: Address them systematically
• Skip documentation: Future maintainers need context

Benefits Achieved

Benefits Achieved

• ✅ Testability: Dependencies injected, easy to mock
• ✅ Readability: Clear, focused methods
• ✅ Maintainability: Single responsibility principle
• ✅ Type Safety: TypeScript interfaces prevent bugs
• ✅ Reusability: Components can be used independently
• ✅ Error Handling: Proper exception handling
• ✅ Modern Patterns: Async/await, dependency injection

Code Assessment

Code Assessment

First, analyze the legacy code to understand:

# Review the codebase structure
tree -L 3 -I 'node_modules|dist|build'

# Check for outdated dependencies
npm outdated  # or pip list --outdated, composer outdated, etc.

# Identify code complexity hotspots
# Use tools like:
# - SonarQube for code smells
# - eslint for JavaScript
# - pylint for Python
# - RuboCop for Ruby

Assessment Checklist:

• [ ] Identify deprecated patterns and APIs
• [ ] Locate tightly coupled components
• [ ] Find duplicated code blocks
• [ ] Review test coverage gaps
• [ ] Document current behavior and edge cases
• [ ] Identify performance bottlenecks

Complete Refactoring Example

Complete Refactoring Example

Before

// legacy-user-service.js - 200 lines of complex, coupled code
var UserService = {
  createUser: function (fn, ln, em, ph, addr) {
    if (!em || em.indexOf("@") === -1) {
      return { error: "Invalid email" };
    }
    var conn = mysql.createConnection(config);
    conn.connect();
    conn.query(
      "INSERT INTO users (first_name, last_name, email, phone, address) VALUES (?, ?, ?, ?, ?)",
      [fn, ln, em.toLowerCase(), ph, addr],
      function (err, result) {
        if (err) {
          console.log(err);
          return { error: "Database error" };
        }
        // Send welcome email
        var nodemailer = require("nodemailer");
        var transporter = nodemailer.createTransport(emailConfig);
        transporter.sendMail(
          {
            to: em,
            subject: "Welcome!",
            html: "<h1>Welcome " + fn + "!</h1>",
          },
          function (err, info) {
            if (err) console.log(err);
          },
        );
        conn.end();
        return { id: result.insertId };
      },
    );
  },
};

After

// user-service.ts - Clean, testable, maintainable
interface UserData {
  firstName: string;
  lastName: string;
  email: string;
  phone: string;
  address: string;
}

class UserService {
  constructor(
    private database: Database,
    private emailService: EmailService,
    private validator: Validator,
  ) {}

  async createUser(userData: UserData): Promise<User> {
    this.validator.validateEmail(userData.email);

    const normalizedData = this.normalizeUserData(userData);
    const user = await this.database.users.create(normalizedData);

    await this.sendWelcomeEmail(user);

    return user;
  }

  private normalizeUserData(data: UserData): UserData {
    return {
      ...data,
      email: data.email.toLowerCase().trim(),
    };
  }

  private async sendWelcomeEmail(user: User): Promise<void> {
    await this.emailService.send({
      to: user.email,
      subject: "Welcome!",
      template: "welcome",
      data: { firstName: user.firstName },
    });
  }
}

// validator.ts
class Validator {
  validateEmail(email: string): void {
    if (!email || !email.includes("@")) {
      throw new ValidationError("Invalid email format");
    }
  }
}

// Easy to test
describe("UserService", () => {
  it("should create user with valid data", async () => {
    const mockDb = createMockDatabase();
    const mockEmail = createMockEmailService();
    const service = new UserService(mockDb, mockEmail, new Validator());

    const user = await service.createUser({
      firstName: "John",
      lastName: "Doe",
      email: "john@example.com",
      phone: "555-0123",
      address: "123 Main St",
    });

    expect(user.id).toBeDefined();
    expect(mockDb.users.create).toHaveBeenCalled();
    expect(mockEmail.send).toHaveBeenCalledWith(
      expect.objectContaining({ to: "john@example.com" }),
    );
  });
});

Establish Safety Net

Establish Safety Net

Before refactoring, ensure you have comprehensive tests:

// Add characterization tests to lock in current behavior
describe("LegacyFeature", () => {
  it("should preserve existing behavior during refactoring", () => {
    // Test current implementation behavior
    const input = {
      /* realistic test data */
    };
    const result = legacyFunction(input);

    // Document expected output
    expect(result).toEqual({
      /* current actual output */
    });
  });
});

Testing Strategy:

• Add unit tests for critical paths
• Create integration tests for component interactions
• Document edge cases and error scenarios
• Set up test coverage monitoring
• Run tests before each refactoring step

Incremental Refactoring

Incremental Refactoring

Apply refactoring patterns systematically:

Extract Function/Method

// BEFORE: Long, complex function
function processUserData(user) {
  // 50 lines of mixed validation, transformation, and business logic
  if (!user.email || !user.email.includes("@")) return null;
  const normalized = user.email.toLowerCase().trim();
  // ... more complex logic
}

// AFTER: Extracted, focused functions
function validateEmail(email) {
  return email && email.includes("@");
}

function normalizeEmail(email) {
  return email.toLowerCase().trim();
}

function processUserData(user) {
  if (!validateEmail(user.email)) return null;
  const email = normalizeEmail(user.email);
  // Clear, readable flow
}

Replace Conditionals with Polymorphism

# BEFORE: Complex conditional logic
def calculate_price(customer_type, base_price):
    if customer_type == 'regular':
        return base_price
    elif customer_type == 'premium':
        return base_price * 0.9
    elif customer_type == 'vip':
        return base_price * 0.8
    else:
        return base_price

# AFTER: Polymorphic approach
class PricingStrategy:
    def calculate(self, base_price):
        return base_price

class RegularPricing(PricingStrategy):
    pass

class PremiumPricing(PricingStrategy):
    def calculate(self, base_price):
        return base_price * 0.9

class VIPPricing(PricingStrategy):
    def calculate(self, base_price):
        return base_price * 0.8

# Usage
pricing = pricing_strategies[customer_type]
price = pricing.calculate(base_price)

Introduce Parameter Object

// BEFORE: Long parameter lists
function createUser(
  firstName: string,
  lastName: string,
  email: string,
  phone: string,
  address: string,
  city: string,
  state: string,
  zip: string,
) {
  // ...
}

// AFTER: Parameter object
interface UserData {
  firstName: string;
  lastName: string;
  email: string;
  phone: string;
  address: Address;
}

interface Address {
  street: string;
  city: string;
  state: string;
  zip: string;
}

function createUser(userData: UserData) {
  // ...
}

Modernize Patterns

Modernize Patterns

Replace outdated patterns with modern equivalents:

Promises over Callbacks

// BEFORE: Callback hell
function fetchUserData(userId, callback) {
  db.query("SELECT * FROM users WHERE id = ?", [userId], (err, user) => {
    if (err) return callback(err);
    db.query(
      "SELECT * FROM orders WHERE user_id = ?",
      [userId],
      (err, orders) => {
        if (err) return callback(err);
        callback(null, { user, orders });
      },
    );
  });
}

// AFTER: Async/await
async function fetchUserData(userId) {
  const user = await db.query("SELECT * FROM users WHERE id = ?", [userId]);
  const orders = await db.query("SELECT * FROM orders WHERE user_id = ?", [
    userId,
  ]);
  return { user, orders };
}

Modern Language Features

// BEFORE: var and string concatenation
var userName = user.firstName + " " + user.lastName;
var isActive = user.status === "active" ? true : false;

// AFTER: const/let and template literals
const userName = `${user.firstName} ${user.lastName}`;
const isActive = user.status === "active";

Reduce Dependencies

Reduce Dependencies

Break tight coupling:

# BEFORE: Tight coupling to specific implementation
class OrderProcessor:
    def __init__(self):
        self.db = MySQLDatabase()  # Tightly coupled
        self.email = SendGridEmail()  # Tightly coupled

    def process_order(self, order):
        self.db.save(order)
        self.email.send(order.customer_email, "Order confirmed")

# AFTER: Dependency injection
class OrderProcessor:
    def __init__(self, database, email_service):
        self.db = database  # Any database implementation
        self.email = email_service  # Any email service

    def process_order(self, order):
        self.db.save(order)
        self.email.send(order.customer_email, "Order confirmed")

# Easy to test with mocks
processor = OrderProcessor(MockDatabase(), MockEmailService())

Documentation

Document refactoring decisions: