1

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.

Improve this question

3 Answers 3

Reset to default
7

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.

Improve this answer
1
  • 3
    1c also supports business rules that determine price or discount based on the cart rather than the product (e.g., "10% off if your entire order is > 150") Commented Oct 16, 2018 at 13:01
3

You are re-implementing ICollection too much here.

d) Get-only Properties

public class ShoppingCart { public ICollection<Product> Products { get; } = new List<Product>(); public decimal TotalPrice => Products.Sum(p => p.Price); }

Your ORM should be able to populate the Products collection fine. You don't need to be able to reassign it.

Improve this answer
-2

Your example with encapsulation/oop needs more encapsulation/oop.

I obviously don't know your use-case in detail, but an OOP approach would rarely actually return a TotalPrice from a ShoppingCart. Returning data is not really oop-like.

Also, rich-domain doesn't mean that you have some additional methods on the objects, it means that you have business methods on the objects. Business methods means actions that are directly derived from the requirements, from the "Ubiquitous language".

So instead of TotalPrice, the ShoppingCart should have a Checkout(). That is what we need the price for anyway, and we shouldn't ask the object for data, we should tell it what to do.

About whether encapsulation is a thing of the past (opinionated): I think it hasn't been dominant at all yet. Having data-only "objects" (aka. records or structs) only seems more flexible. It seems flexible now, because you don't yet have to think about how it will be used. The consequence of which is that it will be used somehow all over the place. The consequence of which in turn is that it will be extremely hard to change it later, since everything will be strongly coupled to it.

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.