C# Shopping Cart – Calculating Total Price of Items

cdesignobject-orientedobject-oriented-design

I am making a shopping cart. A shopping cart will have a total amount of money you've got to pay for the products you've selected.

I'd like to approach the problem using both OOP (encapsulating it) and more anaemically or functionally (without encapsulation).

I'm using the simplest Product model:

public class Product
{
    public string Name { get; set; }
    public decimal Price { get; set; }
}

1. Using encapsulation (aka rich-domain modeling)

TotalPrice can only be calculated inside the shopping cart class.

a) On the fly calculation

public class ShoppingCart
{
    private List<Product> _products = new List<Product>();

    public decimal TotalPrice { get; private set; }
    public IReadOnlyCollection<Product> Products => _products;

    public void AddProduct(Product product)
    {
        _products.Add(product);
        TotalPrice += product.Price;
    }

    public void RemoveProduct(Product product)
    {
        _products.Remove(product);
        TotalPrice -= product.Price;
    }
}

b) On the fly calculation with RecalculateTotalPrice method

Seems less performant than approach above or using computed property. We're executing a loop on each Add and Remove.

public class ShoppingCart
{
    private List<Product> _products = new List<Product>();
    public decimal TotalPrice { get; private set; }

    public void AddProduct(Product product)
    {
        _products.Add(product);
        RecalculateTotalPrice();
    }

    public void RemoveProduct(Product product)
    {
        _products.Remove(product);
        RecalculateTotalPrice();
    }

    private void RecalculateTotalPrice()
    {
        var totalPrice = 0m;
        foreach (var product in _products)
        {
            totalPrice += product.Price;
        }
        TotalPrice = totalPrice;
    }
}

c) Computed property

Calculated only when accessed TotalPrice. AddProduct and RemoveProduct don't calculate. The disadvantage is we can't easily persist this with some ORM. 

public class ShoppingCart
{
    private List<Product> _products = new List<Product>();
    public decimal TotalPrice => CalculateTotalPrice();

    public void AddProduct(Product product)
    {
        _products.Add(product);
    }

    public void RemoveProduct(Product product)
    {
        _products.Remove(product);
    }

    private decimal CalculateTotalPrice()
    {
        var totalPrice = 0m;
        foreach (var product in _products)
        {
            totalPrice += product.Price;
        }
        return totalPrice;
    }
}

2. No encapsulation

Our TotalPrice will be calculated outside the class.

public class ShoppingCart
{
    public ICollection<Product> Products { get; set; } = new List<Product>();
    public decimal TotalPrice { get; set; }
}

Calculation of TotalPrice can happen from anywhere. The advantage of this is we can decouple calculation from the shopping cart and even have special calculator class for it that can be extended with custom calculation rules and so on. Some interface like IShoppingCartCalculator.

Currently, I'm using the 1a approach but I was wondering if making it a POCO would result in more extendable and maintainable code managed from the outside.

No encapsulation results in simpler code that is easier to follow. There's 1 less business layer to maintain. We're losing benefits of encapsulation but we're gaining more possibilities and we're more resistant to changes

So our pipeline works like (request → handler → invoking some calculator service method) instead of (request → handler → invoking some ShoppingCart method)

Should I give up on encapsulation to gain more possibilities and more re-usable classes or even functions in case of functional programming?

Is encapsulation a thing of the past that didn't meet expectations of reality?

I'm open to any suggestions.

Best Answer

Unless we're talking about tens of thousands of products, recalculating the total price every time is not worth considering in terms of performance. My guess is in a typical shopping list, you're rarely going to see more than 10 items. However, if that's not your case, feel free to write me in the comments and I'll reevaluate this.

Once performance is out of the picture, something which does need to be taken into consideration is writing code in such a way that it doesn't potentially leave holes. By this I mean your 1a solution. If you were to add a product, then change the price afterwards, the total price would be inconsistent. Of course, you may very well not ever do this in your program, but you're asking which method is best. Writing code which allows these sorts of problems is not best.

Your 1b solution is at least consistent, but recalculating each and everytime is unnecessary. Ideally for 100 products added to the list, you only really need to calculate it once. Again, performance is not an issue, but lets not make unnecessary calculations if we can avoid it.

Your 2 solution without encapsulation means you would necessarily have to port this logic elsewhere, and it seems a very natural thing that this logic should be present in the container itself, at least if you insist on having a TotalPrice property.

I would lean towards your 1c solution. The fact that it may cause problems with ORM is a technical oversight to an otherwise excellent solution. Most ORMs provide a means to disregard certain properties, so I would be surprised if there weren't something similar in your case. Also considering that the need for an AddProduct and RemoveProduct are unnecessary, I would eliminate these from the class. If you later needed to know if products were added or removed, you could use an observable list to hold the products.

If performance were an issue, you could always lazily calculate the total price with an internal boolean to indicate whether or not the current total is possibly dirty. When products are added or removed, the dirty flag is set and you know next call to TotalPrice should require recalculation.

Related Topic