5
\$\begingroup\$

The solution was to be assessed, not just on completing the task but also on code quality and design, presence of testing, robustness, maintainability etc.

Problem Statement

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.

Samples:

  • Case 1:
    • Input:
      • 1 book at 12.49
      • 1 music CD at 14.99
      • 1 chocolate bar at 0.85
    • Output:
      • 1 book: $12.49
      • 1 music CD: $16.49
      • 1 chocolate bar: $0.85
      • Sales Taxes: $1.50
      • Total: $29.83

  • Case 2:
    • Input:
      • 1 imported box of chocolates at 10.00
      • 1 imported bottle of perfume at 47.50
    • Output:
      • 1 imported box of chocolates: 10.50
      • 1 imported bottle of perfume: 54.65
      • Sales Taxes: 7.65
      • Total: 65.15

  • Case 3:
    • Input:
      • 1 imported bottle of perfume at 27.99
      • 1 bottle of perfume at 18.99
      • 1 packet of headache pills at 9.75
      • 1 box of imported chocolates at 11.25
    • Output:
      • 1 imported bottle of perfume: 32.19
      • 1 bottle of perfume: 20.89
      • 1 packet of headache pills: 9.75
      • 1 imported box of chocolates: 11.85
      • Sales Taxes: 6.70
      • Total: 74.68

Here's what I submitted:

using System;
using System.Collections.Generic;

class Program
{
    static void Main(string[] args)
    {
        try
        {
            double totalBill = 0, salesTax = 0;
            List<Product> productList = getProductList();
            foreach (Product prod in productList)
            {
                double tax = prod.ComputeSalesTax();
                salesTax += tax;
                totalBill += tax + (prod.Quantity * prod.ProductPrice);
                Console.WriteLine(string.Format("Item = {0} : Quantity = {1} : Price = {2} : Tax = {3}", prod.ProductName, prod.Quantity, prod.ProductPrice + tax, tax));
            }
            Console.WriteLine("Total Tax : " + salesTax);
            Console.WriteLine("Total Bill : " + totalBill);
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }
        Console.ReadLine();
    }

    private static List<Product> getProductList()
    {
        List<Product> lstProducts = new List<Product>();
        //input 1
        lstProducts.Add(new Product("Book", 12.49, 1, ProductType.ExemptedProduct, false));
        lstProducts.Add(new Product("Music CD", 14.99, 1, ProductType.TaxPaidProduct, false));
        lstProducts.Add(new Product("Chocolate Bar", .85, 1, ProductType.ExemptedProduct, false));

        //input 2
        //lstProducts.Add(new Product("Imported Chocolate", 10, 1, ProductType.ExemptedProduct,true));
        //lstProducts.Add(new Product("Imported Perfume", 47.50, 1, ProductType.TaxPaidProduct,true));

        //input 3
        //lstProducts.Add(new Product("Imported Perfume", 27.99, 1, ProductType.TaxPaidProduct,true));
        //lstProducts.Add(new Product("Perfume", 18.99, 1, ProductType.TaxPaidProduct,false));
        //lstProducts.Add(new Product("Headache Pills", 9.75, 1, ProductType.ExemptedProduct,false));
        //lstProducts.Add(new Product("Imported Chocolate", 11.25, 1, ProductType.ExemptedProduct,true));
        return lstProducts;
    }
}

public enum ProductType
{
    ExemptedProduct = 1,
    TaxPaidProduct = 2,
    //ImportedProduct=3
}


class Product
{
    private ProductType _typeOfProduct = ProductType.TaxPaidProduct;
    private string _productName = string.Empty;
    private double _productPrice;
    private int _quantity;
    private bool _isImportedProduct = false;
    public string ProductName { get { return _productName; } }
    public double ProductPrice { get { return _productPrice; } }
    public int Quantity { get { return _quantity; } }



    public Product(string productName, double productPrice, int quantity, ProductType type, bool isImportedProduct)
    {
        _productName = productName;
        _productPrice = productPrice;
        _quantity = quantity;
        _typeOfProduct = type;
        _isImportedProduct = isImportedProduct;
    }

    public double ComputeSalesTax()
    {
        double tax = 0;
        if (_isImportedProduct) //charge 5% tax directly
            tax += _productPrice * .05;
        switch (_typeOfProduct)
        {
            case ProductType.ExemptedProduct: break;
            case ProductType.TaxPaidProduct:
                tax += _productPrice * .10;
                break;
        }
        return Math.Round(tax, 2);
        //round result before returning
    }
}
\$\endgroup\$
  • \$\begingroup\$ also on code quality and design, presence of testing, So, where are your tests? \$\endgroup\$ – Mast Jun 26 '15 at 14:03
  • \$\begingroup\$ @Mast-Test I have a seperate but as I got review on my main code so i pasted just main part. \$\endgroup\$ – vish1990 Jun 26 '15 at 14:04
  • \$\begingroup\$ Your code fails on the second and third baskets of goods and your ouput doesn't match the format of the problem statement - that's 2 reasons for rejection without even having to look at the code. \$\endgroup\$ – RobH Jun 26 '15 at 14:43
  • \$\begingroup\$ Thnks I will chek the logic but Is it a bad design.I am told that it is a bad design and should learn more to make it Modular and reusable. \$\endgroup\$ – vish1990 Jun 26 '15 at 14:47
  • \$\begingroup\$ Please see the complete implementation to below link: github.com/gmershad/Sales-Tax \$\endgroup\$ – Gul Md Ershad Oct 10 '17 at 12:02
8
\$\begingroup\$

I think part of the problem was that your object model didn't implement the proper Separation of Concerns. Each product object was responsible for knowing whether it was imported, and if it was imported what the tax rate was. Plus it knew whether or not it was tax exempt. What if medical supplies become taxable in the future? You would need to change every line of code that instantiates a medical supply. Yikes.

This also introduces the need for multiple ifs and switches in your code to determine if something is imported or taxable. By utilizing Domain Driven Design you can largely eliminate both problems, though you introduce more layers to your object model (related: Crafting Wicked Domain Models).

I think a more appropriate object model would be:

  • Product: Basic information about a product, including:
    • Name
    • Price
    • Product type
    • Product origin (local or imported)
  • ProductType
    • Name
    • Tax rate
  • ProductOrigin
    • Name
    • Tax rate
  • TaxRate
    • Name
    • Rate
  • InvoiceItem
    • Product
    • Quantity
    • TotalPrice
    • TotalTax
    • TotalAmount

First, let's explore the Product class.

Product class

It needs a name, a price, where it was manufactured (its origin) and its type. Furthermore, you need to calculate the tax on the product, which could be a combination of a sales tax and import duty, but not necessarily both. It could also be one or the other.

public class Product
{
    public string Name { get; set; }
    public double Price { get; set; }
    public ProductOrigin Origin { get; private set; }
    public ProductType Type { get; private set; }

    public Product(ProductType type, ProductOrigin origin)
    {
        Type = type;
        Origin = origin;
    }

    public Product(ProductType type, ProductOrigin origin, string name, double price)
        : this(type)
    {
        Name = name;
        Price = price;
    }

    public double CalculateTax()
    {
        return Type.CalculateTax(Price) + Origin.CalculateTax(Price);
    }
}

The CalculateTax method would delegate to the ProductType and ProductOrigin classes to determine the real tax. Books are tax exempt, but if they are imported books, they still have an import duty. To create a Book Product:

var localBook = new Product(ProductType.Book,
                            ProductOrigin.Local,
                            "How to do something",
                            20.99);

Now, an imported book:

var importedBook = new Product(ProductType.Book,
                               ProductOrigin.Imported,
                               "Something Else",
                               20.99);

The tax on each item will be different:

localBook.CalculateTax();    -> 20.99 * 0.00 = 0
importedBook.CalculateTax(); -> 20.99 * 0.05 = 1.0495

We saw in the Product.CalculateTax() method that we delegate this to the ProductType and ProductOrigin classes, so let's dive a level deeper and explore that.

ProductType class

The ProductType class is responsible for knowing what its tax rate is. Plus, we don't want people to create arbitrary product types. We can limit this by creating a class with a private constructor, and providing public static readonly properties for the available product types:

public class ProductType
{
    public static readonly ProductType Beauty = new ProductType("Beauty", TaxRate.SalesTax);
    public static readonly ProductType Book = new ProductType("Book", TaxRate.Exempt);
    public static readonly ProductType Entertainment = new ProductType("Entertainment", TaxRate.SalesTax);
    public static readonly ProductType Food = new ProductType("Food", TaxRate.Exempt);
    public static readonly ProductType Medical = new ProductType("Medical", TaxRate.Exempt);

    public string Name { get; private set; }
    public TaxRate TaxRate { get; private set; }

    private ProductType(string name, TaxRate taxRate)
    {
        Name = name;
        TaxRate = taxRate;
    }

    public double CalculateTax(double price)
    {
        return TaxRate.CalculateTax(price);
    }
}

The available types are:

  • Beauty
  • Book
  • Food
  • Medical

We know that books, food and medical are tax exempt, so we pass the TaxRate.Exempt object in the constructor. Also notice that the ProductType class doesn't even calculate the tax. It delegates this to the TaxRate class.

The ProductOrigin class looks very similar.

ProductOrigin class

public class ProductOrigin
{
    public static readonly ProductOrigin Local = new ProductOrigin("Local", TaxRate.Exempt);
    public static readonly ProductOrigin Imported = new ProductOrigin("Imported", TaxRate.ImportDuty);

    public string Name { get; private set; }
    public TaxRate TaxRate { get; private set; }

    private ProductOrigin(string name, TaxRate, taxRate)
    {
        Name = name;
        TaxRate = taxRate;
    }

    public double CalculateTax(double price)
    {
        return TaxRate.CalculateTax(price);
    }
}

We only have two product origins:

  • Local
  • Imported

Each origin takes a tax rate. The Local product origin is tax exempt: TaxRate.Exempt. The Imported product origin has the TaxRate.ImportDuty tax. Again, the ProductOrigin class does not calculate the tax. It simply delegates to the TaxRate again. Now we go one level deeper to actually calculate the tax.

TaxRate class

We start to notice a pattern here. We utilize a private constructor again and limit the tax rates that are available, though to be honest, we could store this in a database.

public class TaxRate
{
    public static readonly TaxRate Exempt = new TaxRate("Tax Exempt", 0.0);
    public static readonly TaxRate SalesTax = new TaxRate("Sales Tax", 0.1);
    public static readonly TaxRate ImportDuty = new TaxRate("Import Duty", 0.05);

    public string Name { get; private set; }
    public double Rate { get; private set; }

    private TaxRate(string name, double rate)
    {
        Name = name;
        Rate = rate;
    }

    public double CalculateTax(double price)
    {
        return price * Rate;
    }
}

We have three tax rates:

  • Exempt, with a zero tax rate
  • SalesTax, with a 10% tax rate
  • ImportDuty, with a 5% tax rate

These objects are used by the ProductOrigin and ProductType classes. This seems like a lot of work for writing price * rate, but this buys you Separate of Concerns. If books become taxable in the future, you simply change one line of code:

public static readonly ProductType Book = new ProductType("Book", TaxRate.SalesTax);

Your entire application gets updated. No if statements. No switches. Just one line of code.

The last requirement was to have a product quantity, and to calculate the total amount, plus tax, and total price. This is yet another concern in the application, necessitating another class: InvoiceItem.

The InvoiceItem class

The invoice item class has a product and a quantity. It should also calculate the total price, total tax and total amount (price + tax).

public class InvoiceItem
{
    public Product Product { get; private set; }
    public long Quantity { get; private set; }
    public double TotalAmount { get; private set; }
    public double TotalPrice { get; private set; }
    public double TotalTax { get; private set; }

    public InvoiceItem(Product product, long quantity = 0)
    {
        Product = product;
        Quantity = quantity;
    }

    public double DecrementQuantity()
    {
        if (Quantity > 0)
        {
            SetQuantity(Quantity - 1);
        }
    }

    public double IncrementQuantity()
    {
        SetQuantity(Quantity + 1);
    }

    public double SetQuantity(long quantity)
    {
        Quantity = quantity;
        TotalPrice = Product.Price * Quantity;
        TotalTax = TotalPrice * Product.CalculateTax() * Quantity;
        TotalAmount = TotalPrice + TotalTax;

        return TotalAmount;
    }
}

All of the properties are private setters. You don't want to introduce bugs by allowing someone to update the quantity, but not the totals. Furthermore, you don't want a clever programmer to set the product to something cheap, calculate the totals, and then set the product to something expensive. Yikes. Sounds bad.

There are three methods that allow you to change the quantity:

  • SetQuantity: This does all of the calculations
  • IncrementQuantity: Hey, sometimes you just want one more
  • DecrementQuantity: Hey, sometimes you got just one too many

And to use this class:

var item = new InvoiceItem(importedBook, 2);

item.TotalAmount -> 44.079
item.TotalPrice  -> 41.98
item.TotalTax    -> 2.099

By utilizing Domain Driven Design you can build flexibility into your system, making it easier to change. For example, if a product type becomes taxable again you only need to change one line of code. Furthermore, you can prevent abuse and bugs by limiting who can modify properties and forcing consumers of your classes to go through public methods to modify the internal data. The InvoiceItem class prevents you from setting the product after creating the object, and it forces you to call a public method to modify the quantity, which internally recalculates the total price, total tax and total amount for that item.

\$\endgroup\$
  • \$\begingroup\$ This shows there ere many problems with my code.Thanks Sir \$\endgroup\$ – vish1990 Jun 29 '15 at 6:19
  • \$\begingroup\$ There are still problems with my implementation. Many types of products have an exise tax. For instance, in the USA, gasoline has an additional tax in many states on top of sales tax. Alcoholic beverages will to, and many of those beverages are also imported, which could include an additional tax. Really you need to store zero or more taxes for a product, product origin and product type. So my implementation isn't perfect either. \$\endgroup\$ – Greg Burghardt Jun 29 '15 at 12:13
  • \$\begingroup\$ There are even taxes for counties and municipalities which can vary, sometimes even within limited time frames (e.g., Tax Increment Financing). To allow for everything, the OP may have to use a rules engine rather than trying to place the tax rules in code. \$\endgroup\$ – Dan Lyons Jun 30 '15 at 17:49
  • \$\begingroup\$ @DanLyons: Agreed. Taxes are a sticky matter. Even the concept of "local" versus "imported" gets muddy when you application can run in multiple regions/countries. I think a rules engine is a tad out of scope for an interview question though. :) \$\endgroup\$ – Greg Burghardt Jun 30 '15 at 17:59
4
\$\begingroup\$

Greg has given you an excellent review. Here are just a few more comments:

switch (_typeOfProduct)
{
    case ProductType.ExemptedProduct: break;
    case ProductType.TaxPaidProduct:
        tax += _productPrice * .10;
        break;
}

I would just use an if right there because you only take action on one condition.


Remove commented code. If it is buggy, it should really be removed because there is the danger of it being uncommented later. If it is used to test the code, maybe you should set up unit tests. Also, it doesn't do anything, and just makes more code for everyone to read through.


You may wish to consider optional arguments in the ctor of Product. Particularly, the ProductType type and bool isImportedProduct can only have two values, so you may wish to allow the user to not have to enter them every time they create a new product.


Don't use doubles to store money as they have the possibility of introducing rounding errors. Rather, you should use System.Decimal, according to this Stack Overflow question: What is the best data type to use for money in C#?

\$\endgroup\$
-2
\$\begingroup\$

I have used Greg's code to make it a bit modular. It's now divided basically in 2 phases (sales tax + import duty tax). Also, I used a dataTable to store data in SQL, as much data simultaneously without affecting performance. Also, a 32-bit GUID is created so as to store data in SQL (as primary key).

Program.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Data;

namespace Console
{

    public class Product
{
      //  private Product item;

    public string Name { get; set; }
    public double Price { get; set; }
    public ProductOrigin Origin { get; private set; }
    public ProductType Type { get; private set; }



    public Product(ProductType type, ProductOrigin origin, string name, double price)
    {
        Type = type;
        Origin = origin;
        Name = name;
        Price = price;
    }



    public double CalculateTax(Product item)
    {

        double p = item.Price;




            double a = item.Type.CalculateTax(item);


        double b = item.Origin.CalculateTax(item);
        return (a+b);
    }
}

    public  class ProductType
    {
        public static readonly ProductType Beauty = new ProductType("Beauty", TaxRate.SalesTax);
        public static readonly ProductType Book = new ProductType("Book", TaxRate.Exempt);
        public static readonly ProductType Entertainment = new ProductType("Entertainment", TaxRate.SalesTax);
        public static readonly ProductType Food = new ProductType("Food", TaxRate.Exempt);
        public static readonly ProductType Medical = new ProductType("Medical", TaxRate.Exempt);

        public string Name { get; private set; }
        public  TaxRate TaxRate { get; private set; }

        public ProductType(string name, TaxRate taxRate)
        {
            Name = name;
            TaxRate = taxRate;
        }

        public double CalculateTax(Product p)
        {
            return TaxRate.CalculateTax(p.Price);
        }



    }


    public class ProductOrigin
{
    public static readonly ProductOrigin Local = new ProductOrigin("Local", TaxRate.Exempt);
    public static readonly ProductOrigin Imported = new ProductOrigin("Imported", TaxRate.ImportDuty);

    public string Name { get;  set; }
    public  TaxRate TaxRate { get; private set; }

    private ProductOrigin(string name, TaxRate taxRate)
    {
        Name = name;
        TaxRate = taxRate;
    }


    public  double CalculateTax(Product item)
    {
        return TaxRate.CalculateTax(item.Price);
    }
}


    public class TaxRate
{
    public static readonly TaxRate Exempt = new TaxRate("Tax Exempt", 0.0);
    public static readonly TaxRate SalesTax = new TaxRate("Sales Tax", 0.1);
    public static readonly TaxRate ImportDuty = new TaxRate("Import Duty", 0.05);

    public string Name { get; private set; }
    public double Rate { get; private set; }

    private TaxRate(string name, double rate)
    {
        Name = name;
        Rate = rate;
    }


        //f
    public double CalculateTax(double price)
    {
        double val=price * Rate;
        return Math.Round(val * 20) / 20;    
    }
}



    class Program
    {
        static void Main(string[] args)
        {

            double ans = 0;

            List<Product> lstProducts = new List<Product>();
            //input 1


            //lstProducts.Add(new Product(ProductType.Book,ProductOrigin.Local,"Book",12.49));
            //lstProducts.Add(new Product(ProductType.Entertainment, ProductOrigin.Local, "CD", 14.99));
            //lstProducts.Add(new Product(ProductType.Food,ProductOrigin.Local,"Choclate",0.85));

            //input 2


            lstProducts.Add(new Product(ProductType.Food, ProductOrigin.Imported, "Choclate", 10.00));
            lstProducts.Add(new Product(ProductType.Beauty, ProductOrigin.Imported, "Perfume", 47.50));

            //DataTable dt = new DataTable();
            //dt.Columns.AddRange(new DataColumn[4] { new DataColumn("GUID"),new DataColumn("Item"), new DataColumn("Price"), new DataColumn("Tax") });




            foreach (Product item in lstProducts)
            {
               double  temp = item.CalculateTax(item);
               ans = ans + temp + item.Price;
              // dt.Rows.Add(Utility.NewGuid().ToString(), item.Name, item.Price + temp, temp);

              System.Console.WriteLine(string.Format("Item = {0} : Price = {1} : Tax = {2}", item.Name, item.Price+temp,temp));


            }
            //bool result = BL.UpdateSales(dt);
            //if(result==true)
            { System.Console.WriteLine("Total Cost of all Item : " + ans); }
            //else
            //{System.Console.WriteLine("failure");}
           System.Console.ReadLine();



        }
    }
}

BL.CS

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Data.SqlClient;
using System.Data;
using System.Configuration;

namespace Console
{
    class BL
    {
        DataTable dt = null;
        public static bool UpdateSales(DataTable _dt)
        {
            bool return_value=false;
            try
            {
                string connectionString = ConfigurationManager.ConnectionStrings["ConfigDetail"].ConnectionString;


                SqlBulkCopy bulkCopy = new SqlBulkCopy(connectionString, SqlBulkCopyOptions.TableLock)
                {
                    DestinationTableName = "dbo.MYTABLE",
                    BatchSize = 100000,
                    BulkCopyTimeout = 360
                };
                bulkCopy.WriteToServer(_dt);

                return true;
            }

            catch (Exception ex)
            {
                System.Console.WriteLine(ex.ToString());
            }
           return return_value;
        }
    }
}

Utility.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;

namespace Console
{
    static class Utility
    {
        static int _sequenced = (int)DateTime.UtcNow.Ticks;

        private static readonly System.Security.Cryptography.RNGCryptoServiceProvider _random =
        new System.Security.Cryptography.RNGCryptoServiceProvider();
        private static readonly byte[] _buffer = new byte[6];

        public static Guid NewGuid()
        {
            long ticks = DateTime.UtcNow.Ticks;
            int sequenceNum = System.Threading.Interlocked.Increment(ref _sequenced);
            lock (_buffer)
            {
                _random.GetBytes(_buffer);
                return new Guid(

                 (int)(ticks >> 18), (short)(ticks >> 16), (short)ticks,
                 (byte)(sequenceNum >> 8), (byte)sequenceNum,
                 _buffer[0], _buffer[1], _buffer[2], _buffer[3], _buffer[4], _buffer[5]
                 );
            }
        }
    }
}

App.config

<?xml version="1.0" encoding="utf-8" ?>
<configuration>

  <connectionStrings>

    <add name="ConfigDetail"
        connectionString=""
        providerName="System.Data.SqlClient" />
  </connectionStrings>



</configuration>
\$\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.