A test-first playbook for refactoring legacy C# safely
The fastest way to break production is to "clean up" code you don't have tests for. You open a crusty 200-line method, your instincts are good, you tidy as you go, and somewhere in the tidying you change behavior nobody asked you to change. The diff looks like an improvement. The pager disagrees.
The mistake isn't the refactoring. It's refactoring without a safety net. Real refactoring has a strict definition: changing the structure of code without changing its behavior. If behavior moved, you didn't refactor. You rewrote, and you did it blind. So the entire discipline comes down to one question you must be able to answer at every step: how do I know the behavior is the same?
The answer is tests. Specifically, tests written before you touch a thing.
Step 1: Characterization tests, not "correct" tests
Legacy code rarely comes with a spec, and the behavior you're inheriting might even be subtly wrong. That's fine. You're not trying to capture what the code should do. You're capturing what it currently does, quirks and all, so that any change you make announces itself.
These are characterization tests. You write them by feeding the code inputs and pinning whatever it returns, even if the output looks odd.
[Fact]
public void Price_BooksOverFifty_AppliesTenPercentOff()
{
var order = new Order([new("Books", 30m, 2)]); // 60.00 before discount
var sut = new Pricing();
// Pin the current behavior, whatever it is
Assert.Equal(54.00m, sut.Price(order));
}If a test surprises you, that's the point. You just learned something about the system before you could break it. Write enough of these to cover the branches you can see, and run them. Green means you now have a net.
If you can't get the code under test at all, that's not a reason to skip this step. It's the first thing to fix, in step 3.
Step 2: Small, behavior-preserving steps
With the net in place, you move in the smallest steps you can stand, running the tests after each one. The point of small steps is not caution for its own sake. It's that when a test goes red, you want the suspect list to be one line long.
The three moves you'll reach for most:
Extract method. Pull a coherent block out into its own named method. The name does documentation work, and the smaller method is testable on its own.
// Before: a discount rule buried inline
var line = item.UnitPrice * item.Quantity;
if (item.Category == "Books") line *= 0.9m;
// After: intent in the name, logic isolated
var line = DiscountedLineTotal(item);Introduce a seam. A seam is a place where you can swap behavior without editing the code
around it. The classic case is a method that reaches straight for DateTime.Now or a concrete
service, which makes it untestable. Introduce an interface and inject it, and you've created a
point of control.
public interface IClock
{
DateTime UtcNow { get; }
}
// The class now depends on an abstraction it can be handed in a test
public sealed class Subscription(IClock clock)
{
public bool IsExpired(DateTime expiry) => clock.UtcNow > expiry;
}That one move often turns "I can't test this" into "I have a net," which is why it sometimes belongs before step 1 for the truly stuck cases.
Replace conditional with polymorphism. When a switch or chain of ifs keeps branching on
the same "type" value, each new case bloats the method and its complexity. Push each branch into
its own type behind a shared interface.
public interface IDiscount
{
decimal Apply(decimal line);
}
public sealed class BookDiscount : IDiscount
{
public decimal Apply(decimal line) => line * 0.9m;
}
public sealed class BulkElectronicsDiscount : IDiscount
{
public decimal Apply(decimal line) => line > 1000 ? line * 0.95m : line;
}The pricing method stops growing an else if every quarter. Adding a category becomes adding a
class, and the existing ones don't change, so they can't regress.
Step 3: Run the tests constantly
This is the part people nod at and then skip under deadline pressure. Run the suite after every move, not at the end of the afternoon. A batch of ten changes followed by a red bar gives you a ten-suspect mystery. One change followed by a red bar gives you the culprit and a one-keystroke undo.
The rhythm is: small change, run tests, commit if green. Then repeat. When you finish, the structure is better and every test that was green at the start is still green, which is the proof that you refactored instead of rewrote.
The two constraints that make it real
Notice that good refactoring is squeezed between two forces pulling in opposite directions. The tests hold behavior still. The cleanup pushes structure forward. Satisfy one and not the other and you've failed: pass the tests but leave the mess, or clean the mess but break the bill. Holding both at once is the actual skill, and it's the one that's hard to practice on your own, because grading your own "did I preserve behavior?" by eye is exactly the thing that fails under pressure.
method length
limit 25 lines
cyclomatic complexity
limit 8
nesting depth
limit 3 levels
✓ 9 / 9 tests still passing (behavior never changed)
That two-sided constraint is what Katabench's refactoring katas are built to train. Each one hands you working-but-gnarly code and grades your submission on both axes at once: a behavioral test suite confirms you didn't change what the code does, and Roslyn re-measures the structure (complexity, nesting, method length, duplication) against authored thresholds to confirm you actually improved it. You don't pass by being correct. You don't pass by being tidy. You pass when both are true, which is the same bar production holds you to.
It's the closest thing to the real experience of inheriting someone's code and being told "make this better, and don't break it." If you want reps on that specific tension, the refactoring track is on Pro. It's a lot cheaper than learning the lesson from a postmortem.