Small, Safe Steps: IDE Refactorings, Parallel Change and Bulk Automated Fixes

Version note: The code in this post was built and run with C# 14 and .NET 10 (SDK 10.0.402), with tests on xUnit.net v3 4.0.3 and Verify.XunitV3 32.0.0.

Guess First: How Many Commits Does It Take to Open This Method?

Bookstore's PricingEngine.Calculate computes the cart total, the discount by customer type and campaign, the coupon, the 30 percent discount cap and the shipping fee for small orders in a single body. It is called from three places: checkout, the cart preview and the quote email.

// customerType: 0 = regular, 1 = member, 2 = vip
public decimal Calculate(List<CartLine> lines, int customerType, string? campaign, string? coupon, bool isPreview)
{
    decimal total = 0;
    int count = 0;
    for (int i = 0; i < lines.Count; i++)
    {
        total = total + lines[i].UnitPrice * lines[i].Quantity;
        count = count + lines[i].Quantity;
    }

    decimal d = 0;
    if (customerType == 2)
    {
        if (campaign == "SUMMER")
            d = total * 0.20m;
        else
            d = total * 0.10m;
    }
    else if (customerType == 1)
    {
        if (campaign == "SUMMER")
            d = total * 0.15m;
        else if (count >= 5)
            d = total * 0.05m;
    }
    else
    {
        if (campaign == "SUMMER" && total >= 500)
            d = total * 0.10m;
    }

    if (coupon != null && coupon != "")
    {
        if (coupon.StartsWith("FIX"))
        {
            if (!(customerType == 2 && campaign == "SUMMER"))
                d = d + decimal.Parse(coupon.Substring(3));
        }
        else if (coupon == "WELCOME10")
        {
            if (customerType == 0)
                d = d + total * 0.10m;
        }
    }

    if (d > total * 0.30m)
        d = total * 0.30m;

    decimal result = total - d;
    if (!isPreview && result < 150)
        result = result + 29.90m;

    return Math.Round(result, 2);
}

The goal is to break this method into readable pieces and move the five-parameter signature to a parameter object. How many commits do you think this takes? And how many minutes in total is it acceptable for the tests to stay red along the way?

The Rhythm: Small Step, Compile, Test, Commit

As in the second post, we start with a safety net. Calculate's input space is small and countable, so approving every combination in one snapshot with Verify's combination support is enough: three carts, three customer types, two campaign states, four coupon values (including null and an empty string) and both values of the preview flag. 144 lines in total:

[Fact]
public Task Calculate_for_all_combinations() =>
    Combination()
        .Verify(
            (string cart, int customerType, string? campaign, string? coupon, bool isPreview) =>
                new PricingEngine().Calculate(Carts[cart], customerType, campaign, coupon, isPreview),
            Carts.Keys,
            [0, 1, 2],
            [null, "SUMMER"],
            [null, "", "WELCOME10", "FIX20"],
            [false, true]);

  one book  , 0, null  , null     , False: 149.90,
  one book  , 0, null  , null     , True : 120.0,
  one book  , 0, null  , WELCOME10, False: 137.90,
  one book  , 0, null  , WELCOME10, True : 108.00,

It matters that this test runs in under a second, because we'll run it after every small step. The rhythm: make a single transformation, compile, run the tests, commit if green. If red, don't try to fix it; undo the step and try a smaller one. The value of this discipline is that when something breaks, the culprit is always the last few lines.

The IDE's automated refactorings can do most of the steps: Rename, Extract Method, Introduce Parameter Object, Move. They work on Roslyn's semantic model; when you rename a variable they don't touch another variable with the same name, and they compute the parameters of an extracted method from the variables it uses. Ctrl+. in Visual Studio and VS Code and Alt+Enter in Rider open these menus; the details are in the IDEs series post "Refactoring Tools and Code Style: Quick Actions, Alt+Enter, .editorconfig and Code Cleanup". Automated refactoring is safer than editing by hand, but it isn't magic; at the end of the post we'll see where it gets things wrong.

Mechanical Cleanup First, in Its Own Commit

Leaving changes that need no thinking to a tool and putting them in a separate commit keeps the diffs of the later steps clean. For the x = x + y assignments and the Substring call in Calculate there are two IDE rules: IDE0054 (compound assignment) and IDE0057 (range operator). First we look at what the tool would do:

$ dotnet format style --diagnostics IDE0054 IDE0057 --severity info --verify-no-changes
src/Bookstore.Pricing/PricingEngine.cs(12,19): info IDE0054: Use compound assignment [src/Bookstore.Pricing/Bookstore.Pricing.csproj]
src/Bookstore.Pricing/PricingEngine.cs(13,19): info IDE0054: Use compound assignment [src/Bookstore.Pricing/Bookstore.Pricing.csproj]
src/Bookstore.Pricing/PricingEngine.cs(42,23): info IDE0054: Use compound assignment [src/Bookstore.Pricing/Bookstore.Pricing.csproj]
src/Bookstore.Pricing/PricingEngine.cs(47,23): info IDE0054: Use compound assignment [src/Bookstore.Pricing/Bookstore.Pricing.csproj]
src/Bookstore.Pricing/PricingEngine.cs(56,20): info IDE0054: Use compound assignment [src/Bookstore.Pricing/Bookstore.Pricing.csproj]
src/Bookstore.Pricing/PricingEngine.cs(42,60): info IDE0057: Substring can be simplified [src/Bookstore.Pricing/Bookstore.Pricing.csproj]

Without --verify-no-changes the same command applies the fixes. The result is a five-line diff; after reading it line by line and running the tests, it goes into its own commit:

-                    d = d + decimal.Parse(coupon.Substring(3));
+                    d += decimal.Parse(coupon[3..]);

The IDE equivalent is the "Fix all occurrences in: Document / Project / Solution" option in the lightbulb menu. On the command line, code-style rules starting with IDE are fixed with dotnet format style, and CA or third-party analyzer rules with dotnet format analyzers; both let you pick rules one by one with --diagnostics. Applying bulk fixes rule by rule makes even a diff touching hundreds of files reviewable: the reviewer's only question becomes "was this rule applied correctly?"

In the Java world the well-known tool for this kind of bulk transformation is OpenRewrite; its C# support is developed by Moderne. As of 14 November 2026, the C# parser and base recipes come under the Moderne Source Available License, so you can run them on your own code at no cost. Many C# recipes in the catalog, however (such as the code quality and dependency analysis recipes), are labelled "Proprietary" and need a commercial license. C# recipes run not through a Maven or Gradle plugin but through the Moderne CLI (mod run). Check the license of each recipe you pick before using it. Writing bulk transformations specific to your own code base (a custom Roslyn analyzer and code fix) is the subject of the Static Analysis series.

Opening Up Calculate: Rename, Split, Extract, Move

The first step is names. d, total, count and result become discount, subtotal, bookCount and net. Since the Rename refactoring changes a single variable along with all its uses, this step is nearly risk-free, yet it is the most valuable one for reading the steps that follow.

The second step is splitting the loop. The single loop does two jobs at once: it adds up the amount and the number of books. Once separated, each becomes a one-line expression:

decimal subtotal = lines.Sum(l => l.UnitPrice * l.Quantity);
int bookCount = lines.Sum(l => l.Quantity);

Walking the list twice makes no measurable performance difference; a cart has a few dozen lines at most. If performance ever becomes a problem, it gets measured and, if needed, we go back to a single loop.

The third, fourth and fifth steps are Extract Method: the discount based on customer type and campaign goes to TierDiscount, the coupon to CouponDiscount, the shipping fee to ShippingFee. The shipping step holds a small lesson. The first version I wrote by hand was this:

private static decimal ShippingFee(decimal net) => net < 150 ? 29.90m : 0.00m;

Mathematically correct, but the snapshot went red in 13 of the 144 combinations:

<   five books, 0, null  , null     , False: 375.0,
---
>   five books, 0, null  , null     , False: 375.00,

In .NET a decimal carries its number of decimal places (its scale) as part of the value. 375m + 0.00m is 375.00, and ToString(), the JSON serializer and Verify all write it as "375.00". The old code added nothing to orders without a shipping fee, so the scale was preserved. Written with 0m, the test went green again. A reviewer's eye won't catch a difference like this; a client consuming the API response might. That is exactly the value of a good safety net.

The last step is Move: the shipping fee is knowledge of the shipping policy, not of pricing. We move ShippingFee into its own static class as ShippingPolicy.FeeFor. In the end the body of Calculate has become text that reads as its intent:

public decimal Calculate(PriceRequest request, bool isPreview)
{
    (List<CartLine> lines, int customerType, string? campaign, string? coupon) = request;

    decimal subtotal = lines.Sum(l => l.UnitPrice * l.Quantity);
    int bookCount = lines.Sum(l => l.Quantity);

    decimal discount = TierDiscount(customerType, campaign, subtotal, bookCount);

    discount += CouponDiscount(coupon, customerType, campaign, subtotal);

    if (discount > subtotal * 0.30m)
        discount = subtotal * 0.30m;

    decimal net = subtotal - discount;
    if (!isPreview)
        net += ShippingPolicy.FeeFor(net);

    return Math.Round(net, 2);
}

The nested ifs inside TierDiscount and CouponDiscount are still there, and customerType is still an int. They were left on purpose: changing the conditional structure isn't a mechanical step but a design decision, and it is the subject of the fourth post.

The Answer: Fourteen Commits, No Red Commit

The whole transformation was done in these commits. The tests ran before each one and none was committed red; the only time things stayed red was the few minutes when I saw the scale difference in the shipping step.

$ git log --oneline
c26eaad refactor: move ShippingFee to ShippingPolicy
625958b refactor: remove obsolete Calculate overload (contract)
de6305d test: characterization test uses PriceRequest
a900882 refactor: QuoteEmail uses PriceRequest (migrate 3/3)
dcd8733 refactor: CartPreview uses PriceRequest (migrate 2/3)
58f1a98 refactor: CheckoutService uses PriceRequest (migrate 1/3)
0025068 refactor: introduce PriceRequest overload (expand)
80f3491 refactor: extract ShippingFee
6f69f04 refactor: extract CouponDiscount
ca8167f refactor: extract TierDiscount
5758d9e refactor: split the totals loop into two Sum calls
cd9cb85 refactor: rename locals in Calculate
e2a6762 style: IDE0054 and IDE0057 via dotnet format
8bf1f27 test: characterize PricingEngine.Calculate (144 combinations)

Fourteen commits may look like a lot, but each is a few lines and does one thing. If a bug showed up in production, git bisect would narrow it down to a one-step diff. Had we squeezed everything into a single "refactor PricingEngine" commit, we would be hunting the same bug in a diff covering almost the whole file. How many pieces you split the PR itself into depends on your team's review capacity; the signature change in the next section draws a good boundary for that.

Changing a Signature: Parallel Change

Turning the Calculate(lines, customerType, campaign, coupon, isPreview) signature into Calculate(PriceRequest request, bool isPreview) requires changing every caller at once. If the three callers live in the same repo, the IDE's Change Signature refactoring can do it in one move. But if the callers are in other teams' code, in another repo or among the users of a NuGet package, one move isn't possible. Parallel change (expand and contract), which Danilo Sato described on martinfowler.com in 2014, is for this situation: splitting the change into three separate phases.

In the expand phase the new signature is added and the old one forwards to it. Marking the old one with [Obsolete] makes the compiler list the remaining callers for us:

[Obsolete("Use Calculate(PriceRequest, bool) instead.")]
public decimal Calculate(List<CartLine> lines, int customerType, string? campaign, string? coupon, bool isPreview) =>
    Calculate(new PriceRequest(lines, customerType, campaign, coupon), isPreview);

public decimal Calculate(PriceRequest request, bool isPreview)
{
    // ... the old body

$ dotnet build --no-incremental
src/Bookstore.Ordering/Callers.cs(8,9): warning CS0618: 'PricingEngine.Calculate(List<CartLine>, int, string?, string?, bool)' is obsolete: 'Use Calculate(PriceRequest, bool) instead.' [src/Bookstore.Ordering/Bookstore.Ordering.csproj]
src/Bookstore.Ordering/Callers.cs(14,9): warning CS0618: 'PricingEngine.Calculate(List<CartLine>, int, string?, string?, bool)' is obsolete: 'Use Calculate(PriceRequest, bool) instead.' [src/Bookstore.Ordering/Bookstore.Ordering.csproj]
src/Bookstore.Ordering/Callers.cs(20,24): warning CS0618: 'PricingEngine.Calculate(List<CartLine>, int, string?, string?, bool)' is obsolete: 'Use Calculate(PriceRequest, bool) instead.' [src/Bookstore.Ordering/Bookstore.Ordering.csproj]
tests/Bookstore.Pricing.Tests/PricingEngineCharacterizationTests.cs(19,21): warning CS0618: 'PricingEngine.Calculate(List<CartLine>, int, string?, string?, bool)' is obsolete: 'Use Calculate(PriceRequest, bool) instead.' [tests/Bookstore.Pricing.Tests/Bookstore.Pricing.Tests.csproj]

Four warnings, four callers. In the migrate phase each caller moves to the new signature in its own commit, and the warning count drops 4, 3, 2, 1. I left the characterization test for last on purpose: as long as it calls the old signature, it also verifies that the old signature forwards correctly to the new code. When the last caller has moved, the warning count is zero; in the contract phase the old overload is deleted.

These three phases may or may not fit in one PR. The rule is simple: at the end of each phase the system compiles, the tests pass and it can be deployed. If the callers are in other teams, expand is one PR, each team's migration is its own PR, and contract is a separate PR once everyone has moved. If you write a library, carrying the old signature as [Obsolete] for at least one release gives your users time to move. In a project with TreatWarningsAsErrors on, CS0618 breaks the build; during the expand phase you may need to add this warning to WarningsNotAsErrors temporarily.

Where Automated Refactoring Can't See

The IDE's rename follows C# symbols. It can't see places where a name lives outside C#, inside a string or a convention. The small program below shows two common cases:

// 1) JSON contract: the property name is the API field name.
Console.WriteLine(JsonSerializer.Serialize(new PriceQuote(375m, 0m)));
Console.WriteLine(JsonSerializer.Serialize(new PriceQuote(375m + 0.00m, 0m)));

// 2) Registration by convention: every class whose name ends in "Rule" is a discount rule.
ServiceCollection services = new();
foreach (Type type in Assembly.GetExecutingAssembly().GetTypes()
             .Where(t => t.IsClass && !t.IsAbstract && t.Name.EndsWith("Rule")))
{
    services.AddSingleton(typeof(IDiscountRule), type);
}
using ServiceProvider provider = services.BuildServiceProvider();
Console.WriteLine($"Registered rules: {string.Join(", ", provider.GetServices<IDiscountRule>().Select(r => r.GetType().Name))}");

public sealed record PriceQuote(decimal Total, decimal Discount);

public interface IDiscountRule;
public sealed class MemberRule : IDiscountRule;
public sealed class VipRule : IDiscountRule;
public sealed class SummerCampaignRule : IDiscountRule;

{"Total":375,"Discount":0}
{"Total":375.00,"Discount":0}
Registered rules: MemberRule, VipRule, SummerCampaignRule

Now we do two renames: PriceQuote.Total becomes GrandTotal, and VipRule becomes the more descriptive VipDiscount. The build has zero warnings and zero errors. The output, however:

{"GrandTotal":375,"Discount":0}
{"GrandTotal":375.00,"Discount":0}
Registered rules: MemberRule, SummerCampaignRule

The API's JSON contract changed, and every client reading that field will silently start seeing null. The VIP discount also dropped out of the DI container; no compiler error, no exception, VIP customers simply no longer get a discount. The second line is the API-side counterpart of the scale issue from the shipping step: the same amount, different JSON.

The family grows. Methods found by reflection through a string (GetMethod("Calculate")), the column name EF Core derives from a property name (a rename turns into a column rename in the next migration), property names in Razor and Angular templates, options classes bound to section names in appsettings.json, field names in structured log messages. How to protect these on the code side ([JsonPropertyName], HasColumnName, the tolerant reader) we'll see in detail in the seventh post, under refactoring while contracts change. For now the practical rule: after a rename, search the solution for the old name as plain text; use the IDEs' "rename in strings and comments" option deliberately; and have tests that pin these names (a JSON snapshot, a DI registration test).

Common Pitfalls

Starting with a big step and staying red for hours. Work that begins with "let me move all the discount logic into strategies, then I'll run the tests" ends two hours later with 40 compile errors and a developer who doesn't know which change broke what. If a test goes red and you can't fix it within two minutes, undo (git restore or git stash) and make the step smaller.

Merging automated tool output without reviewing it. dotnet format or "Fix all in solution" can change hundreds of files. The tool applies the rule correctly, but the rule may not be right in every context: there are cases where a code fix changes the public API or touches generated code. Bulk fixes come in their own commit, applied rule by rule, and are read at least by sampling.

Putting a mechanical change and a meaningful change in the same commit. When a formatting fix and an extract method sit in the same diff, the reviewer has to look for the important line among hundreds of indentation changes. It is the tool version of the two-hats rule from the first post: mechanical work separate, work that needs thought separate.

Forgetting the contract phase of a parallel change. Expand gets done, half the callers move, then priorities shift and two signatures doing the same job live in the code base for years. Opening the contract step as an issue and tracking the [Obsolete] warnings prevents this.

Updating the snapshot because "nothing really changed". The 13-line difference in the shipping step looked unimportant at first sight. Whether it is unimportant is decided by understanding where the difference comes from, not by updating the snapshot.

A Small Experiment

★☆☆ Run dotnet format style --diagnostics IDE0054 --severity info --verify-no-changes on your own code base. How many violations in how many files? Estimate how long it would take to review the result as a single commit.

★★☆ Set up the combination snapshot and open up this post's Calculate step by step yourself. Run the test after each step and compare your git log with the one here. Where did you need fewer or more commits?

★★★ Apply parallel change to a public method in your team: add the new signature, make the old one [Obsolete], get the CS0618 list with dotnet build --no-incremental. Write down in an issue how many PRs you'll split the callers into and when the contract phase will happen.

Calculate reads well now, but the pile of conditionals inside TierDiscount and CouponDiscount is still there, and the isPreview flag makes one method do two jobs. In the next post we'll turn these conditionals into three different targets, polymorphism, a switch expression and a rule table, and weigh which one fits which kind of change.