What Refactoring Is and Isn't: Preserving Behavior, the Two Hats and the Rewrite Decision

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.

Guess First: A PR That Says "No Behavior Change"

Bookstore's pricing module has a small class that adds up the lines of an order. One afternoon Alan looks at it, finds the foreach loop too long and simplifies it with LINQ. The PR title is clear: no behavior change. This is the diff the reviewer sees:

-    public static decimal Calculate(IEnumerable<OrderLine> lines)
-    {
-        decimal total = 0m;
-        foreach (OrderLine line in lines)
-        {
-            decimal lineTotal = line.UnitPrice * line.Quantity * (1 - line.DiscountRate);
-            total += Math.Round(lineTotal, 2);
-        }
-        return total;
-    }
+    public static decimal Calculate(IEnumerable<OrderLine> lines) =>
+        Math.Round(lines.Sum(LineTotal), 2);
+
+    private static decimal LineTotal(OrderLine line) =>
+        line.UnitPrice * line.Quantity * (1 - line.DiscountRate);

The only test in the repo checks that an order with Dune at 10 percent off and two copies of Emma comes to 765. CI is green:

Test run summary: Passed!
  total: 1
  failed: 0
  succeeded: 1
  skipped: 0
  duration: 473ms

The code is shorter, the test passes, the title says "no behavior change". Would you approve it as the reviewer? Before you do, one question: does an order of two books at 39.90 and 49.90, each with a 15 percent discount, give the same total before and after the change?

Defining Refactoring: Behavior in Whose Eyes?

Martin Fowler's definition on refactoring.com is short: "Refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior." The sentence has two centres of gravity. The first is "disciplined": refactoring is a method, not an intention. The code changes in steps that are each small and behavior-preserving, and the system works after every step. The second is "external behavior": only the internal structure changes, everything visible from outside stays the same.

"Visible from outside" needs to be made concrete, because most arguments start here. The observable behavior of a method is everything its callers and the users of the system can notice: the return value, the type of exception thrown, the rows written to the database, the email sent, the log entry, the field names in an API's JSON output. A one-cent rounding difference belongs on this list too; if it shows up on the customer's invoice, it is behavior.

Some things usually stay off the list. The name of a private method, which file a class lives in, whether a loop is a foreach or LINQ: none of this is visible from outside. Performance sits in between: if the code got 2 percent slower after a refactoring we usually don't call it a behavior change, but if an endpoint's response time is bound by an SLA, we do. So the answer to "did the behavior change?" always depends on context: who is observing what.

In the Clean Code series, "A Code Smell Catalog" showed where smells live and "Clean Code in Practice: Cleaning Up a Legacy OrderService Step by Step" cleaned a single class commit by commit. This series treats refactoring itself as a discipline: the safety net, step size, breaking dependencies in legacy code, planning large changes and explaining all of it to the team.

The Answer: A One-Cent Behavior Change

Let's answer the opening question by running both versions side by side. We add up the same two lines with the old and the new calculation:

OrderLine[] lines =
[
    new("Dune", 39.90m, 1, 0.15m),
    new("Emma", 49.90m, 1, 0.15m)
];

foreach (OrderLine line in lines)
{
    decimal raw = line.UnitPrice * line.Quantity * (1 - line.DiscountRate);
    Console.WriteLine($"{line.Title,-6} raw={raw}  printed={Math.Round(raw, 2)}");
}

Console.WriteLine($"Before refactor: {Before(lines)}");
Console.WriteLine($"After refactor:  {After(lines)}");

Dune   raw=33.9150  printed=33.92
Emma   raw=42.4150  printed=42.42
Before refactor: 76.34
After refactor:  76.33

The old code rounded each line and then added them up: 33.92 + 42.42 = 76.34. The invoice prints the lines the same way. The new code adds the raw values first (76.33) and rounds afterwards. The result is one cent short, and now the lines on the invoice no longer add up to the total at the bottom. Accounting notices at the end of the month, as a difference that has piled up over thousands of orders.

The change isn't malicious; whether Math.Round sits inside or outside the loop is a detail that slips by easily while simplifying code. The real problem is in two places. First, the existing test never exercised this behavior: 450 × 0.90 and 180 × 2 are numbers that need no rounding. Second, the PR title switched off the reviewer's attention. People look at a diff labelled "no behavior change" with less suspicion.

Once we add a test that pins the behavior, the refactored code is caught immediately:

[Fact]
public void Calculate_rounds_each_line_like_the_invoice()
{
    OrderLine[] lines =
    [
        new("Dune", 39.90m, 1, 0.15m),
        new("Emma", 49.90m, 1, 0.15m)
    ];

    Assert.Equal(76.34m, OrderTotals.Calculate(lines));
}

failed Bookstore.Pricing.Tests.OrderTotalsTests.Calculate_rounds_each_line_like_the_invoice (14ms)
  Assert.Equal() Failure: Values differ
  Expected: 76.34
  Actual:   76.33

The lesson: the safety of a refactoring comes from tests that measure behavior, not from the attention of the person changing the code. Without a test, or with a test that doesn't exercise the behavior that matters, what you are doing isn't refactoring; it is a change made in hope. The whole of the second post is about building that safety net.

Why the Word "Refactoring" Drifts

In many teams the word "refactoring" slowly drifts to mean "any work on the code that isn't a new feature". Upgrading the framework, rewriting a module from scratch, changing the database schema, even a restructuring during which the build stays broken for two weeks gets called a "refactoring sprint". Fowler noted this drift in 2004 in "Refactoring Malapropism": if the system is broken for a couple of days, the work is not refactoring but the more general activity of restructuring. Refactoring is a specific way of restructuring: small, behavior-preserving steps and a working system at the end of each one.

The drift does three concrete kinds of damage. The first is in review: the "refactor" label tells the reviewer "you don't need to look at behavior", as in the opening PR. The second is in planning: the Product Owner starts hearing "refactoring" as "work with no business value that developers do for their own pleasure", and rightly questions it. Yet a good refactoring is usually done to lower the cost of the next feature; in the ninth post we'll talk about making that case in measurable terms. The third is risk: when a two-week rewrite is presented as "refactoring", everyone behaves as if the guarantees of small steps were in place, but they aren't.

A practical suggestion: use the word "refactor" in a PR title or commit message only when the behavior really doesn't change. If it does change, or you aren't sure, name what you did: "upgrade", "rewrite", "fix", "change rounding rule".

Two Hats: Never Feature and Refactoring at the Same Time

Kent Beck has a metaphor that spread with Fowler's Refactoring book: when you program, you wear one of two hats. With the refactoring hat on, every change preserves behavior, the tests stay green, and you don't write new tests. With the feature hat on, you change behavior: you add new tests and some tests go red on purpose. You can swap hats often, even every few minutes; what matters is never wearing both at once.

Why does it matter so much? Because when the hats get mixed and a test turns red, you can't tell why. Did the new feature change the behavior as intended, or did the "little cleanup" you did on the side break something? In the opening PR Alan thought he was wearing only the refactoring hat; in fact the behavior changed too, and with no test saying so, nobody noticed.

Let's do the same work with the hats separated. Bookstore gets a new rule: a 5 percent bulk discount on orders of five or more books. First the test that pins the behavior, then the refactoring, finally the feature; each in its own commit:

$ git log --oneline --stat fbd742a..HEAD
e932f1c feat: 5% bulk discount from five books
 src/Bookstore.Pricing/OrderTotals.cs              | 15 +++++++++++++--
 tests/Bookstore.Pricing.Tests/OrderTotalsTests.cs |  8 ++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)
5e22257 refactor: extract OrderTotals.LineTotal
 src/Bookstore.Pricing/OrderTotals.cs | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
ad13c13 test: pin per-line rounding of OrderTotals
 tests/Bookstore.Pricing.Tests/OrderTotalsTests.cs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

The first commit only adds a test, and it is green against the old code. The second commit changes only production code and doesn't touch the test file; since the same tests stay green, we know the behavior is preserved. This time the rounding is in the right place, inside the line calculation:

public static decimal Calculate(IEnumerable<OrderLine> lines) =>
    lines.Sum(LineTotal);

// The invoice rounds every line on its own; the total must follow the same rule.
private static decimal LineTotal(OrderLine line) =>
    Math.Round(line.UnitPrice * line.Quantity * (1 - line.DiscountRate), 2);

The third commit changes behavior and says so openly: a new test, new code. The reviewer's job gets easier: on the refactor: commit they check "is the test file untouched and are the tests green?", on the feat: commit they focus on the business rule. If you need to tidy up commits afterwards, the steps in the Git series post "Git Interactive Rebase and Squash: Tidying History Before a PR" apply here too.

Kent Beck's 2023 book "Tidy First?" takes the same idea one step further. Beck calls small edits that don't change behavior "tidyings" and recommends not mixing structural changes and behavior changes in one PR: ideally they go in separate PRs, at the very least in separate commits. A Beck line that Fowler passes on in his 2015 article "An example of preparatory refactoring" sums up the order: "make the change easy, then make the easy change". First build the structure that makes the change easy, then make the change that has become easy.

When Do You Refactor?

Refactoring doesn't have to be a separate line in the project plan; it is most effective when spread through daily work. Fowler names these moments one by one in his 2014 piece "Workflows of Refactoring". Three of them, with Bookstore examples, are enough for everyday work.

Preparatory refactoring. Reshaping OrderTotals so the line calculation sits in its own method before the bulk discount arrived was exactly this. If a new feature fits awkwardly into the existing structure, you change the structure first; the feature then goes in with a few lines.

Comprehension refactoring. If you go back to the start of a method three times while reading it, refactoring is how you write what you understood into the code: giving the tmp2 variable a meaningful name, extracting a condition into a named method. If what you understood stays in your head, the next reader walks the same path from scratch.

Litter-pickup refactoring. If you notice dead code or a badly named parameter next to the bug you are fixing, you fix it; if it is big, you note it down for later. This is the habit teams know as the camp site rule or the boy scout rule.

There are planned and long-term refactorings too: transforming a module from end to end, an infrastructure change spread over months. These should be the exception, and they need the techniques of the sixth post. There are also times when not refactoring is reasonable: code that works, that nobody reads and that you won't touch again; a module that will be deleted soon; or the rare cases where rewriting really is cheaper.

Rewrite or Refactor?

Sooner or later every team facing a legacy module asks: "Wouldn't it be faster to write this from scratch?" Joel Spolsky's "Things You Should Never Do, Part I" from 6 April 2000 is the classic warning about that question. Netscape had decided to rewrite its browser from scratch; almost three years passed between 4.0 and 6.0, and in that time its market share melted away. Spolsky's main argument: every ugly-looking corner of old code carries the trace of a bug that was once found and fixed. When you rewrite from scratch, you throw that accumulated knowledge away as well.

The risks of a big rewrite feed each other. The new system delivers nothing to users until it does everything the old one did. In the meantime bug fixes and new features landing in the old system have to be carried over to the new one, so the target keeps moving. Undocumented behavior of the old system, like the per-line rounding from the opening, only surfaces when someone complains in production. And teams tend to design the second system to also do everything the first one didn't.

Still, there are cases where a rewrite is defensible. When the platform is dead, for example when moving a Web Forms UI on .NET Framework to .NET 10 is impossible without rewriting the pages. When the module is small, its behavior is well defined and there are tests describing it. When the old behavior itself is no longer wanted, that is, the business rules will change as well. Even then, rewriting piece by piece instead of in one go removes most of the risk: old and new run side by side and traffic moves to the new side gradually. The Microservices series post "From Monolith to Microservices: Strangler Fig, Branch by Abstraction and the Distributed Monolith Trap" covers this at service scale, and the sixth post of this series covers it inside a monolith.

A simple question helps with the decision: "Can I list everything the old code does today?" If you can't, you aren't ready to rewrite; you first need to build that list with characterization tests. If you can, refactoring has probably stopped being scary as well.

The Series Roadmap

Instead of touring Fowler's catalog in order, this series moves through Bookstore's legacy parts. Technique names come up where they are needed. Each part on the map stands for a kind of problem.

InvoiceRenderer is a 400-line class that builds the text of the order invoice; it has no tests and its output is a long piece of text. In the second post, before touching it, we'll record its current output with approval testing and build a safety net. PricingEngine has a long Calculate method and conditionals in which customer type, campaign and coupon are nested inside each other. In the third post we'll open it up in small steps with IDE refactorings, and in the fourth we'll turn the conditionals into a readable design.

OrderImportJob is a background job that reads a CSV file, calls a static SmtpHelper and uses DateTime.Now and new SqlConnection. In the fifth post we'll break its dependencies to make it testable. LegacyShippingClient is an old client that talks to the shipping company through synchronous, static calls; in the sixth post we'll replace it with a modern client using the Mikado Method and branch by abstraction. The seventh post covers changing the Book.Price field together in the API, the database and the Angular model, the eighth modernizes the old CheckoutComponent in Angular, and the ninth uses hotspot analysis to choose which of these parts to invest in first.

Common Pitfalls

The untested "refactor" PR. The problem with the opening PR wasn't the code itself but that nothing verified the change. If there is no test that measures the behavior, that test gets written first; if it can't be written because the code is too entangled to test, the work starts with the dependency-breaking techniques of the fifth post.

Hiding a behavior change in a refactor commit. A small rule added "while I was here", a fixed null check or a changed default value disappears inside a commit titled refactor:. Later, when you hunt for a bug, git log points you to the wrong place. A behavior change, however small, comes in its own commit.

Making refactoring a separate project phase. The plan "let's finish the features first, then do a refactoring sprint" usually ends with that sprint never arriving. If it does arrive, the backlog of work is big and risky. Small, continuous refactoring is cheaper than rare, big refactoring.

Changing code because it would be "prettier". A refactoring needs a purpose: making the next change easier or making the code understandable. Reshaping a working module that nobody touches to suit personal taste creates review load and carries risk while gaining nothing.

Basing the rewrite decision on a guess. The estimate "we'll rewrite it in six months" rests on the assumption that everything the old system does is known. That assumption is rarely true; recording the behavior before deciding makes the estimate realistic and often shows that refactoring is enough.

A Small Experiment

★☆☆ Look at the messages of the last 50 commits in your own repo (git log --oneline -50) and mark the ones that mention "refactor". Glance at each diff: did a test file change, was a business rule added? The count shows how far the word has drifted in your team.

★★☆ Set up the opening OrderTotals example yourself. First run the refactored version with only the first test and see it go green. Then add the second test that catches the rounding difference. Write down which inputs you tried to find that second test; which numbers trigger the rounding?

★★★ Pick a module your team recently talked about "rewriting from scratch". List everything the module does today: inputs, outputs, side effects, known quirks. How many items did the list reach, and how many are you unsure about? The ones you are unsure about are the hidden cost of the rewrite.

A single test caught the bug in the opening; but in real legacy code you don't know which test you need to write. In the next post, before touching the 400-line InvoiceRenderer, we'll record its current behavior with characterization tests and approval testing, then check with deliberate mutations whether that safety net really protects us.