Refactoring exercise for Olo

OrderSummary.cs

using System;
using System.Collections.Generic;
using System.Text;

namespace Refactoring
{
    class Program
    {
        static void Main(string[] args)
        {
            // This is a cleaner way to write the same code. It requires more boilerplate
            // OO and Design Patterns principles implementation but it's safer, cleaner and better organized.
            // It took me around 35 minutes plus 10 more minutes debugging and cleaning up.
            var newOrder = new Order("John Doe")
                            .WithProduct(Product.CreateProduct("Pulled Pork", 6.99m, 0.5m, PricingMethod.PerPound))
                            .WithProduct(Product.CreateProduct("Coke", 3m, 2, PricingMethod.PerItem))
                            .WithProduct(Product.CreateProduct("Candy", 0.1m, 2, PricingMethod.None));

            Console.WriteLine(newOrder.ToString());
            Console.WriteLine("Total Price: $" + newOrder.OrderTotal);
            Console.ReadKey();
        }
    }


    // Instead of having wild free texts (prone to typos = bug) we should convert this to an enum.
    public enum PricingMethod
    {
        PerPound,
        PerItem,
        None
    }

    #region PricingStrategy
    // Reason for refactoring
    // Since we have very specific algorithms for these calculations
    // we can use the Strategy pattern to inject them into our models.
    // This way we can add more strategies in the future or actually
    // change the algorithm for any product at runtime in case we decide to 
    // give away free Coke.
    public interface IPricingStrategy
    {
        decimal CalculatePrice(Product product);
        string FormatPricingStrategySummary(Product product);
    }

    // Here I decided to use the same interface for the Formatting part
    // I could've also used a different interface for this and add it.
    // Ex.: PerPoundPriceCalculationFormatting: IPricingStrategy, IPricingFormatting
    // This would achieve the same. This is ideal if you want to keep functionality very well defined.
    // Better to inject, also helps achieve (S)OL(I)D principles.
    public class PerPoundPriceStrategy : IPricingStrategy
    {
        public decimal CalculatePrice(Product product)
        {
            return (product.Weight.Value * product.Price);
        }

        public string FormatPricingStrategySummary(Product product)
        {
            return $"{product.ProductName} {product.CalculatePrice():C2} ({product.Weight } pounds at ${product.Price } per pound)";
        }
    }

    // This is an example of extending pricing algorithms without modifying any other code.
    // It also uses the latest flavors for the language. Lambda like function body.
    public class FreeProductPriceStrategy : IPricingStrategy
    {
        public decimal CalculatePrice(Product product) => 0m;

        public string FormatPricingStrategySummary(Product product) => $"{product.ProductName} {product.CalculatePrice():C2} (FREE like in \"FREE CANDY\"!)";
    }

    public class PerItemPriceStrategy : IPricingStrategy
    {
        public decimal CalculatePrice(Product product)
        {
            return (product.Quantity.Value * product.Price);
        }

        public string FormatPricingStrategySummary(Product product)
        {
            return $"{product.ProductName} {product.CalculatePrice():C2} ({product.Quantity } items at ${product.Price } each)";
        }
    }

    #endregion


    // I decided to replace the Tuple for an actual class so that I can encapsulate
    // extra functionality that belongs to this class specifically.
    // I decided also to override ToString() but you can actually have a similar method 
    // that returns a string.
    // The only responsibility of this class is to store the Order items 
    // and calculate the total for the order.
    public class Order
    {
        public string CustomerName { get; private set; }
        public List<Product> Products { get; private set; }
        public decimal OrderTotal => CalculateOrderTotal();

        public Order(string customerName)
        {
            CustomerName = customerName;
            Products = new List<Product>();
        }

        public Order WithProduct(Product product)
        {
            AddProduct(product);
            return this;
        }

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

        private decimal CalculateOrderTotal()
        {
            var orderTotal = 0m;

            Products.ForEach(item =>
            {
                orderTotal += item.CalculatePrice();
            });

            return orderTotal;
        }

        public override string ToString()
        {
            StringBuilder sb = new StringBuilder();
            sb.AppendLine($"ORDER SUMMARY FOR {CustomerName}:");

            Products.ForEach(item =>
            {
                sb.AppendLine(item.ToString());
            });

            return sb.ToString();
        }
    }


    // I am not too happy with having the Quantity property in this model.
    // I would've created another object where I specify the quantity and the Product.
    // Also, these properties, the public ones, should be {get; private set;} to avoid 
    // client code modifying the values at will. Values should be only modifiable via a method.
    // Again... it was only 30 minutes.
    // Price, Weight and Quantity are perfect candidates for ValueObjects. 
    public class Product
    {
        public string ProductName;
        public decimal Price;
        public decimal? Weight;
        public int? Quantity;
        public PricingMethod PricingMethod;
        private IPricingStrategy pricingCalculator;


        // I set the constructor private because I want to be in control of how my entity
        // is created. Obviously this is useless if I don't set the properties to { private set;}
        // The idea is to avoid modifications to our members directly.
        private Product(string productName,
            decimal price,
            decimal? weight,
            int? quantity,
            PricingMethod pricingMethod,
            IPricingStrategy pricingStrategy)
        {
            ProductName = productName;
            Price = price;
            Weight = weight;
            Quantity = quantity;
            PricingMethod = pricingMethod;
            pricingCalculator = pricingStrategy;
        }

        // This can be a class separately. I only had 30 minutes, so... Improvise!
        // The reason for these overloads is to simplify the creation process, make it cleaner.
        // It reduces the chances of someone entering weight and quantity (which seem to be the delta)
        // for an unique product.
        #region Factories

        public static Product CreateProduct(
            string productName,
            decimal price,
            int? quantity,
            PricingMethod pricingMethod)
        {
            return CreateProduct(productName, price, null, quantity, pricingMethod);
        }

        public static Product CreateProduct(
            string productName,
            decimal price,
            decimal? weight,
            PricingMethod pricingMethod)
        {
            return CreateProduct(productName, price, weight, null, pricingMethod);
        }

        private static Product CreateProduct(
            string productName,
            decimal price,
            decimal? weight,
            int? quantity,
            PricingMethod pricingMethod)
        {
            var pricingStrategy = PricingStrategyProvider.GetPricingStrategy(pricingMethod);
            return new Product(productName, price, weight, quantity, pricingMethod, pricingStrategy);
        }
        #endregion

        // Here I am delegating to the algorithm the responsibility to make the calculation
        // In case of dynamically passing Strategies into the class at runtime, we can add
        // the Pricing Strategy interface as an argument here. 
        // Ex.: CalculatePrice(IPricingStrategy strategy)
        // Use cases for both approaches are; first, if the customer buys more than 4 sandwiches, we throw in
        // some fries for free. This decision can only be made at runtime and it would work
        // with the current implementation. If the decision is based on offering discounts on 
        // products that are already in the order, then you need to use the argument way. Obviously there
        // are many ways to tackle this issue.
        public decimal CalculatePrice()
        {
            return pricingCalculator.CalculatePrice(this);
        }

        // I am using the overloading of ToString() because the summary
        // sounds to me like a good 'justification' for ToString(). 
        public override string ToString()
        {
            return pricingCalculator.FormatPricingStrategySummary(this);
        }
    }

    // I don't like this approach since you tighly couple the Factory (living in the model)
    // to PricingStrategyProvider. However, this can be easily replaced with Dependency Injection.
    public class PricingStrategyProvider
    {
        public static IPricingStrategy GetPricingStrategy(PricingMethod pricingMethod)
        {
            switch (pricingMethod)
            {
                case PricingMethod.PerPound:
                    return new PerPoundPriceStrategy();
                case PricingMethod.PerItem:
                    return new PerItemPriceStrategy();
                default:
                    return new FreeProductPriceStrategy();
            }
        }
    }
}