HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Sales tax calculator with branching based on postal address

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
addresswithtaxbranchingsalesbasedcalculatorpostal

Problem

A problem I come across very often is what to do with branching classes. The class below is one example of a class that can turn ugly very fast.

The requirements of the class is that if the address is in Alabama use an external service, but all other states should use a database. The switch statements filter down which method to use. Each method has it's own way of processing the data and formatting it properly.

How can I make this class and these types of situations cleaner?

```
public class TaxRateServiceConnector : ITaxRateServiceConnector
{
private readonly AppContext _context;

public TaxRateServiceConnector(AppContext context)
{
_context = context;
}

public List GetTaxRates(Address address)
{
List taxList;

if (address == null)
{
throw new ArgumentNullException("Address may not be null");
}

var state = address.Region.Description;

switch (state)
{
case "Alabama":
taxList = GetAlabamaTaxRates(address);
break;
default:
taxList = GetDefaultTaxRates(address);
break;
}

// If tax list is empty, then return a default 6.5% tax
return taxList ?? new List()
{
new Taxes()
{
TaxDescription = "Sales Tax",
TaxAmount = 0.08M
}
};
}

// Use Alabama service for taxes
private List GetAlabamaTaxRates(Address address)
{
AlabamaTaxService alabamaService = new AlabamaTaxService();
List taxRates = null;

string zip = address.PostCode.Substring(0, 5);
int zipNum;

bool convertSuccess = Int32.TryParse(zip, out zipNum);

if (!convertSuccess)
{
return null;
}

var alabamaServiceResponse = alabamaService.GetSalesTaxRate(zipNum, DateTime.Now);

bool zipFound = alabamaServiceResponse.int

Solution

I think you can refactor the sources as ITaxRateSource like this:

public interface ITaxRateSource 
{ 
    List GetTaxRates(Address address);
}


and implement each one in a separate class:

public class DefaultTaxRateSource : ITaxRateSource
{
    public List GetTaxRates(Address address)
    {
        ...
    }
}

public class AlabamaTaxRateSource : ITaxRateSource
{
    public List GetTaxRates(Address address)
    {
        ...
    }
}


Then you modify the constructor to accept custom sources (or by params, whatever you like better):

public TaxRateServiceConnector(
    AppContext context, 
    IEnumerable> taxRateSources = null)
{
    _context = context;

    // whether you add a default one or not is your choice
    _taxRateSources = new Dictionary
    {
        [string.Empty] = new DefaultTaxRateSource()
    };

    if (taxRateSources != null)
    {
        foreach (var item in taxRateSources)
        {
            // you may use the indexer _taxRateSources.[item.Key] 
            // if you want to be able to override the default source
            _taxRateSources.Add(item.Key, item.Value);
        }
    }
}


and GetTaxRates to use the dictionary insteas of a switch:

public List GetTaxRates(Address address)
{
    ...

    var state = address.Region.Description;

    var taxRateSource = (ITaxRateSource)null;
    if (!_taxRateSources.TryGetValue(state, out taxRateSource)) 
    {
        taxRateSource = _taxRateSources[string.Empty];
    }

    var taxRates = taxRateSource.GetTaxRates(...);

    ...
}


to create a new instance of the TaxRateServiceConnector you pass it a dictionary with the custom source:

var taxRateSources = new Dictionary
{
    ["Alabama"] = new AlabamaTaxRateSource()
};


Now you are free to create as many custom sources as you like.

Other designs are possible too. You could create custom sources by overriding the GetTaxRates method of same base class. Sometimes it's a matter of taste whether you choose an interface or an inheritance.

Yet another one would be to create an attribute like [State("Alabama")] and decorate the sources with it and build a source with composition and use the attribute for source lookup.

Yet another one would be to create an enum State and decorate its values with a factory attribute that creates the appropriate source for the state.

Code Snippets

public interface ITaxRateSource 
{ 
    List<Taxes> GetTaxRates(Address address);
}
public class DefaultTaxRateSource : ITaxRateSource
{
    public List<Taxes> GetTaxRates(Address address)
    {
        ...
    }
}

public class AlabamaTaxRateSource : ITaxRateSource
{
    public List<Taxes> GetTaxRates(Address address)
    {
        ...
    }
}
public TaxRateServiceConnector(
    AppContext context, 
    IEnumerable<KeyValuePair<string, ITaxRateSource>> taxRateSources = null)
{
    _context = context;

    // whether you add a default one or not is your choice
    _taxRateSources = new Dictionary<string, ITaxRateSource>
    {
        [string.Empty] = new DefaultTaxRateSource()
    };

    if (taxRateSources != null)
    {
        foreach (var item in taxRateSources)
        {
            // you may use the indexer _taxRateSources.[item.Key] 
            // if you want to be able to override the default source
            _taxRateSources.Add(item.Key, item.Value);
        }
    }
}
public List<Taxes> GetTaxRates(Address address)
{
    ...

    var state = address.Region.Description;

    var taxRateSource = (ITaxRateSource)null;
    if (!_taxRateSources.TryGetValue(state, out taxRateSource)) 
    {
        taxRateSource = _taxRateSources[string.Empty];
    }

    var taxRates = taxRateSource.GetTaxRates(...);

    ...
}
var taxRateSources = new Dictionary<string, ITaxRateSource>
{
    ["Alabama"] = new AlabamaTaxRateSource()
};

Context

StackExchange Code Review Q#143124, answer score: 4

Revisions (0)

No revisions yet.