Refactor Without Breaking It — Turn "It Looks Cleaner" into "It Still Behaves Exactly the Same"

A complete refactoring plan should answer:

1. What problem is the refactoring solving?
2. Which behavior must remain unchanged?
3. Which behavior is intentionally changing?
4. What public interfaces are affected?
5. What hidden side effects exist?
6. What dependencies are involved?
7. What tests currently protect the code?
8. Which behaviors are undocumented?
9. What is the smallest safe sequence of changes?
10. How will each step be validated?
11. What risks are introduced?
12. What should not be changed?
13. How will performance be compared?
14. How will compatibility be verified?
15. How will the change be rolled back?
16. What future changes become easier afterward?

Refactoring Objective

Simplify an order-processing method that validates an order, charges a payment card, saves the order, sends a confirmation email, and writes an audit event.

Existing Code

public async Task<OrderResult> ProcessOrder(
    OrderRequest request)
{
    if (request == null)
    {
        throw new ArgumentNullException(
            nameof(request)
        );
    }
    if (request.CustomerId == null)
    {
        return new OrderResult
        {
            Success = false,
            Error = "Customer is required"
        };
    }
    if (request.Items == null ||
        request.Items.Count == 0)
    {
        return new OrderResult
        {
            Success = false,
            Error = "At least one item is required"
        };
    }
    decimal subtotal = 0;
    foreach (var item in request.Items)
    {
        subtotal += item.Price * item.Quantity;
    }
    var payment = await _paymentClient.ChargeAsync(
        request.PaymentToken,
        subtotal
    );
    if (!payment.Success)
    {
        _logger.LogWarning(
            "Payment failed for customer {CustomerId}",
            request.CustomerId
        );
        return new OrderResult
        {
            Success = false,
            Error = "Payment failed"
        };
    }
    var order = new Order
    {
        CustomerId = request.CustomerId,
        Items = request.Items,
        Subtotal = subtotal,
        PaymentId = payment.PaymentId,
        CreatedAt = DateTime.UtcNow
    };
    await _repository.SaveAsync(order);
    await _emailSender.SendOrderConfirmationAsync(
        request.CustomerId,
        order.Id
    );
    await _auditWriter.WriteAsync(
        "OrderCreated",
        request.CustomerId,
        order.Id
    );
    return new OrderResult
    {
        Success = true,
        OrderId = order.Id
    };
}

The method currently performs five responsibilities:

1. Input validation
2. Total calculation
3. Payment processing
4. Persistence
5. Post-order notifications and auditing

The method also contains several important behavioral contracts:

• Null request throws ArgumentNullException.
• Missing customer returns an unsuccessful result.
• Empty items return an unsuccessful result.
• Payment failure returns an unsuccessful result and writes a warning.
• Order persistence occurs before email and audit.
• Email failure occurs after payment and persistence.
• Audit failure occurs after email.
• The order timestamp uses the system UTC clock.
• No rollback occurs when email or audit fails.
• The caller may receive an exception after an order has already been persisted.

The last behavior is operationally significant and should not be changed accidentally during refactoring.

Step RF-001 — Add Characterization Tests

Capture:

• Null request exception
• Validation results
• Total calculation
• Payment failure behavior
• Save-before-email ordering
• Save-before-audit ordering
• Behavior when email fails
• Behavior when audit fails
• UTC timestamp behavior

Step RF-002 — Extract Validation

Move request validation into a private or domain-level function without changing result types or messages.

Step RF-003 — Extract Total Calculation

Move subtotal calculation into a pure function.

Step RF-004 — Inject Clock

Replace direct DateTime.UtcNow use with an explicit clock while preserving UTC values.

Step RF-005 — Extract Order Construction

Move order mapping into a dedicated function.

Step RF-006 — Extract Post-Persistence Actions

Group email and audit operations only if their ordering and failure behavior remain explicit.

Step RF-007 — Review Transaction and Recovery Design Separately

Do not alter persistence, email, or audit transactional behavior as part of the structural refactoring.

public sealed class OrderService
{
    private readonly IPaymentClient _paymentClient;
    private readonly IOrderRepository _repository;
    private readonly IEmailSender _emailSender;
    private readonly IAuditWriter _auditWriter;
    private readonly IClock _clock;
    private readonly ILogger<OrderService> _logger;

    public OrderService(
        IPaymentClient paymentClient,
        IOrderRepository repository,
        IEmailSender emailSender,
        IAuditWriter auditWriter,
        IClock clock,
        ILogger<OrderService> logger)
    {
        _paymentClient = paymentClient;
        _repository = repository;
        _emailSender = emailSender;
        _auditWriter = auditWriter;
        _clock = clock;
        _logger = logger;
    }

    public async Task<OrderResult> ProcessOrder(
        OrderRequest request)
    {
        ArgumentNullException.ThrowIfNull(request);
        var validationResult = Validate(request);
        if (validationResult is not null)
        {
            return validationResult;
        }
        var subtotal = CalculateSubtotal(
            request.Items!
        );
        var payment = await _paymentClient.ChargeAsync(
            request.PaymentToken,
            subtotal
        );
        if (!payment.Success)
        {
            LogPaymentFailure(request.CustomerId!);
            return Failure("Payment failed");
        }
        var order = CreateOrder(
            request,
            subtotal,
            payment.PaymentId
        );
        await _repository.SaveAsync(order);
        await SendPostOrderActions(
            request.CustomerId!,
            order.Id
        );
        return new OrderResult
        {
            Success = true,
            OrderId = order.Id
        };
    }

    private static OrderResult? Validate(
        OrderRequest request)
    {
        if (request.CustomerId is null)
        {
            return Failure(
                "Customer is required"
            );
        }
        if (request.Items is null ||
            request.Items.Count == 0)
        {
            return Failure(
                "At least one item is required"
            );
        }
        return null;
    }

    private static decimal CalculateSubtotal(
        IEnumerable<OrderItem> items)
    {
        return items.Sum(
            item => item.Price * item.Quantity
        );
    }

    private Order CreateOrder(
        OrderRequest request,
        decimal subtotal,
        string paymentId)
    {
        return new Order
        {
            CustomerId = request.CustomerId!,
            Items = request.Items!,
            Subtotal = subtotal,
            PaymentId = paymentId,
            CreatedAt = _clock.UtcNow
        };
    }

    private async Task SendPostOrderActions(
        string customerId,
        string orderId)
    {
        await _emailSender
            .SendOrderConfirmationAsync(
                customerId,
                orderId
            );
        await _auditWriter.WriteAsync(
            "OrderCreated",
            customerId,
            orderId
        );
    }

    private void LogPaymentFailure(
        string customerId)
    {
        _logger.LogWarning(
            "Payment failed for customer {CustomerId}",
            customerId
        );
    }

    private static OrderResult Failure(
        string error)
    {
        return new OrderResult
        {
            Success = false,
            Error = error
        };
    }
}

This refactoring improves readability and testability while preserving the original sequencing.

It does not solve the risk of successful payment and persistence followed by email or audit failure. That issue should be handled as a separate behavioral and architectural change.

[Fact]
public async Task ThrowsWhenRequestIsNull()
{
    var service = CreateService();
    await Assert.ThrowsAsync<
        ArgumentNullException
    >(
        () => service.ProcessOrder(null!)
    );
}

[Fact]
public async Task ReturnsFailureWhenCustomerIsMissing()
{
    var service = CreateService();
    var request = new OrderRequest
    {
        CustomerId = null,
        Items = ValidItems()
    };
    var result = await service.ProcessOrder(
        request
    );
    result.Success.Should().BeFalse();
    result.Error.Should().Be(
        "Customer is required"
    );
}

[Fact]
public async Task SavesOrderBeforeSendingEmail()
{
    var sequence = new List<string>();
    var repository = Substitute.For<
        IOrderRepository
    >();
    var emailSender = Substitute.For<
        IEmailSender
    >();
    repository
        .When(x => x.SaveAsync(
            Arg.Any<Order>()
        ))
        .Do(_ => sequence.Add("save"));
    emailSender
        .When(x => x.SendOrderConfirmationAsync(
            Arg.Any<string>(),
            Arg.Any<string>()
        ))
        .Do(_ => sequence.Add("email"));
    var service = CreateService(
        repository: repository,
        emailSender: emailSender
    );
    await service.ProcessOrder(
        ValidRequest()
    );
    sequence.Should().ContainInOrder(
        "save",
        "email"
    );
}

[Fact]
public async Task UsesInjectedUtcTime()
{
    var fixedTime = new DateTimeOffset(
        2026,
        7,
        21,
        14,
        30,
        0,
        TimeSpan.Zero
    );
    var clock = Substitute.For<IClock>();
    clock.UtcNow.Returns(fixedTime);
    Order? savedOrder = null;
    var repository = Substitute.For<
        IOrderRepository
    >();
    repository
        .When(x => x.SaveAsync(
            Arg.Any<Order>()
        ))
        .Do(call =>
        {
            savedOrder = call.Arg<Order>();
        });
    var service = CreateService(
        clock: clock,
        repository: repository
    );
    await service.ProcessOrder(
        ValidRequest()
    );
    savedOrder!.CreatedAt.Should().Be(
        fixedTime
    );
}

The exact framework and mocking syntax should match the project.

A safe sequence often looks like:

1. Define the objective.
2. Document public behavior.
3. Add characterization tests.
4. Establish a performance baseline.
5. Isolate one responsibility.
6. Run tests.
7. Review the diff.
8. Commit the step.
9. Repeat for the next responsibility.
10. Remove verified duplication.
11. Update documentation.
12. Run full validation.
13. Compare behavior and performance.
14. Deploy incrementally.
15. Monitor.

Avoid combining every structural improvement into one change.

A large refactoring may be divided into:

PR 1 — Tests and Baseline

• Characterization tests
• Performance baseline
• No behavior changes

PR 2 — Naming and Local Extraction

• Internal names
• Small pure functions
• No public changes

PR 3 — Dependency Isolation

• Introduce interfaces or adapters
• Preserve existing construction through compatibility paths

PR 4 — Responsibility Separation

• Move logic into cohesive components
• Preserve public facade

PR 5 — Verified Dead-Code Removal

• Remove code only after usage validation

PR 6 — Intentional Behavior Improvements

• Treat as a separate feature or bug-fix change
• Update contracts and tests explicitly

Even behavior-preserving refactoring may require careful deployment when it changes:

• Dependency wiring
• Configuration
• Package versions
• Service registration
• Database access patterns
• Resource usage
• Startup behavior
• Logging
• Deployment artifacts

Use:

• Feature flags where appropriate
• Canary deployments
• Side-by-side comparison
• Error-rate monitoring
• Performance monitoring
• Rollback thresholds
• Version compatibility checks

A rollback plan should identify:

• Previous version
• Reversible configuration
• Database compatibility
• Package compatibility
• Event compatibility
• Deployment order
• Data written by the new version
• Monitoring triggers
• Responsible owner

A code rollback may be insufficient if the refactoring changed persistent data or external contracts.

Teams may track:

• Change failure rate
• Defect rate after refactoring
• Complexity reduction
• Duplication reduction
• Test coverage by behavior
• Build time
• Test execution time
• Review time
• Deployment frequency
• Lead time for future changes
• Onboarding time
• Number of public-contract changes
• Rollbacks
• Flaky tests
• Technical debt closed

Metrics should reflect engineering outcomes, not reward code churn.

Define:

• Required characterization testing
• Public-contract review
• Security review thresholds
• Maximum pull-request scope
• Required performance validation
• Required rollback planning
• Dead-code removal standards
• Approved AI platforms
• Source-code handling rules
• Human review requirements
• Refactoring ownership
• Architecture review triggers
• Documentation requirements
• Compatibility policy
• Test expectations
• Post-deployment monitoring