Open/Closed Principle

Title: Adding One Shipping Option Broke International Orders for an Hour

Marketing wanted a “same-day delivery” option for Black Friday weekend. Easy request. I opened our ShippingCalculator, added one more else-if, and deployed.

An hour later, support told us international customers were being charged domestic shipping.

I had put my new branch in the wrong place in the chain. One new feature broke a feature that had been working for two years.

What Open/Closed really means

The Open/Closed Principle says software should be open for extension but closed for modification. In plain words: you should be able to add new behavior without editing code that already works.

Here is the code that caused the problem:

public class ShippingCalculator
{
    public decimal Calculate(string method, Order order)
    {
        if (method == "standard")
            return 5m;
        else if (method == "express")
            return 15m;
        else if (method == "international")
            return 25m + order.WeightKg * 4m;
        else
            throw new ArgumentException($"Unknown method: {method}");
    }
}

Every new shipping method means opening this class and changing tested logic. Every edit risks breaking the branches you didn’t mean to touch. The class is open for modification and closed for extension, which is exactly backwards.

The fix: one class per rule

Step 1: define a contract that every shipping method follows.

public interface IShippingStrategy
{
    string Method { get; }
    decimal Calculate(Order order);
}

Step 2: move each branch into its own class, unchanged.

public class StandardShipping : IShippingStrategy
{
    public string Method => "standard";
    public decimal Calculate(Order order) => 5m;
}

public class ExpressShipping : IShippingStrategy
{
    public string Method => "express";
    public decimal Calculate(Order order) => 15m;
}

public class InternationalShipping : IShippingStrategy
{
    public string Method => "international";
    public decimal Calculate(Order order) => 25m + order.WeightKg * 4m;
}

Step 3: the calculator no longer knows any shipping method by name.

public class ShippingCalculator
{
    private readonly IReadOnlyDictionary<string, IShippingStrategy> _strategies;

    public ShippingCalculator(IEnumerable<IShippingStrategy> strategies)
    {
        _strategies = strategies.ToDictionary(s => s.Method);
    }

    public decimal Calculate(string method, Order order)
    {
        if (!_strategies.TryGetValue(method, out var strategy))
            throw new ArgumentException($"Unknown method: {method}");

        return strategy.Calculate(order);
    }
}

How the calculator gets its strategies

You might wonder where that IEnumerable comes from. The DI container supplies it. You register every implementation under the same interface:

builder.Services.AddScoped<IShippingStrategy, StandardShipping>();
builder.Services.AddScoped<IShippingStrategy, ExpressShipping>();
builder.Services.AddScoped<IShippingStrategy, InternationalShipping>();
builder.Services.AddScoped<ShippingCalculator>();

When something needs a ShippingCalculator, the container sees the IEnumerable parameter and passes in all three registered strategies.

Now add same-day delivery

public class SameDayShipping : IShippingStrategy
{
    public string Method => "same-day";
    public decimal Calculate(Order order) => 30m;
}

builder.Services.AddScoped<IShippingStrategy, SameDayShipping>();

ShippingCalculator did not change. The standard, express, and international code did not change. There is nothing existing that this new feature can break, and the new class gets its own small unit test.

Bonus: two bugs we got for free

An unknown method now throws instead of silently falling through. And ToDictionary throws at startup if two strategies share a Method name, so a copy-paste mistake fails immediately and not in production.

When not to do this

Don’t create strategies for two branches that never change. Wait until the second or third variation shows up, so you know what the real axis of change is. Abstracting too early gives you classes nobody needed.

The lesson

If adding a feature means editing a long if-else or switch, that’s the signal. Put each variation in its own class behind an interface, and let new behavior arrive as new code, not edits to old code.

Comments

Leave a Reply

Your email address will not be published. Required fields are marked *