How to create the perfect OOP application

Recently I was trying for a company ‘x’. They sent me some set of questions and told me to solve only one.

The problem is like this -

Basic sales tax is applicable at a rate of 10% on all goods, except books, food, and medical products that are exempt.
Import duty is an additional sales tax applicable on all imported goods at a rate of 5%, with no exemptions.

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 (np/100 rounded up to the nearest 0.05) amount of sales tax.

“They told me, they are interested in the Design Aspect of your solution and would like to evaluate my Object Oriented Programming Skills.”

This is what they told in their own words

• For the solution, we would want you use either Java, Ruby or C#.
• We are interested in the DESIGN ASPECT of your solution and would like to evaluate your Object Oriented Programming Skills.
• You may use external libraries or tools for building or testing purposes. Specifically, you may use unit testing libraries or build tools available for your chosen language (e.g., JUnit, Ant, NUnit, NAnt, Test::Unit, Rake etc.)
• Optionally, you may also include a brief explanation of your design and assumptions along with your code.
• Kindly note that we are NOT expecting a web-based application or a comprehensive UI. Rather, we are expecting a simple, console based application and interested in your source code.

So I provided below code – you can just copy paste code and run in VS.

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
     }
 }

you can uncommnet input and run for different inputs.

I provided the solution but I was rejected.

"They said, they are unable to consider me for our current open positions because code solution is not satisfactory."

Please guide me what is missing here. Is this solution is not a good OOAD solution.
How can I improve my OOAD skills.
My seniors also says perfect OOAD application will also not work practically.

Thanks

First off good heavens do not do financial calculations in double. Do financial calculations in decimal; that is what it is for. Use double to solve physics problems, not financial problems.

The major design flaw in your program is that policy is in the wrong place. Who is in charge of computing the taxes? You've put the product in charge of computing the taxes, but when you buy an apple or a book or a washing machine, the thing you are about to buy is not responsible for telling you how much tax you're going to pay on it. Government policy is responsible for telling you that. Your design massively violates the basic OO design principle that objects should be responsible for their own concerns, and not anyone else's. The concern of a washing machine is washing your clothes, not charging the right import duty. If the tax laws change, you don't want to change the washing machine object, you want to change the policy object.

So, how to approach these sorts of problems in the future?

I would have started by highlighting every important noun in the problem description:

> Basic sales tax is applicable at a rate of 10% on all goods, except books, food, and medical products that are exempt. Import duty is an additional sales tax applicable on all imported goods at a rate of 5%, with no exemptions. 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 (np/100 rounded up to the nearest 0.05) amount of sales tax.

Now, what are the relationships between all those nouns?

• Basic Sales Tax is a kind of Sales Tax
• Import Duty is a kind of Sales Tax
• A Sales Tax has a Rate which is a Decimal
• Books are a kind of Item
• Food is a kind of Item
• Medical Products are a kind of Item
• Items may be Imported Goods
• An Item has a Name which is a String
• An Item has a Shelf Price which is a Decimal. (Note: does an item really have a price? two identical washing machines might be for sale for different prices at different stores, or at the same store at different times. A better design might be to say that a Pricing Policy relates an Item to its Price.)
• A Sales Tax Exemption Policy describes the conditions under which a Sales Tax is inapplicable on an Item.
• A Receipt has a list of Items, their prices and their taxes.
• A Receipt has a total
• A Receipt has a total tax

... and so on. Once you have all the relationships between all the nouns worked out, then you can start designing a class hierarchy. There is an abstract base class Item. Book inherits from it. There is an abstract class SalesTax; BasicSalesTax inherits from it. And so on.

If company tells something about libraries like NUnit, JUnit or Test::Unit is more than probable that TDD is really importat to them. In your code sample is no tests at all.

I would try to demonstrate practical knowledge of:

• Unit tests (eg. NUnit, XUnit, ArchUnitNet, ...)
• Design patterns
• SOLID principles
• Clean Architecture
• Persistence (eg. Entity Framework, NHibernate)
• IoC Containers (eg. AutoFac)

I would like to recomend the www.dimecasts.net as impressive source of free, good quality screencasts which covers all above mentioned topics.

This is highly subjective but here are a few points that I'd make about your code:

• In my opinion you mixed Product and ShoppingCartItem. Product should have the product name, tax status, etc. but not quantity. Quantity is not a property of a product - it'll be different for each customer of the company who buys that particular product.

• ShoppingCartItem should have a Product and the quantity. That way the customer can freely buy more or less of the same product. With your current setup that's not possible.

• Calculating the final tax also shouldn't be part of the Product - it should be part of something like ShoppingCart since the final tax calculation may involve knowing all products in the cart.

First of all, this is a very good interview question. It's a good gauge of many skills.

There're many things you need to understand to provide a good answer (there is no perfect answer), both high-level and low-level. Here're a couple:

• Domain Modeling -> how do you create a good model of the solution? What objects do you create? How will they solve the requirements? Looking for the nouns is a good start, but how do you decide if your choice of entities is good? What other entities do you need? What domain knowledge do you need to solve it?
• Separation of concerns, loose coupling, high cohesion -> How do you separate out the parts of the design that have different concerns or rates of change and how do you relate them? How do you keep your design flexible and current?
• Unit testing, refactoring, TDD -> What's your process for coming up with a solution? Do you write tests, use mock objects, refactor, iterate?
• Clean code, Language idioms -> Do you use the features of your programming language to help you? Do you write understandable code? Do your levels of abstraction make sense? How maintainable is the code?
• Tools: Do you use source control? Build tools? IDEs?

From there, you can have many interesting discussions, involving design principles (like the SOLID principles), design patterns, analysis patterns, domain modeling, technology choices, future evolution paths (e.g. what if I add a database, or a rich UI layer, what needs to change?), trade-offs, non-functional requirements (performance, maintainability, security, ...), acceptance testing, etc...

I won't comment on how you should change your solution, just that you should focus more on these concepts.

But, I can show you how I (partially) solved this problem, just as an example (in Java). Look in the Program class to see how it all comes together to print this receipt:

------------------ THIS IS YOUR ORDER ------------------
(001)                Domain Driven Design -----   $69.99
(001)    Growing Object Oriented Software -----   $49.99
(001)                 House M.D. Season 1 -----   $29.99
(001)                 House M.D. Season 7 -----   $34.50
(IMD)    Growing Object Oriented Software -----    $2.50
(BST)                 House M.D. Season 1 -----    $3.00
(BST)                 House M.D. Season 7 -----    $3.45
(IMD)                 House M.D. Season 7 -----    $1.73
SUB-TOTAL -----  $184.47
TAX TOTAL -----   $10.68
TOTAL -----  $195.15
---------------- THANKS FOR CHOOSING US ----------------

You should definitely take a look at those books :-)

Just as a caveat: my solution is still very incomplete, I just focused on the happy path scenario in order to have a good foundation to build on.

Except the fact that you are using a class called product, you have not demonstrated you know what inheritance is, you have not created multiple classed inheriting from Product, no polymorphism. The problem could have been solved using multiple OOP concepts (even just to show that you know them). This is an interview problem so you want to show how much you know.

I wouldn't however turn into depression right now. The fact that you didn't demonstrate them here does not mean you don't already know them or are not able to learn them.

You just need a little more experience with either OOP or interviews.

Good luck!

People who have start learning programming with OOP don't have great problems to understand what it means, because it is just as in real life. If you have skills with other programming familly than OO, it could be more difficult to understand.

First of all, turn-off your screen, or exit your favorite IDE. Take a paper and a pencil and make a list of entities, relations, people, machines, processes, stuff, etc. everything that could be encountered into your final program.

Second, try to get the different basic entities. You will understand that some can share properties or abilities, you have to put it in abstract objects. You should start to draw a nice schema of your program.

Next you have to put fonctionnalities (methods, functions, subroutines, call it as you want): for example, a product object should not be able to compute sales tax. A sales engine object should.

Don't feel trouble with all the big words(interfaces, properties, polymorphism, heritage, etc. ) and design patterns in a first time, don't even try to make beautiful code or whatever... Just think to simple objects and interractions between it as in real life.

After, try to read some serious concise litterature about this. I think Wikipedia and Wikibooks are a really good way to begin and then just read stuff about GoF and Design Patterns and UML.

First don't mix Product class with Receipt(ShoppingCart) class, the quantity should be part of ReceipItem(ShoppingCartItem), as well as Tax&Cost. The TotalTax&TotalCost should be part of ShoppingCart.

My Product class, has only Name&Price & some readonly properties like IsImported:

class Product
{
	static readonly IDictionary<ProductType, string[]> productType_Identifiers = 
		new Dictionary<ProductType, string[]>
		{
			{ProductType.Food, new[]{ "chocolate", "chocolates" }},
			{ProductType.Medical, new[]{ "pills" }},
			{ProductType.Book, new[]{ "book" }}
		};

	public decimal ShelfPrice { get; set; }

	public string Name { get; set; }

	public bool IsImported { get { return Name.Contains("imported "); } }

	public bool IsOf(ProductType productType)
	{
		return productType_Identifiers.ContainsKey(productType) &&
			productType_Identifiers[productType].Any(x => Name.Contains(x));
	}
}

class ShoppringCart
{
    public IList<ShoppringCartItem> CartItems { get; set; }

    public decimal TotalTax { get { return CartItems.Sum(x => x.Tax); } }

    public decimal TotalCost { get { return CartItems.Sum(x => x.Cost); } }
}

class ShoppringCartItem
{
    public Product Product { get; set; }

    public int Quantity { get; set; }

    public decimal Tax { get; set; }

    public decimal Cost { get { return Quantity * (Tax + Product.ShelfPrice); } }
}

Your tax calculation part is coupled with Product. A Product doesn't define tax policies it's Tax classes. Based on the problem's description, there are two kind of Sales Taxes: Basic and Duty taxes. You can use Template Method Design Pattern to achieve it:

abstract class SalesTax
{
    abstract public bool IsApplicable(Product item);
    abstract public decimal Rate { get; }

    public decimal Calculate(Product item)
    {
        if (IsApplicable(item))
        {
            //sales tax are that for a tax rate of n%, a shelf price of p contains (np/100)
            var tax = (item.ShelfPrice * Rate) / 100;

            //The rounding rules: rounded up to the nearest 0.05
            tax = Math.Ceiling(tax / 0.05m) * 0.05m;

            return tax;
        }

        return 0;
    }
}

class BasicSalesTax : SalesTax
{
    private ProductType[] _taxExcemptions = new[] 
    { 
        ProductType.Food, ProductType.Medical, ProductType.Book 
    };

    public override bool IsApplicable(Product item)
    {
        return !(_taxExcemptions.Any(x => item.IsOf(x)));
    }

    public override decimal Rate { get { return 10.00M; } }
}

class ImportedDutySalesTax : SalesTax
{
    public override bool IsApplicable(Product item)
    {
        return item.IsImported;
    }

    public override decimal Rate { get { return 5.00M; } }
}

And finally a class to apply taxes:

class TaxCalculator
{
    private SalesTax[] _Taxes = new SalesTax[] { new BasicSalesTax(), new ImportedDutySalesTax() };

    public void Calculate(ShoppringCart shoppringCart)
    {
        foreach (var cartItem in shoppringCart.CartItems)
        {
            cartItem.Tax = _Taxes.Sum(x => x.Calculate(cartItem.Product));
        }

    }
}

You can try them out at MyFiddle.

A very good starting point about design rules are the SOLID principles.

For instance the Open Closed principle states that if you want to add new functionality you don't have to add code to existing class, but rather add new class.

For your sample application this would mean that adding new sales tax would require adding new class. The same goes for different products that are exceptions to the rule.

The rounding rule obviously goes in separate class - the Single Responsibility principle states that every class has a single responsibility.

I think trying to write the code yourself would bring by far more benefit than simply writing a good solution and pasting it here.

A simple algorithm to write the perfect designed program would be:

1. Write some code that solves the problem
2. Check whether the code complies to the SOLID principles
3. If there are rule violations than goto 1.

A perfect OOP implementation is completely debatable. From what I see in your question, you could modularize the code based on the role they perform to compute the final price like Product, Tax, ProductDB and so on.

1. Product could be an abstract class and the derived types like Books, Food could be inherited from it. Tax applicability can be decided by the derived types. Product would tell whether the tax is applicable or not based on the derived class.

2. TaxCriteria can be an enum and the this can be specified during purchase (imported, Sales Tax applicability).

3. Tax class will compute tax based on TaxCriteria.

4. Having a ShoppingCartItem as suggested by XXBBCC can encapsulate Product and Tax instances and it is a great way to segregate product details with quantity, total price with tax etc.

Good luck.

From a strictly OOA/D perspective, one major issue I see is that most of your class attributes have the redundant name of the class in the attribute name. e.g. product Price, typeOf Product. In this case, everywhere you use this class you will have overly verbose and somewhat confusing code, e.g. product.productName. Remove the redundant class name prefix/suffixes from your attributes.

Also, I did not see any classes concerned with purchasing and creating a receipt as was asked in the question.

Here's a great example of an OO pattern for Products, Tax, etc... Notice the use of Interfaces, which is essential in OO design.

http://www.dreamincode.net/forums/topic/185426-design-patterns-strategy/

Attacked the Cost with Tax problem using a Visitor pattern.

public class Tests
    {
        [SetUp]
        public void Setup()
        {
        }
        
        [Test]
        public void Input1Test()
        {
            var items = new List<IItem> {
                new Book("Book", 12.49M, 1, false),
                new Other("Music CD", 14.99M, 1, false),
                new Food("Chocolate Bar", 0.85M, 1, false)};

            var visitor = new ItemCostWithTaxVisitor();

            Assert.AreEqual(12.49, items[0].Accept(visitor));
            Assert.AreEqual(16.49, items[1].Accept(visitor));
            Assert.AreEqual(0.85, items[2].Accept(visitor));
        }

        [Test]
        public void Input2Test()
        {
            var items = new List<IItem> {
                new Food("Bottle of Chocolates", 10.00M, 1, true),
                new Other("Bottle of Perfume", 47.50M, 1, true)};

            var visitor = new ItemCostWithTaxVisitor();

            Assert.AreEqual(10.50, items[0].Accept(visitor));
            Assert.AreEqual(54.65, items[1].Accept(visitor));
        }

        [Test]
        public void Input3Test()
        {
            var items = new List<IItem> {
                new Other("Bottle of Perfume", 27.99M, 1, true),
                new Other("Bottle of Perfume", 18.99M, 1, false),
                new Medicine("Packet of headache pills", 9.75M, 1, false),
                new Food("Box of Chocolate", 11.25M, 1, true)};

            var visitor = new ItemCostWithTaxVisitor();

            Assert.AreEqual(32.19, items[0].Accept(visitor));
            Assert.AreEqual(20.89, items[1].Accept(visitor));
            Assert.AreEqual(9.75, items[2].Accept(visitor));
            Assert.AreEqual(11.80, items[3].Accept(visitor));
        }
    }

    public abstract class IItem : IItemVisitable
    { 
        public IItem(string name,
            decimal price,
            int quantity,
            bool isImported)
            {
                Name = name;
                Price = price;
                Quantity = quantity;
                IsImported = isImported;
            }

        public string Name { get; set; }
        public decimal Price { get; set; }
        public int Quantity { get; set; }
        public bool IsImported { get; set; }

        public abstract decimal Accept(IItemVisitor visitor);
    }

    public class Other : IItem, IItemVisitable
    {
        public Other(string name, decimal price, int quantity, bool isImported) : base(name, price, quantity, isImported)
        {
        }

        public override decimal Accept(IItemVisitor visitor) => Math.Round(visitor.Visit(this), 2);
    }

    public class Book : IItem, IItemVisitable
    {
        public Book(string name, decimal price, int quantity, bool isImported) : base(name, price, quantity, isImported)
        {
        }

        public override decimal Accept(IItemVisitor visitor) => Math.Round(visitor.Visit(this),2);
    }

    public class Food : IItem, IItemVisitable
    {
        public Food(string name, decimal price, int quantity, bool isImported) : base(name, price, quantity, isImported)
        {
        }

        public override decimal Accept(IItemVisitor visitor) => Math.Round(visitor.Visit(this), 2);
    }

    public class Medicine : IItem, IItemVisitable
    {
        public Medicine(string name, decimal price, int quantity, bool isImported) : base(name, price, quantity, isImported)
        {
        }

        public override decimal Accept(IItemVisitor visitor) => Math.Round(visitor.Visit(this), 2);
    }

    public interface IItemVisitable
    {
        decimal Accept(IItemVisitor visitor);
    }

    public class ItemCostWithTaxVisitor : IItemVisitor
    {
        public decimal Visit(Food item) => CalculateCostWithTax(item);

        public decimal Visit(Book item) => CalculateCostWithTax(item);

        public decimal Visit(Medicine item) => CalculateCostWithTax(item);

        public decimal CalculateCostWithTax(IItem item) => item.IsImported ?
            Math.Round(item.Price * item.Quantity * .05M * 20.0M, MidpointRounding.AwayFromZero) / 20.0M + (item.Price * item.Quantity)
            : item.Price * item.Quantity;
               
        public decimal Visit(Other item) => item.IsImported ?
            Math.Round(item.Price * item.Quantity * .15M * 20.0M, MidpointRounding.AwayFromZero) / 20.0M + (item.Price * item.Quantity)
            : Math.Round(item.Price * item.Quantity * .10M * 20.0M, MidpointRounding.AwayFromZero) / 20.0M + (item.Price * item.Quantity);
    }

    public interface IItemVisitor
    {
        decimal Visit(Food item);
        decimal Visit(Book item);
        decimal Visit(Medicine item);
        decimal Visit(Other item);
    }