Super Market Checkout Pricing Strategies

I saw this question and thought it looked like a great opportunity to try my hand at the Strategy Pattern. I've never used it before, but I think I did pretty well. Did I?

The code below can also be found on github. I'm using C# 6.0 for this, so if there are any opportunities to use new features that I missed, I'd like to know.

The Challenge


"...checkout that calculates the total price of a number of items. In
a normal supermarket, things are identified using Stock Keeping Units,
or SKUs. In our store, we’ll use individual letters of the alphabet
(A, B, C, and so on). Our goods are priced individually. In addition,
some items are multipriced: buy n of them, and they’ll cost you y
cents. For example, item ‘A’ might cost 50 cents individually, but
this week we have a special offer: buy three ‘A’s and they’ll cost you
$1.30. In fact this week’s prices are:"

| Item Name | Price | Special Price |
|:-----------|------------:|:------------: |
| A | 50 | 3 for 130 |
| B | 30 | 2 for 45 |
| C | 20 | |
| D | 15 | |



"Our checkout accepts items in any order, so that if we scan a B, an
A, and another B, we’ll recognize the two B’s and price them at 45
(for a total price so far of 95). Because the pricing changes
frequently, we need to be able to pass in a set of pricing rules each
time we start handling a checkout transaction. "


Back To The Checkout

The Approach

• I created a Sku struct for type safety and to allow myself to implement more realistic Skus down the road if I choose to.

• The Cashier is instantiated with a a List of Pricing Strategies.

• The Cashier is then responsible for checking out a list of Skus and returning the total price.

• Each strategy implements a IPricingStrategy interface.

• Two different base classes fell out of my TDD exercise. A "Regular" and a "Sale" strategy. Strategies fo

I'm going to try writing a short answer.
Structs are immutable. Not ifs, no buts.

A private field like this doesn't suggest immutability:

private char _value;

This does:

private readonly char _value;

If you can break encapsulation and do this:

public Sku(Sku sku)
{
    _value = sku._value;
}

Then you can break encapsulation and do that:

public MutateSku(ref Sku sku)
{
    sku._value = 42;
}

Where's the public char Value { get { return _value; } } property getter? I see why you don't need one...

SKU'se me sir, but that's not a SKU.

That Sku struct is essentially... a character. Sure it fulfills the A, B, C sample SKU's from the problem statement, but real-world SKU's could be 10-digit codes... your implementation simply doesn't scale all that well.

Does it make sense to treat any '%', '$' and ',' as a SKU? Absolutely not. There's no gain in using a Sku vs. using a char throughout your code!

I would have approached the data structure differently. First of all, I would have created a Product class, something like this:

public class Product
{
    public Product(string sku)
    {
        _sku = sku;
    }

    private readonly string _sku;
    public string Sku { get { return _sku; } }

    /* a real-life Product class would have other properties... */
    // private readonly string _description;
    // private readonly int _supplierId;

    /* Equals/GetHashCode based on _sku, and type being Product */
}

Then go on with ProductPricing strategies, without allowing just about any string out there to be treated as a Product.

Double is no money.

The correct type to use to talk cash, is decimal. By using double, you've exposed yourself to floating-point rounding errors, which is not cool for something granular like a SKU. I'm pretty sure at least one of your tests break if prices start involving non-integer amounts... and if not, then there's a false sense of accuracy going on here - using double for money is going to end up causing rounding errors one way or another, one day or another.

StackExchange Code Review Q#94213, answer score: 6