From 4708b894578f48c879f3dcae4e4cc70a289b3821 Mon Sep 17 00:00:00 2001 From: CESARDL Date: Thu, 2 Feb 2017 13:34:44 -0800 Subject: [PATCH] Partial refactoring about Order and OrderItems. Set the fields to explicit private Using .ToReadOnly(), but still might need to use List<> instead HashSet<> so we won't need the .ToList() conversion which is "expensive".. --- .../AggregatesModel/BuyerAggregate/Buyer.cs | 2 +- .../AggregatesModel/OrderAggregate/Order.cs | 34 +++++++++++++++---- .../OrderAggregate/OrderItem.cs | 12 ++++--- .../OrderingContext.cs | 3 +- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/Services/Ordering/Ordering.Domain/AggregatesModel/BuyerAggregate/Buyer.cs b/src/Services/Ordering/Ordering.Domain/AggregatesModel/BuyerAggregate/Buyer.cs index ce09978c2..21b802041 100644 --- a/src/Services/Ordering/Ordering.Domain/AggregatesModel/BuyerAggregate/Buyer.cs +++ b/src/Services/Ordering/Ordering.Domain/AggregatesModel/BuyerAggregate/Buyer.cs @@ -12,7 +12,7 @@ namespace Microsoft.eShopOnContainers.Services.Ordering.Domain.AggregatesModel.B private HashSet _paymentMethods; - public IEnumerable PaymentMethods => _paymentMethods?.ToList().AsEnumerable(); + public IEnumerable PaymentMethods => _paymentMethods?.ToList().AsReadOnly(); protected Buyer() { } diff --git a/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/Order.cs b/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/Order.cs index 692ce59cd..e7c893b46 100644 --- a/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/Order.cs +++ b/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/Order.cs @@ -9,24 +9,41 @@ namespace Microsoft.eShopOnContainers.Services.Ordering.Domain.AggregatesModel.O public class Order : Entity { + // DDD Patterns comment + // Using private fields, allowed since EF Core 1.1, is a much better encapsulation + // aligned with DDD Aggregates and Domain Entities (Instead of properties and property collections) + private DateTime _orderDate; + + //TO DO: These fields need to be converted to a VALUE-OBJECT "Address" private string _street; private string _city; private string _state; private string _country; private string _zipCode; - private DateTime _orderDate; public Buyer Buyer { get; private set; } - int _buyerId; + private int _buyerId; public OrderStatus OrderStatus { get; private set; } - int _orderStatusId; + private int _orderStatusId; - HashSet _orderItems; - public IEnumerable OrderItems => _orderItems.ToList().AsEnumerable(); + // DDD Patterns comment + // Using a private collection field, better for DDD Aggregate's encapsulation + // so OrderItems cannot be added from "outside the AggregateRoot" directly to the collection, + // but only through the method OrderAggrergateRoot.AddOrderItem() which includes behaviour. + private readonly HashSet _orderItems; + + //TODO: Use List<> instead of HashSet<> because of the comment below + // So we won't need the ".ToList()" + + public IEnumerable OrderItems => _orderItems.ToList().AsReadOnly(); + // Using List<>.AsReadOnly() + //This will create a read only wrapper around the private list. + //It's much cheaper than .ToList() because it will not have to copy all items in a new collection. (Just one heap alloc for the wrapper instance) + //https://msdn.microsoft.com/en-us/library/e78dcd75(v=vs.110).aspx public PaymentMethod PaymentMethod { get; private set; } - int _paymentMethodId; + private int _paymentMethodId; protected Order() { } @@ -46,7 +63,10 @@ namespace Microsoft.eShopOnContainers.Services.Ordering.Domain.AggregatesModel.O _orderItems = new HashSet(); } - + // DDD Patterns comment + // This Order AggregateRoot's method "AddOrderitem()" should be the only way to add Items to the Order, + // so any behavior (discounts, etc.) and validations are controlled by the AggregateRoot + // in order to maintain consistency between the whole Aggregate. public void AddOrderItem(int productId, string productName, decimal unitPrice, decimal discount, string pictureUrl, int units = 1) { var existingOrderForProduct = _orderItems.Where(o => o.ProductId == productId) diff --git a/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/OrderItem.cs b/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/OrderItem.cs index 0b5fe1f72..22c7cc93c 100644 --- a/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/OrderItem.cs +++ b/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/OrderItem.cs @@ -6,16 +6,18 @@ namespace Microsoft.eShopOnContainers.Services.Ordering.Domain.AggregatesModel.O public class OrderItem : Entity { - private string _productName; - private string _pictureUrl; - private int _orderId; + // DDD Patterns comment + // Using private fields, allowed since EF Core 1.1, is a much better encapsulation + // aligned with DDD Aggregates and Domain Entities (Instead of properties and property collections) + private string _productName; + private string _pictureUrl; + private int _orderId; private decimal _unitPrice; private decimal _discount; - private int _units; + private int _units; public int ProductId { get; private set; } - protected OrderItem() { } public OrderItem(int productId, string productName, decimal unitPrice, decimal discount, string PictureUrl, int units = 1) diff --git a/src/Services/Ordering/Ordering.Infrastructure/OrderingContext.cs b/src/Services/Ordering/Ordering.Infrastructure/OrderingContext.cs index d2ed4efed..36cc33147 100644 --- a/src/Services/Ordering/Ordering.Infrastructure/OrderingContext.cs +++ b/src/Services/Ordering/Ordering.Infrastructure/OrderingContext.cs @@ -119,7 +119,8 @@ namespace Microsoft.eShopOnContainers.Services.Ordering.Infrastructure orderConfiguration.Property("PaymentMethodId").IsRequired(); var navigation = orderConfiguration.Metadata.FindNavigation(nameof(Order.OrderItems)); - + // DDD Patterns comment: + //Set as Field (New since EF 1.1) to access the OrderItem collection property through its field navigation.SetPropertyAccessMode(PropertyAccessMode.Field); orderConfiguration.HasOne(o => o.PaymentMethod)