//Countries should be extracted to a Class or better use EF to keep a contract between valid values in DB and code. //if we think that there will be more country specific rules, we can introduce a new data structure that will keep country name and tax value //so it is easy to extend it w/o a need of modifying this class. public void CreateOrder(Order order) { // 1) potentially float can overflow, there will be a precision lost // float can overflow when we sum it and when we multiply it (in this code) // I would recommend changing it to Decimal // or if we really have to keep float, casting float to decimal first and then performing sum and multiplication. // I changed the way how total is calculated to mitigate it: // old code: order.Total = Convert.ToDecimal(order.Items.Sum(_ => _.Price) * 1.2); // new code: order.Total = order.Items.Select(x => CastToDecimal(x.Price)).Sum() * 1.2M; // 2)Converting float to decimal directly can loose a precision, 125.609375f cast to decimal becomes 125.6094M // old school way to mitigate the risk of precision lost is to first // cast float to string as a "round trip" formatter and then create decimal from that string. // 3) Also... we don't know who is using this method already, and because I am changing the way how total is calculated // calculated total can be different, and it may be problematic for people that used this code in past. // Again there are ways to mitigate it... I can elaborate on it more if asked. if (order.Customer.Country == Countries.AU) { order.Total = order.Items.Select(x => CastToDecimal(x.Price)).Sum() * Countries.AUSalesTax; } else if (order.Customer.Country == Countries.UK) { order.Total = order.Items.Select(x => CastToDecimal(x.Price)).Sum() * Countries.UKSalesTax; } else //specification mentioned that Total should be generated for all orders, it means that also for order that are not in named above countries. { order.Total = order.Items.Select(x => CastToDecimal(x.Price)).Sum(); } //Total is being calculated when we save an order to a DB, but there is no guarantee that some process //will add extra orderItems for a given order directly into DB, and due to that when we get order from DB, total will be incorrect. //I can elaborate on it more if asked. _orderDataContext.CreateOrder(order); }
/// <summary> /// ToDo: Move the calculation logic to Order /// </summary> /// <param name="order"></param> public void CreateOrder(Order order) { if (order.Customer.Country == "AU") { order.Total = Convert.ToDecimal(order.Items.Sum(_ => _.Price) * 1.1); } else if (order.Customer.Country == "UK") { order.Total = Convert.ToDecimal(order.Items.Sum(_ => _.Price) * 1.2); } _context.CreateOrder(order); }