Refactoring Assistant Skill

Overview

Systematic refactoring skill for identifying code smells, applying design patterns, improving maintainability, and transforming legacy code into clean, testable architecture.

Capabilities

1. Code Smell Detection

• Long methods
• Large classes
• Duplicate code
• Dead code
• Complex conditionals
• Primitive obsession
• Feature envy
• Data clumps

2. Refactoring Patterns

• Extract Method/Function
• Extract Class
• Introduce Parameter Object
• Replace Conditional with Polymorphism
• Replace Magic Numbers with Constants
• Consolidate Duplicate Conditional Fragments

3. Design Pattern Application

• Strategy Pattern
• Factory Pattern
• Observer Pattern
• Decorator Pattern
• Dependency Injection
• Repository Pattern

4. Testing Support

• Add tests before refactoring
• Ensure tests pass after each step
• Improve test coverage during refactoring

Common Code Smells & Solutions

1. Long Method

Smell: Method with too many lines (>50)

Before:

def process_order(order_data):
    # Validate (20 lines)
    if not order_data.get('email'):
        return {'error': 'Email required'}
    if not order_data.get('items'):
        return {'error': 'Items required'}
    # ... more validation

    # Calculate totals (30 lines)
    subtotal = 0
    for item in order_data['items']:
        price = get_price(item['id'])
        quantity = item['quantity']
        subtotal += price * quantity
    # ... more calculation

    # Save to database (15 lines)
    conn = get_db_connection()
    cursor = conn.cursor()
    cursor.execute("INSERT INTO orders ...")
    # ... more database code

    # Send email (20 lines)
    smtp = smtplib.SMTP('smtp.gmail.com')
    # ... email code

    return {'success': True}

Refactoring: Extract Method

def process_order(order_data):
    validation_errors = validate_order(order_data)
    if validation_errors:
        return {'errors': validation_errors}, 400

    order_total = calculate_order_total(order_data)
    order_id = save_order(order_data, order_total)
    send_order_confirmation(order_data['email'], order_id)

    return {'order_id': order_id}, 201

def validate_order(order_data):
    errors = []
    if not order_data.get('email'):
        errors.append('Email required')
    if not order_data.get('items'):
        errors.append('Items required')
    return errors

def calculate_order_total(order_data):
    return sum(
        get_price(item['id']) * item['quantity']
        for item in order_data['items']
    )

def save_order(order_data, total):
    # Database logic
    pass

def send_order_confirmation(email, order_id):
    # Email logic
    pass

2. Large Class (God Object)

Smell: Class with too many responsibilities

Before:

class UserManager {
    public void createUser(String email, String password) { }
    public void deleteUser(int id) { }
    public void updateUser(int id, User data) { }
    public User findUser(int id) { }

    public void sendEmail(String to, String subject) { }
    public void sendSMS(String phone, String message) { }

    public void logActivity(String activity) { }
    public void logError(String error) { }

    public void generateReport() { }
    public void exportToCsv() { }

    // 50+ methods...
}

Refactoring: Extract Class

// Separate concerns
class UserRepository {
    public void create(User user) { }
    public void delete(int id) { }
    public void update(int id, User data) { }
    public User findById(int id) { }
}

class NotificationService {
    public void sendEmail(String to, String subject, String body) { }
    public void sendSMS(String phone, String message) { }
}

class Logger {
    public void logActivity(String activity) { }
    public void logError(String error) { }
}

class ReportGenerator {
    public void generateReport() { }
    public void exportToCsv() { }
}

class UserService {
    private UserRepository repository;
    private NotificationService notifications;
    private Logger logger;

    public void createUser(String email, String password) {
        User user = new User(email, password);
        repository.create(user);
        notifications.sendEmail(email, "Welcome!");
        logger.logActivity("User created: " + email);
    }
}

3. Duplicate Code

Smell: Same code in multiple places

Before:

function calculateTotalPrice(items) {
    let total = 0;
    for (let item of items) {
        total += item.price * item.quantity;
    }
    total = total * 1.1; // 10% tax
    return total;
}

function calculateCartPrice(cart) {
    let total = 0;
    for (let item of cart.items) {
        total += item.price * item.quantity;
    }
    total = total * 1.1; // 10% tax
    return total;
}

function calculateInvoiceTotal(invoice) {
    let total = 0;
    for (let line of invoice.lines) {
        total += line.price * line.quantity;
    }
    total = total * 1.1; // 10% tax
    return total;
}

Refactoring: Eliminate Duplication

function calculateTotal(items) {
    const subtotal = items.reduce(
        (sum, item) => sum + (item.price * item.quantity),
        0
    );
    return applyTax(subtotal);
}

function applyTax(amount) {
    const TAX_RATE = 0.1;
    return amount * (1 + TAX_RATE);
}

// Usage
const orderTotal = calculateTotal(order.items);
const cartTotal = calculateTotal(cart.items);
const invoiceTotal = calculateTotal(invoice.lines);

4. Complex Conditional

Smell: Nested if-else statements

Before:

def get_shipping_cost(order):
    if order.total > 100:
        if order.customer.premium:
            if order.destination == 'domestic':
                return 0
            else:
                return 10
        else:
            if order.destination == 'domestic':
                return 5
            else:
                return 20
    else:
        if order.customer.premium:
            if order.destination == 'domestic':
                return 3
            else:
                return 15
        else:
            if order.destination == 'domestic':
                return 8
            else:
                return 25

Refactoring: Strategy Pattern

class ShippingStrategy:
    def calculate_cost(self, order):
        raise NotImplementedError

class PremiumDomesticShipping(ShippingStrategy):
    def calculate_cost(self, order):
        return 0 if order.total > 100 else 3

class PremiumInternationalShipping(ShippingStrategy):
    def calculate_cost(self, order):
        return 10 if order.total > 100 else 15

class StandardDomesticShipping(ShippingStrategy):
    def calculate_cost(self, order):
        return 5 if order.total > 100 else 8

class StandardInternationalShipping(ShippingStrategy):
    def calculate_cost(self, order):
        return 20 if order.total > 100 else 25

class ShippingCalculator:
    def __init__(self):
        self.strategies = {
            ('premium', 'domestic'): PremiumDomesticShipping(),
            ('premium', 'international'): PremiumInternationalShipping(),
            ('standard', 'domestic'): StandardDomesticShipping(),
            ('standard', 'international'): StandardInternationalShipping(),
        }

    def calculate(self, order):
        customer_type = 'premium' if order.customer.premium else 'standard'
        key = (customer_type, order.destination)
        strategy = self.strategies[key]
        return strategy.calculate_cost(order)

5. Primitive Obsession

Smell: Using primitives instead of small objects

Before:

class Order {
    private String customerEmail;  // Primitives everywhere
    private String customerPhone;
    private String shippingAddress;
    private String billingAddress;

    public boolean isValidEmail() {
        return customerEmail.matches("^[A-Za-z0-9+_.-]+@(.+)$");
    }

    public boolean isValidPhone() {
        return customerPhone.matches("\\d{10}");
    }
}

Refactoring: Introduce Value Objects

class Email {
    private final String value;

    public Email(String value) {
        if (!isValid(value)) {
            throw new IllegalArgumentException("Invalid email");
        }
        this.value = value;
    }

    private boolean isValid(String email) {
        return email.matches("^[A-Za-z0-9+_.-]+@(.+)$");
    }

    public String getValue() { return value; }
}

class PhoneNumber {
    private final String value;

    public PhoneNumber(String value) {
        if (!isValid(value)) {
            throw new IllegalArgumentException("Invalid phone");
        }
        this.value = value;
    }

    private boolean isValid(String phone) {
        return phone.matches("\\d{10}");
    }
}

class Address {
    private final String street;
    private final String city;
    private final String zipCode;

    // Constructor with validation
}

class Order {
    private Email customerEmail;
    private PhoneNumber customerPhone;
    private Address shippingAddress;
    private Address billingAddress;

    // No validation needed - value objects ensure validity
}

6. Feature Envy

Smell: Method in one class uses data from another class more than its own

Before:

class Order:
    def __init__(self, customer):
        self.customer = customer

    def calculate_discount(self):
        # Feature envy - uses customer data extensively
        if self.customer.membership_years > 5:
            if self.customer.total_purchases > 10000:
                return 0.20
            elif self.customer.total_purchases > 5000:
                return 0.15
            else:
                return 0.10
        elif self.customer.membership_years > 2:
            return 0.05
        else:
            return 0

Refactoring: Move Method

class Customer:
    def __init__(self, membership_years, total_purchases):
        self.membership_years = membership_years
        self.total_purchases = total_purchases

    def get_discount_rate(self):
        if self.membership_years > 5:
            if self.total_purchases > 10000:
                return 0.20
            elif self.total_purchases > 5000:
                return 0.15
            else:
                return 0.10
        elif self.membership_years > 2:
            return 0.05
        else:
            return 0

class Order:
    def __init__(self, customer):
        self.customer = customer

    def calculate_discount(self):
        return self.customer.get_discount_rate()

Refactoring Workflow

Safe Refactoring Process

1. Write Tests First

# Ensure existing tests pass
pytest tests/

# Add tests if coverage is low
pytest --cov=src tests/

2. Make Small Changes

# Don't refactor everything at once
# Refactor one method/class at a time

3. Run Tests After Each Change

# After each refactoring step
pytest tests/

4. Commit Frequently

git add .
git commit -m "refactor: extract calculate_total method"

5. Review Changes

git diff HEAD~1

Integration Scripts

refactor_detector.py

Detect code smells:

#!/usr/bin/env python3
import ast
import os

class RefactoringDetector(ast.NodeVisitor):
    def __init__(self):
        self.long_methods = []
        self.complex_methods = []
        self.large_classes = []

    def visit_FunctionDef(self, node):
        # Long method detection
        lines = node.end_lineno - node.lineno
        if lines > 50:
            self.long_methods.append({
                'name': node.name,
                'lines': lines,
                'line_start': node.lineno
            })

        # Complexity detection (rough estimate)
        complexity = self.calculate_complexity(node)
        if complexity > 10:
            self.complex_methods.append({
                'name': node.name,
                'complexity': complexity,
                'line': node.lineno
            })

        self.generic_visit(node)

    def visit_ClassDef(self, node):
        # Large class detection
        methods = [n for n in node.body if isinstance(n, ast.FunctionDef)]
        if len(methods) > 20:
            self.large_classes.append({
                'name': node.name,
                'methods': len(methods),
                'line': node.lineno
            })

        self.generic_visit(node)

    def calculate_complexity(self, node):
        complexity = 1
        for child in ast.walk(node):
            if isinstance(child, (ast.If, ast.While, ast.For, ast.ExceptHandler)):
                complexity += 1
        return complexity

def analyze_file(filepath):
    with open(filepath) as f:
        tree = ast.parse(f.read(), filepath)

    detector = RefactoringDetector()
    detector.visit(tree)

    return detector

def scan_directory(directory):
    print("=== Refactoring Opportunities ===\n")

    all_long_methods = []
    all_complex_methods = []
    all_large_classes = []

    for root, dirs, files in os.walk(directory):
        for file in files:
            if file.endswith('.py'):
                filepath = os.path.join(root, file)
                try:
                    detector = analyze_file(filepath)
                    if detector.long_methods:
                        for method in detector.long_methods:
                            method['file'] = filepath
                            all_long_methods.append(method)
                    if detector.complex_methods:
                        for method in detector.complex_methods:
                            method['file'] = filepath
                            all_complex_methods.append(method)
                    if detector.large_classes:
                        for cls in detector.large_classes:
                            cls['file'] = filepath
                            all_large_classes.append(cls)
                except:
                    pass

    # Report findings
    if all_long_methods:
        print(f"⚠️  Long Methods ({len(all_long_methods)}):")
        for method in all_long_methods[:10]:
            print(f"  {method['file']}:{method['line_start']}")
            print(f"    {method['name']}: {method['lines']} lines")
            print(f"    💡 Consider extracting into smaller methods\n")

    if all_complex_methods:
        print(f"\n⚠️  Complex Methods ({len(all_complex_methods)}):")
        for method in all_complex_methods[:10]:
            print(f"  {method['file']}:{method['line']}")
            print(f"    {method['name']}: complexity = {method['complexity']}")
            print(f"    💡 Consider simplifying or extracting logic\n")

    if all_large_classes:
        print(f"\n⚠️  Large Classes ({len(all_large_classes)}):")
        for cls in all_large_classes[:10]:
            print(f"  {cls['file']}:{cls['line']}")
            print(f"    {cls['name']}: {cls['methods']} methods")
            print(f"    💡 Consider splitting into smaller classes\n")

if __name__ == '__main__':
    import sys
    directory = sys.argv[1] if len(sys.argv) > 1 else '.'
    scan_directory(directory)

duplicate_detector.sh

Find duplicate code:

#!/bin/bash
# Detect duplicate code blocks

PROJECT_DIR=${1:-.}

echo "=== Duplicate Code Detection ==="

# Python
if command -v pylint &> /dev/null; then
    echo "Checking Python files..."
    pylint --disable=all --enable=duplicate-code $PROJECT_DIR
fi

# JavaScript
if command -v jscpd &> /dev/null; then
    echo "Checking JavaScript files..."
    jscpd $PROJECT_DIR --min-lines 5 --min-tokens 50
fi

# Java
if command -v cpd &> /dev/null; then
    echo "Checking Java files..."
    cpd --minimum-tokens 50 --files $PROJECT_DIR --language java
fi

Best Practices

1. Test First: Always have tests before refactoring
2. Small Steps: One refactoring at a time
3. Continuous Testing: Run tests after each change
4. Commit Often: Version control each successful step
5. Don't Change Behavior: Refactoring shouldn't add features
6. Use IDE Tools: Automated refactoring when possible
7. Code Reviews: Get feedback on refactored code
8. Measure Impact: Track complexity, duplication metrics

Common Refactoring Patterns

Requirements

# Python
pip install pylint radon

# JavaScript
npm install -g jscpd

# Java
# PMD (includes CPD)
wget https://github.com/pmd/pmd/releases/download/pmd_releases%2F6.55.0/pmd-bin-6.55.0.zip

Metrics to Track

• Code complexity: Trending down
• Method length: < 50 lines
• Class size: < 20 methods
• Code duplication: < 5%
• Test coverage: Maintained or improved