5
\$\begingroup\$

Similar questions have been asked here before and my solution is pretty much in line with some of the answers. I wanted to work on this a bit.

Scenario

Basic sales tax is applicable at a rate of 10% on all goods – except books, food and medical products, which are exempt. Import duty is an additional sales tax applicable on all imported goods at a rate of 5%, with no exemptions. The tax rates or item categories may change in future.

When I purchase items I receive a receipt which lists the name of all the items and their price (including tax), finishing with the total cost of the items, and the total amounts of sales taxes paid. The rounding rules for sales tax are that for a tax rate of n%, a shelf price of p contains (n*p/100 rounded up to the nearest 0.05) amount of sales tax.

I defined a interface IProductTax

public interface IProductTax
{
    bool IsApplicableTo(IProduct product);
    decimal Compute(IProduct product);
}

which each tax class, Sales, Import must implement. IsApplicableTo checks if that tax is valid for the input Product and Compute computes the tax on this product.

The TaxProcessor class takes in a list of taxes during construction and for each product thereby passed it runs the entire tax rules and check if it is valid for the product and then applies it.

public class TaxProcessor : ITaxProcessor
{
    List<IProductTax> SalesTaxes;
    public TaxProcessor()
    {
        SalesTaxes = new List<IProductTax>()
        {
            new SalesTax(),
            new ImportDuty()
        };
    }
    public decimal ComputeTotalTax(IProduct product)
    {
        decimal computedSalesTax = 0;
        foreach (var tax in SalesTaxes)
        {
            computedSalesTax += tax.Compute(product);
        }

        return computedSalesTax;
    }
}

One drawback of this approach is that the single product will pretty much run through the entire set of possible taxes to compute the taxes. I'm not sure how to overcome this in a OO way.

public class SalesTax : IProductTax
{
    public decimal Compute(IProduct product)
    {
        if (IsApplicableTo(product))
            return product.Price * 0.1m;
        return 0m;
    }

    public bool IsApplicableTo(IProduct product)
    {
        return !product.ItemType.HasFlag(ItemTypes.SalesTaxEmptedItemTypes);
    }
}

public class ImportDuty : IProductTax
{
    public decimal Compute(IProduct product)
    {
        if (IsApplicableTo(product))
            return product.Price * 0.05m;
        return 0m;
    }

    public bool IsApplicableTo(IProduct product)
    {
        return product.IsImport;
    }
}

The BillProcessor class return a Receipt DTO after processing the shopping cart

public class BillProcessor
{
    public BillProcessor(ITaxProcessor salesTaxProcess)
    {
        SalesTaxProcessor = salesTaxProcess;
    }

    public Reciept ProcessCart(List<ShoppingItem> shoppingCart)
    {
        var billedShopppingItems = new List<BilledShopppingItem>();
        decimal totalTaxForCart = 0;
        decimal totalBilledAmount = 0;
        foreach (var shoppingItem in shoppingCart)
        {
            decimal individualTax = SalesTaxProcessor.ComputeTotalTax(shoppingItem.Product);
            decimal taxForAllProducts = individualTax * shoppingItem.Quantity;

            decimal totalPrice = shoppingItem.Product.Price * shoppingItem.Quantity;
            decimal totalPriceAfterTax = totalPrice + taxForAllProducts;

            billedShopppingItems.Add(new BilledShopppingItem(shoppingItem, taxForAllProducts,
                totalPriceAfterTax));

            totalTaxForCart += taxForAllProducts;
            totalBilledAmount += totalPriceAfterTax;
        }
        return new Reciept(billedShopppingItems, totalBilledAmount, totalTaxForCart);
    }

    public ITaxProcessor SalesTaxProcessor { get; }
}

And a few DTOs and enum

Shopping Item

public class ShoppingItem
{
    public ShoppingItem(IProduct product, int quantity)
    {
        Product = product;
        Quantity = quantity;
    }

    public IProduct Product { get; }
    public int Quantity { get; }
}

Product

public class Product : IProduct
{
    public Product(string name, decimal price, bool isImport, ItemTypes itemType)
    {
        Name = name;
        Price = price;
        IsImport = isImport;
        ItemType = itemType;
    }

    public string Name { get; }
    public decimal Price { get; }
    public bool IsImport { get; }
    public ItemTypes ItemType { get; }
}

BilledShoppingItem

public class BilledShopppingItem
{
    public BilledShopppingItem(ShoppingItem shoppingItem, decimal tax, decimal totalPrice)
    {
        ShoppingItem = shoppingItem;
        Tax = tax;
        TotalPrice = totalPrice;
    }

    public ShoppingItem ShoppingItem { get; }
    public decimal Tax { get; }
    public decimal TotalPrice { get; }
}

Reciept

public class Reciept
{
    public Reciept(List<BilledShopppingItem> processedShoppingCart, decimal totalBillAmount, decimal totalSalesTax)
    {
        ProcessedShoppingCart = processedShoppingCart;
        TotalBillAmount = totalBillAmount;
        TotalSalesTax = totalSalesTax;
    }

    public void PrintBill()
    {
        foreach (var processedItem in ProcessedShoppingCart)
        {
            Console.WriteLine($"{processedItem.ShoppingItem.Product.Name} { processedItem.TotalPrice }");
        }
        Console.WriteLine($"Sales Taxes {TotalSalesTax}");
        Console.WriteLine($"Total {TotalBillAmount}");
    }
    public List<BilledShopppingItem> ProcessedShoppingCart { get; }
    public decimal TotalBillAmount { get; }
    public decimal TotalSalesTax { get; }
}

Types of products

[Flags]
public enum ItemTypes
{
    None = 0,
    Book = 1 << 0,
    Food = 1 << 1,
    Medical = 1 << 2,
    Others = 1 << 3,
    SalesTaxEmptedItemTypes = Book | Food | Medical
}

I know there access modifiers are pretty liberal and naming conventions isn't the best.

However, I'm looking if this type of solution is extendable especially when we have more products and more and more tax rules.

I'm not very keen on putting isImport inside of Product and that depends on the geography.

The code is also on github if thats easier to read though. https://github.com/benneyman/SalesTax

\$\endgroup\$
  • 1
    \$\begingroup\$ Very clean. One thought, it looks like the IsApplicableTo makes sense only internally so it might be a better idea to make it a protected abstract method of an abstract class ProductTax. I don't think anyone else would need it but the derived types. \$\endgroup\$ – t3chb0t Apr 26 at 6:52
  • \$\begingroup\$ Can you clarify wether it is possible that multiple taxes are applicable for the same product? \$\endgroup\$ – Heslacher Apr 26 at 6:59
  • \$\begingroup\$ Yes, that's totally possible. The Tax processor takes in a list of applicable tax objects. Currently, sales and import duty are being applied for products which are imported. \$\endgroup\$ – thebenman Apr 26 at 7:05
  • 1
    \$\begingroup\$ @t3chb0t That makes sense. However, I was thinking if we need to support operations like given a product find all the applicable taxes. It makes sense to have them as public. \$\endgroup\$ – thebenman Apr 26 at 7:11
  • 1
    \$\begingroup\$ The problem statement says ImportDuty is a SalesTax - and I'm not sure if in future ImportDuty class could have more functionality than SalesTax class. So we can make SalesTax methods virtual while implementing the interface IProductTax and inherit ImportDuty from SalesTax by overriding parent class methods. Also we can make virtual properties for % values like 0.1m and Tax classes can override them (this will be more OO way). This will also make calculation in Compute method more intuitive. \$\endgroup\$ – URB Apr 26 at 10:04
2
\$\begingroup\$

Personally I tend to avoid creating classes, which do not implement any logic. Sometimes it makes sense or is even necessary, but always when I see such a class, I try to find out what operations are performed on its data and if they shouldn't be implemented as methods of that class.

What I don't like about your code is the ProcessCart method, which pretty much contains all the logic and as such needs to be modified virtually every time when any requirement changes. The BilledShopppingItem class could at least calculate the tax itself, getting the tax rate as a constructor parameter, but I would probably go further and pass it the whole ITaxProcessor. Ideally, the only responsibility of the BillProcessor class should to iterate through all the elements and perform only operations requiring knowledge of the whole shopping cart (e.g. the cheapest article is for free, 10% discount if total bill amount > $100 etc.).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.