Refactoring: Using List<>.AsReadOnly() instead of HashSet<>.ToList() that is more expensive. Related to issues: https://github.com/dotnet/eShopOnContainers/issues/29 and

src/Services/Basket/Basket.API/Controllers/BasketController.cs

@@ -8,7 +8,7 @@ using Microsoft.AspNetCore.Authorization;
 
 namespace Microsoft.eShopOnContainers.Services.Basket.API.Controllers
 {
-    //NOTE: Right now this is a very chunky API, as the app evolves it is possible we would
+    //TODO NOTE: Right now this is a very chunky API, as the app evolves it is possible we would
     //want to make the actions more fine graned, add basket item as an action for example.
     //If this is the case we should also investigate changing the serialization format used for Redis,
     //using a HashSet instead of a simple string.

src/Services/Ordering/Ordering.Domain/AggregatesModel/BuyerAggregate/Buyer.cs

@@ -10,9 +10,9 @@ namespace Microsoft.eShopOnContainers.Services.Ordering.Domain.AggregatesModel.B
     {
         public string FullName { get; private set; }
 
-        private HashSet<PaymentMethod> _paymentMethods;
+        private List<PaymentMethod> _paymentMethods;
 
-        public IEnumerable<PaymentMethod> PaymentMethods => _paymentMethods?.ToList().AsReadOnly();
+        public IEnumerable<PaymentMethod> PaymentMethods => _paymentMethods?.AsReadOnly();
 
         protected Buyer() { }
 
@@ -25,7 +25,7 @@ namespace Microsoft.eShopOnContainers.Services.Ordering.Domain.AggregatesModel.B
 
             FullName = identity;
 
-            _paymentMethods = new HashSet<PaymentMethod>();
+            _paymentMethods = new List<PaymentMethod>();
         }
 
         public PaymentMethod AddPaymentMethod(int cardTypeId, string alias, string cardNumber, string securityNumber, string cardHolderName, DateTime expiration)

src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/Order.cs

@@ -31,15 +31,12 @@ namespace Microsoft.eShopOnContainers.Services.Ordering.Domain.AggregatesModel.O
         // 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<OrderItem> _orderItems;
+        private readonly List<OrderItem> _orderItems;
 
-        //TODO: Use List<> instead of HashSet<> because of the comment below
-        // So we won't need the ".ToList()"
-
-        public IEnumerable<OrderItem> OrderItems => _orderItems.ToList().AsReadOnly();
+        public IEnumerable<OrderItem> OrderItems => _orderItems.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)
+        // This will create a read only wrapper around the private list so is protected against "external updates".
+        // 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; }
@@ -60,7 +57,7 @@ namespace Microsoft.eShopOnContainers.Services.Ordering.Domain.AggregatesModel.O
             _country = address.Country;
             _zipCode = address.ZipCode;
 
-            _orderItems = new HashSet<OrderItem>();
+            _orderItems = new List<OrderItem>();
         }
 
         // DDD Patterns comment