Every team has one: a seventy-line method buried in nested if statements that nobody dares to touch because it calculates real money. When the boss asks for "a new coupon for new customers", a developer has to read all seventy lines, wedge in another else if, and hope no other branch breaks. This article is a case study in refactoring: we take exactly that kind of method, running on JDK 21, and move it from one big if/else block to a Strategy structure in small steps, with a 2,835-combination safety net guarding every step. The goal is not to show off patterns. It is to show when a pattern appears naturally and when to stop.

This article follows Testing patterns: Test Data Builder, Object Mother and test doubles, where we covered how to write durable tests, and Anti-patterns in Java: God object, anemic model, service locator, where we learned to spot design smells. The underlying pattern used here already has its own article: Strategy pattern with lambdas and functional interfaces. The code lives in code/s6-35 and runs on JDK 21.0.12 and JUnit 5.10.5 (via spring-boot-starter-parent 3.3.5) on an Apple M4 with 16 GB of RAM.

What is refactoring, and why refactor "to" patterns instead of "with" patterns

Martin Fowler defines refactoring in two senses. As a noun, it is a change to the internal structure that makes code easier to understand and cheaper to change later, without changing its externally observable behavior. As a verb, it is applying a series of such refactorings. The crucial part is the second half: behavior does not change. Changing behavior (fixing a bug, adding a feature) is a different job, and mixing the two in one commit is the source of most risk.

Joshua Kerievsky pushes the idea further in Refactoring to Patterns (Addison-Wesley, 2004). According to Industrial Logic's overview page, the book's central thesis is that using patterns to improve an existing design is better than using patterns early in a new design. In other words, a pattern is the destination of a series of small transformations once the code has shown real pain, not a blueprint to impose up front. This view avoids the "pattern-happy" disease of building Factory, Strategy, and Visitor for a business rule that will still have only two branches next year.

Replace Conditional with Polymorphism is the specific technique we will use. When a conditional chooses behavior by the type or property of an object, we replace it with objects that share an interface, each holding one branch. The guide on refactoring.guru sets a clear preparation step: if the conditional sits inside a method that does other work, apply Extract Method first. The case study below follows that order.

The original code: a quote calculation method

The problem is the quoting system of a multichannel store. The total function takes the customer type (RETAIL, VIP, DEALER), the sales channel (WEB, STORE, MARKETPLACE), the region, a coupon code, the shipping method, and the cart, and returns the amount to pay (in the local currency unit).

The business rules piled up over many rounds of requests: VIP customers get 5% off; dealers get 3%, or 12% when the order is at least 5,000,000; the GIAM10 coupon gives 10% off, but only when it beats the discount already applied, and the discount is capped at 500,000; FREESHIP waives shipping; NEWCUS subtracts a flat 50,000 for orders of at least 300,000; coupons do not apply on the marketplace channel; stores charge no shipping; standard shipping is free from 1,000,000 after discount; and VAT is 10%. This is the code we will refactor. Here is the coupon-applying section of the original (LegacyQuoteCalculator, 73 lines for the total method alone):

        // coupon
        long flat = 0;
        boolean freeShip = false;
        if (coupon != null) {
            if (coupon.equals("GIAM10")) {
                if (!channel.equals("MARKETPLACE")) {
                    if (percent < 10) {
                        percent = 10;
                    }
                }
            } else if (coupon.equals("FREESHIP")) {
                if (!channel.equals("MARKETPLACE")) {
                    freeShip = true;
                }
            } else if (coupon.equals("NEWCUS")) {
                if (!channel.equals("MARKETPLACE") && subtotal >= 300_000) {
                    flat = 50_000;
                }
            }
        }
        long discount = subtotal * percent / 100;
        if (coupon != null && coupon.equals("GIAM10") && !channel.equals("MARKETPLACE") && percent == 10 && discount > 500_000) {
            discount = 500_000;
        }

The shipping fee section below it is even longer, and it is the next target of the refactoring. The smells are visible at once: the method does five jobs, the freeShip and flat flags are intermediate variables leaking from one block to another, the !channel.equals("MARKETPLACE") condition repeats three times, and line 48 of the file has to restate the code, the channel, and percent == 10 just to know whether the 500,000 cap applies. The counting tool in the repo reports 29 decision points in the method (if, case, for, &&, ||, ?). This is a raw count, not true cyclomatic complexity, but it is enough to compare versions.

Step 0 of the refactoring: lock behavior with a characterization test

Before the first refactoring edit, before changing a single line, we need something that guarantees "behavior does not change". For old code with no tests, the most practical tool is the characterization test that Michael Feathers describes in Working Effectively with Legacy Code: a test that records the code's actual behavior, even when that behavior is probably wrong. The purpose is not to check correctness but to detect unintended change.

Here we use the "golden master" variant: run the original over a wide grid of inputs, write the results to a file, then compare every combination. The grid has 3 customer types, 3 channels, 3 regions, 5 coupon values (including null and a nonexistent code), 3 shipping methods, and 7 carts: 3×3×3×5×3×7 = 2,835 combinations. The seven carts are not random: four are representative (small, medium, large, very large) and three sit exactly on the boundary of the >= conditions, that is 300,000, 1,000,000, and 5,000,000.

Why care about boundaries? This lesson comes from my own first run. With a grid of only the four representative carts (1,620 combinations), I wrote a manual mutation script (mutate.sh): change one comparison operator in the final version, rerun the tests, and see whether they turn red. The real results:

NOT caught (tests still green) | subtotal >= 5_000_000 ? 12 : 3  ->  subtotal > 5_000_000 ? 12 : 3
NOT caught (tests still green) | ctx.subtotal() >= 300_000  ->  ctx.subtotal() > 300_000
NOT caught (tests still green) | afterDiscount >= 1_000_000 || freeShip  ->  afterDiscount > 1_000_000 || freeShip
CAUGHT | Math.min(discount, 500_000)  ->  Math.min(discount, 400_000)
CAUGHT | new long\[\] {45_000, 60_000, 55_000}  ->  new long[] {45_000, 60_000, 56_000}

The three mutations that change >= to > at exactly the 5,000,000, 300,000, and 1,000,000 thresholds were not caught, even though the grid had 1,620 combinations. The two mutations that change constants (the discount cap, the express shipping fee) were caught. The reason: no cart totals exactly the threshold, so >= and > give the same result. A grid with many rows is not necessarily a good grid. After adding the three boundary carts (the grid becomes 2,835 combinations), I reran it:

CAUGHT | subtotal >= 5_000_000 ? 12 : 3  ->  subtotal > 5_000_000 ? 12 : 3
CAUGHT | ctx.subtotal() >= 300_000  ->  ctx.subtotal() > 300_000
CAUGHT | afterDiscount >= 1_000_000 || freeShip  ->  afterDiscount > 1_000_000 || freeShip
CAUGHT | Math.min(discount, 500_000)  ->  Math.min(discount, 400_000)
CAUGHT | new long\[\] {45_000, 60_000, 55_000}  ->  new long[] {45_000, 60_000, 56_000}

All five mutations were caught. That is the safety net every refactoring needs. The comparison lives in a JUnit 5 @TestFactory, which generates one test per code version and compares against the golden.csv file:

    @TestFactory
    Stream<DynamicTest> everyVersionMatchesGoldenMaster() throws IOException {
        Map<String, Long> golden = golden();
        return VERSIONS.entrySet().stream().map(e -> DynamicTest.dynamicTest(e.getKey(), () -> {
            int n = 0;
            for (Grid.Case c : Grid.cases()) {
                long actual = e.getValue().total(c.customerType(), c.channel(), c.region(), c.coupon(),
                        c.shipping(), Grid.cart(c.cart()));
                assertEquals(golden.get(c.key()), actual, () -> e.getKey() + " mismatch at " + c.key());
                n++;
            }
            System.out.println(e.getKey() + ": " + n + "/" + golden.size() + " combinations match the golden master");
        }));

Step 1 of the refactoring: Extract Method, one named rule at a time

The first refactoring step does not touch patterns at all. We only give names to each block: subtotal, customerPercent, couponEffect, shippingFee. The intermediate variables (percent, flat, freeShip, the cap flag) that leaked everywhere are gathered into a record CouponEffect, an immutable value returned from a single place. The main method now reads like a story:

    @Override
    public long total(String customerType, String channel, String region, String coupon, String shipping, List<Line> lines) {
        long subtotal = subtotal(lines);
        CouponEffect effect = couponEffect(coupon, channel, subtotal, customerPercent(customerType, subtotal));
        long discount = subtotal * effect.percent() / 100;
        if (effect.capped() && discount > 500_000) {
            discount = 500_000;
        }
        long afterDiscount = subtotal - discount - effect.flat();
        long fee = shippingFee(channel, shipping, region, afterDiscount, effect.freeShip());
        return afterDiscount + afterDiscount * 10 / 100 + fee;
    }

Notable changes from this refactoring: the "coupons do not apply on the marketplace" condition now lives in one place (the top of couponEffect) instead of three. The discount cap no longer has to be inferred from percent == 10, because CouponEffect carries the capped flag. Running the tests:

v0 legacy: 2835/2835 combinations match the golden master
v1 extract: 2835/2835 combinations match the golden master
v2 strategy: 2835/2835 combinations match the golden master
v3 coupon: 2835/2835 combinations match the golden master
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 -- in demo.quote.CharacterizationTest
GIAM20 on 1,000,000, North region, standard shipping: 905000
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 -- in demo.quote.NewCouponTest
[INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

The line v1 extract: 2835/2835 combinations match the golden master confirms that behavior is unchanged. Extract Method is something an IDE can do automatically, so the risk is very low.

Step 2 of the refactoring: Replace Conditional with Polymorphism

After the first refactoring step, the two remaining conditional chains have the same shape: they choose behavior by a string (customer type, shipping method). This is exactly the signal the polymorphism technique targets in a refactoring. With Java 21 we do not need a class hierarchy: a single-method interface plus lambdas is enough (see the earlier article on the Strategy pattern with lambdas), registered in a Map keyed by string:

    private static final Map<String, CustomerPolicy> CUSTOMERS = Map.of(
            "VIP", subtotal -> 5,
            "DEALER", subtotal -> subtotal >= 5_000_000 ? 12 : 3);

    /** Strategy 2: how shipping fees are calculated. */
    interface ShippingPolicy {
        long fee(String region, long afterDiscount, boolean freeShip);
        ShippingPolicy FREE = (region, afterDiscount, freeShip) -> 0;
    }
    private static final Map<String, ShippingPolicy> SHIPPINGS = Map.of(
            "STANDARD", (region, after, free) -> (after >= 1_000_000 || free) ? 0 : new Fees(25_000, 35_000, 30_000).of(region),
            "EXPRESS", (region, after, free) -> free ? 0 : new Fees(45_000, 60_000, 55_000).of(region));

The default behavior is preserved with getOrDefault(..., NONE): an unknown customer type gets no discount, and an unknown shipping method (such as PICKUP) is free. This is exactly the detail the characterization test protects, because the original does not throw on unknown values. If you want to "fix" that by throwing an error, that is a behavior change and belongs in a separate commit after the refactoring is done.

We have to stop and ask directly: is this Strategy worth it? With two variants of each kind, Map.of plus lambdas is little more than a lookup table. It is worth it when variants keep growing or need their own tests; otherwise v1 is already good enough.

Step 3 of the refactoring: Strategy for coupons

In this refactoring, as in most real code, most new business requests are coupons: every campaign wants a new code. That is where the pattern pays off. We split Coupon into an interface with one method that takes a context (Context: channel, cart total, base percentage) and returns an Effect:

/** Strategy 3: each coupon is an object; adding a coupon does not require editing the calculator. */
public interface Coupon {
    /** The coupon's effect on a specific order. */
    record Effect(long percent, long flat, boolean freeShip, boolean capped) {
        static Effect none(long base) {
            return new Effect(base, 0, false, false);
        }
    }

    /** Precomputed context that a coupon can use. */
    record Context(String channel, long subtotal, long basePercent) {}

    Effect apply(Context ctx);
}

The calculator receives a Map<String, Coupon> through its constructor (the same idea as Dependency Injection), so a new coupon can be registered without editing any existing class. The core is now a four-step pipeline, one line per step:

        Effect effect = effectOf(coupon, new Context(channel, subtotal, base));
        long discount = subtotal * effect.percent() / 100;
        if (effect.capped()) discount = Math.min(discount, 500_000);
        long afterDiscount = subtotal - discount - effect.flat();
        long fee = channel.equals("STORE") ? 0 : shippingFee(shipping, region, afterDiscount, effect.freeShip());
        return afterDiscount + afterDiscount * 10 / 100 + fee;
    }

The final test proves what we wanted: adding the GIAM20 coupon takes one registration line in the calling code, with no edits to CouponQuoteCalculator:

    @Test
    void themMaGiam20ChoKhachMoi() {
        Map<String, demo.quote.v3.Coupon> coupons = new HashMap<>();
        coupons.put("GIAM20", ctx -> new Effect(20, 0, false, true));
        CouponQuoteCalculator calc = new CouponQuoteCalculator(coupons);
        // 1,000,000 - 20% = 800,000; VAT 80,000; STANDARD 25,000 because after discount < 1,000,000 (North region)
        long total = calc.total("RETAIL", "WEB", "NORTH", "GIAM20", "STANDARD", List.of(new Line("SKU-3", 1, 1_000_000)));
        System.out.println("GIAM20 on 1,000,000, North region, standard shipping: " + total);
        assertEquals(905_000, total);
    }

The result is in the output above (GIAM20 on 1,000,000, North region, standard shipping: 905000): 1,000,000 minus 20% leaves 800,000, plus 80,000 VAT and the 25,000 standard shipping fee for the North region, for a total of 905,000, exactly matching the hand calculation. This test differs from the characterization test in that it describes new behavior, so it may only be written after the refactoring is complete.

Measuring again: did the refactoring really improve anything

We do not want to flatter ourselves. The metrics.py script counts lines and decision points of the total method in each version:

version      lines in total()  decision points in total()  decision points, whole file
v0 legacy                  73                          29                           29
v1 extract                 11                           2                           21
v2 strategy                11                           3                           16
v3 coupon                  14                           5                           16

After the refactoring, the main method shrinks from 73 lines and 29 decision points to 11 to 14 lines and only a few decision points. But two things need to be said plainly. First, the total decision points of the whole file only drop from 29 to 16: the logic does not disappear, it is just split up and named. Second, from v2 to v3 the decision points in total rise from 3 to 5 because I moved the customer-type selection into a switch right inside the method. That number reflects the real code; it is not a metric to optimize. What we truly gain is local changeability: changing the rule of one coupon now touches only one lambda, and the grid test will flag it immediately if we slip.

There is a cost to this refactoring too: more files, and newcomers must learn Coupon, Context, and Effect. For a business with three rarely changing coupons, v1 is a better balance than v3.

Other patterns that often appear when refactoring

This case study ends at Strategy, but with a different smell the destination would differ. According to Kerievsky's catalog (27 pattern-directed refactorings, 12 design smells), here are a few common pairs:

Smell in the codeRefactoringTarget pattern
An if chain that chooses an algorithm by typeReplace Conditional with PolymorphismStrategy (this article)
Order status drives behavior through scattered switch statementsReplace State-Altering Conditionals with StateState
Several subclasses differ in only one step of the same algorithm skeletonForm Template MethodTemplate Method
Objects created through long constructors with many variantsReplace Constructors with Creation MethodsBuilder / Factory
One action must notify several parties in several stepsReplace Hard-Coded Notifications with ObserverObserver

When not to refactor to a pattern

  • No safety net: if you cannot yet write a characterization test, do not refactor the structure. Start by building the net as in step 0.
  • Code that does not change: an ugly method that has been stable for three years does not need touching; refactoring pays off where the business still changes.
  • More pattern than problem (over-refactoring): two branches do not need Strategy. The rule of three is a reasonable guide: the third time the same kind of change arrives is the time to generalize.
  • Mixing behavior and structure: fixing a business bug in the same commit as the refactoring makes the golden grid useless, because you can no longer tell whether a mismatch came from the refactoring or from the bug fix.

Common mistakes when refactoring

  • A grid that is long but not deep: like the three mutations that escaped the grid above. Always check the boundary of every comparison condition, and if you can, run a mutation testing tool (such as PIT) instead of hand-made mutations.
  • Refactoring too much at once: every step should be small enough that when the grid turns red, you know immediately which change caused it. In the case study, each of v1, v2, and v3 is a separate class and deserves a separate commit.
  • Changing the public signature midway: here QuoteCalculator keeps its signature across all four versions, so callers need no edits. If you must change the interface, use an intermediate step (an adapter) and migrate gradually.
  • The golden master records bugs too: the golden file describes current behavior, bugs included. If you suspect a result is wrong, note it and handle it in a separate bug-fix step; do not quietly "fix" the golden file.

Checklist for refactoring to patterns

  • Identify the concrete pain (which change happens most often?) before thinking about patterns.
  • Build a characterization test with a grid that includes boundary values; check the grid with mutations.
  • Separate structure from behavior: refactor in its own commit, with no bug fixes at the same time.
  • Extract Method first, polymorphism second, in the order refactoring.guru recommends.
  • Keep the public signature unchanged; compare every version on the same grid.
  • Measure the result of the refactoring (lines, decision points) and be honest about the cost: file count, indirection.
  • Stop at the simplest level that solves the pain; the pattern is not the goal.

In short, refactoring to patterns is a discipline, not an art: lock behavior, move in small steps, let the pattern appear where the business really changes, and stop at the right moment. Thanks to the 2,835-combination net, we went from a 73-line method to an extensible pipeline without changing a single cent in the results.

Next article

The final article of the series looks back at a language tool that helped us a great deal in the steps above: immutable record types and sealed classes for modeling data. Continue with Records and sealed classes: immutable data models with Java records.