Sales tax calculator

Please review the sales tax problem which has been designed using strategy pattern.

The Problem:


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. Also the Sales Tax should be rounded off to the
nearest 0.05.

Item.h

```
/*
* Item.h
*
* Created on: Jun 7, 2011
* Author: som
*/

#ifndef ITEM_H_
#define ITEM_H_

class SalesTax;

//This represents the Items which don't have an Import duty or any sales tax
class Item
{
public:

//Constructors
Item();
Item(SalesTax *aSalesTax);

//Interface Functions for Item

//To calculate the price after tax and import duty
virtual void CalculateTotalPrice();

//To calculate the total tax and import duty
virtual void CalculateTotalTax();

//To set the price of the Items
void SetPrice(double aPrice);

//To get the price of the Items before tax
double getPrice();

//To get the price of the items after tax
double getTotalPrice();

//To get the total tax and import duty of the items
double getTax();

//Data
protected:
//Works as the Strategy of the Sales Tax problem.
//If in future the tax calculation becomes more complicated for different Items
//we will just have to change this Strategy. We can also subclass this Strategy class
//for future expansion of the tax calculation strategy
SalesTax *iSalesTax;
//Data
protected:

//These are the basic properties of any Item.
//Hence these are made protected members so that the subclasses of Item can inherit
//these properties
double iPrice;
double iTotalPrice;
double iTotalTax;
};

//This class represents the Items which have only Import Duty
class ImportedItem : virtual public Item
{
public:
//Constructors
ImportedItem();

//This constructor helps to create Items having only Import duty
ImportedItem(SalesTax *aSalesTax, double aImportDuty);

class NonFoodBookMedicalItem : virtual public Item
{

I don't feel like this is the right approach, for several reasons. For a start, you've baked quite a bit of application logic right into the type name, which will be copied and reused throughout your code. How much work will be involved if the government decides that sales tax should apply to books? Every single usage of it will need to be updated to NonFoodMedicalItem.

As an aside: never assume that tax situations won't change. UK Value Added Tax (VAT) was 17.5% for as long as anyone could remember, and everyone's formulae had it hard-coded. Thousands of forms had the VAT rate element pre-printed with 17.5%. This worked fine until the government changed it in the wake of the global financial crisis.

Secondly, the naming is just awkward. Is it a non-food item that can be a book or medical item, or is it a non-food, non-book and non-medical item? The ordinary usage of the English language leaves this ambiguous.

class NormalItem: public ImportedItem, public NonFoodBookMedicalItem
{

The public inheritance from ImportedItem here is questionable. Public inheritance in C++ should be used to model the "is-A" relationship, i.e. if Foo publically inherits from Bar then it should be the case that every instance of Foo is a Bar. It's clearly not the case that every item is an imported item.

I'm not sure I see why you're using inheritance in the way you are, while at the same time using the strategy pattern. It looks like your aim was to employ the strategy pattern so that each instance delegated its tax calculations to its iSalesTax member object. The type of the object that the iSalesTax object points to can vary at run-time independently of the type of the object itself -- this is how the strategy pattern is supposed to work. But if you do this, I don't see the advantage in having a different static type for each tax situation. Should I be able to replace the iSalesTax in a book item with one that adds sales tax? If so, why declare a different type? If not, why use the strategy pattern?

StackExchange Code Review Q#2929, answer score: 4