Refactoring Strategies for Magento 2

The Rule

"Always leave the code cleaner than you found it."

Applying It in Magento 2

When Touching Code

// BEFORE: Messy code you encounter while fixing a bug
public function getOrderTotal($orderId)
{
    $order = $this->_orderFactory->create();
    $order->load($orderId);
    $total = $order->getGrandTotal();
    $tax = $order->getTaxAmount();
    $shipping = $order->getShippingAmount();
    $discount = $order->getDiscountAmount();
    return $total + $tax + $shipping - $discount;
}

// AFTER: Clean up while you're there
public function getOrderTotal(int $orderId): float
{
    $order = $this->orderRepository->get($orderId);
    
    return (float) (
        $order->getGrandTotal()
        + $order->getTaxAmount()
        + $order->getShippingAmount()
        - abs($order->getDiscountAmount())
    );
}

Small, Safe Changes

Boy Scout Activities (5-15 minutes each):
- Extract magic numbers to constants
- Add type hints to method signatures
- Remove unused imports
- Rename unclear variable names
- Add return type declarations
- Replace deprecated method calls
- Fix one PHPStan warning

Measuring Progress

# Before cleanup
vendor/bin/phpstan analyse --level=6 src/ | tail -5
# Found 47 errors

# After boy scout session
vendor/bin/phpstan analyse --level=6 src/ | tail -5
# Found 42 errors

# Track over time
rg -c 'TODO|FIXME' src/ | wc -l
# 23 → 19 after cleanup

Team Practice

1. Each PR includes one small cleanup unrelated to the feature
2. No dedicated cleanup sprint needed
3. Code quality improves gradually
4. No risk of breaking changes
5. Everyone participates equally

Concept

Named after the strangler fig tree that grows around a host tree, eventually replacing it. Incrementally replace legacy code by building new alongside old.

Magento 2 Application

Phase 1: Identify seams

// Legacy monolithic class
class OrderService
{
    public function processOrder($data) { /* 500 lines */ }
    public function validateOrder($data) { /* 200 lines */ }
    public function calculateTotals($data) { /* 300 lines */ }
    public function sendNotifications($data) { /* 150 lines */ }
}

Phase 2: Extract to new service

// New clean service
interface OrderValidatorInterface
{
    public function validate(array $data): ValidationResult;
}

class OrderValidator implements OrderValidatorInterface
{
    public function validate(array $data): ValidationResult
    {
        // Clean, testable validation logic
    }
}

// Update old code to delegate
class OrderService
{
    public function __construct(
        private OrderValidatorInterface $validator
    ) {}
    
    public function processOrder($data)
    {
        $result = $this->validator->validate($data);
        // ... rest of processing
    }
}

Phase 3: Route traffic

<!-- di.xml configuration -->
<config>
    <type name="Vendor\NewModule\Service\OrderProcessorInterface">
        <plugin name="route-to-new" type="Vendor\NewModule\Plugin\OrderRoutePlugin"/>
    </type>
</config>

Phase 4: Remove old code

// Once all traffic routes to new service
class OrderService
{
    /**
     * @deprecated Use OrderValidatorInterface instead
     */
    public function validateOrder($data)
    {
        @trigger_error('Use OrderValidatorInterface', E_USER_DEPRECATED);
        return $this->validator->validate($data);
    }
}

Benefits

• No big-bang rewrite risk
• Continuous delivery of value
• Easy rollback at each step
• Team learns incrementally

Strategy: Small Steps, Big Impact

Refactoring Catalog for Magento

Extract Service Class

// BEFORE: Logic mixed into model
public class Product extends AbstractModel
{
    public function calculateDiscount()
    {
        // 50 lines of discount logic
    }
    
    public function applyPromotion()
    {
        // 30 lines of promotion logic
    }
}

// AFTER: Separate service
class ProductDiscountService
{
    public function calculateDiscount(Product $product): float
    {
        // Clean, testable logic
    }
}

Replace Helper with Service

// BEFORE: Helper doing too much
class Data extends AbstractHelper
{
    public function formatPrice($price) { /* ... */ }
    public function validateEmail($email) { /* ... */ }
    public function generateToken() { /* ... */ }
}

// AFTER: Focused services
interface PriceFormatterInterface { /* ... */ }
interface EmailValidatorInterface { /* ... */ }
interface TokenGeneratorInterface { /* ... */ }

Introduce Repository Pattern

// BEFORE: Direct resource model usage
$order = $this->orderFactory->create();
$order->load($id);

// AFTER: Repository abstraction
$order = $this->orderRepository->get($id);

Refactoring Checklist

Before:
- [ ] Write characterization test for current behavior
- [ ] Document current API contract
- [ ] Create backup/branch
- [ ] Identify all callers of changed code

During:
- [ ] Make one small change at a time
- [ ] Run tests after each change
- [ ] Keep backward compatibility
- [ ] Update all callers

After:
- [ ] All tests passing
- [ ] PHPStan level maintained or improved
- [ ] Documentation updated
- [ ] Deprecated old API if replacing

Magento Backward Compatibility Rules

What Breaks Compatibility

- Removing public methods
- Changing method signatures
- Renaming classes/interfaces
- Changing database schema
- Removing configuration fields
- Changing API response format
- Modifying event names/arguments

Deprecation Strategy

// Step 1: Mark as deprecated
class LegacyService
{
    /**
     * @deprecated Use NewServiceInterface::process() instead
     * @see NewServiceInterface
     */
    public function process($data)
    {
        @trigger_error(
            sprintf('Method %s is deprecated, use %s', __METHOD__, NewServiceInterface::class . '::process'),
            E_USER_DEPRECATED
        );
        return $this->newService->process($data);
    }
}

// Step 2: Keep for 2 minor versions
// Step 3: Remove in next major version

Plugin-Based Refactoring

<!-- Safely intercept without modifying core -->
<config>
    <type name="Magento\Catalog\Model\Product">
        <plugin name="improved-validation"
                type="Vendor\Module\Plugin\ProductValidationPlugin"
                sortOrder="10"/>
    </type>
</config>

Service Contract Approach

// Define interface (stable API)
interface ProductRepositoryInterface
{
    public function get($id);
    public function save(ProductInterface $product);
}

// Implementation can change freely
class ProductRepository implements ProductRepositoryInterface
{
    // Internal implementation details hidden
}

Migration Testing

// Test backward compatibility
public function testLegacyMethodStillWorks(): void
{
    $legacy = new LegacyService($this->newService);
    $result = $legacy->process($data);
    $this->assertEquals($expected, $result);
}

// Test new method works
public function testNewMethodWorks(): void
{
    $service = new NewService();
    $result = $service->process($data);
    $this->assertEquals($expected, $result);
}