patterncsharpMinor
Sales tax calculator with branching based on postal address
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
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
and implement each one in a separate class:
Then you modify the constructor to accept custom sources (or by
and
to create a new instance of the
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
Yet another one would be to create an attribute like
Yet another one would be to create an
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.