Mixing
Reversible tax calculator class
Clash Royale CLAN TAG#URR8PPP
up vote
4
down vote
favorite
1
I have a class that contains 2 properties of the same type. decimal NetAmount and decimal GrossAmount
I would like to initialize it using either GrossAmount or NetAmount and based on the one specified calculate the second one.
Which way is the most elegant and why? (parameter validation is omitted for brevity)
1
public class TaxedPrice
private TaxedPrice()
public decimal NetAmount get; private set;
public decimal GrossAmount get; private set;
public static TaxedPrice FromNet(decimal netAmount, decimal taxRate)
return new TaxedPrice
NetAmount = decimal.Round(netAmount, 2, MidpointRounding.AwayFromZero),
GrossAmount = decimal.Round(netAmount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero)
;
public static TaxedPrice FromGross(decimal grossAmount, decimal taxRate)
return new TaxedPrice
GrossAmount = decimal.Round(grossAmount, 2, MidpointRounding.AwayFromZero),
NetAmount = decimal.Round(grossAmount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero)
;
2
public class TaxedPrice
public TaxedPrice(decimal netAmount, decimal grossAmount, decimal taxRate)
if (grossAmount != default)
GrossAmount = decimal.Round(grossAmount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(grossAmount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else if (netAmount != default)
NetAmount = decimal.Round(netAmount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(netAmount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
else
throw new InvalidOperationException($"Either nameof(netAmount) or grossAmount must be set.");
public decimal NetAmount get;
public decimal GrossAmount get;
3
public class TaxedPrice
public enum Type
Gross,
Net
public TaxedPrice(decimal amount, Type type, decimal taxRate)
if (type == Type.Gross)
GrossAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(amount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else if (type == Type.Net)
NetAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(amount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
public decimal NetAmount get;
public decimal GrossAmount get;
4
public class TaxedPrice
public TaxedPrice(decimal amount, bool fromGross, decimal taxRate)
if (fromGross)
GrossAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(amount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else
NetAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(amount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
public decimal NetAmount get;
public decimal GrossAmount get;
How it looks like from caller's side:
// 1
var taxedPrice = TaxedPrice.FromNet(2.123m, 0.23m);
// 2
var taxedPrice = new TaxedPrice(2.123m, default, 0.23m); // uses the first one to calculate the second one
var taxedPrice2 = new TaxedPrice(2.123m, 1.11m, 0.23m); // uses the first one to calculate the second one
var taxedPrice3 = new TaxedPrice(default, 1.11m, 0.23m); // uses the second one to calculate the first one
// 3
var taxedPrice = new TaxedPrice(2.123m, TaxedPrice.Type.Net, 0.23m);
// 4
var taxedPrice = new TaxedPrice(2.123m, false, 0.23m);
Extensions for tax:
public static class TaxExtensions
public static decimal ApplyTax(this decimal netPrice, decimal taxRate)
return netPrice * (taxRate + 1);
public static decimal RemoveTax(this decimal grossPrice, decimal taxRate)
return grossPrice / (taxRate + 1);
Mapping perspective
In my upper layer I pass those prices in POCOs/DTOs like:
public class PriceDTO
public decimal NetAmount get; set;
public decimal GrossAmount get; set;
And I have to check there as well which one was passed to decide from which to calculate. So in case of 1 mapping would look like:
if (priceDto.GrossAmount != default)
return TaxedPrice.FromGross(priceDto.GrossAmount, taxRate);
else if (priceDto.NetAmount != default)
return TaxedPrice.FromNet(priceDto.NetAmount, taxRate);
else
// error
In case of 2 (no need to check in the mapping code)
return new TaxedPrice(priceDto.NetAmount, priceDto.GrossAmount, taxRate)
3 - there's a check as well
4 - same like 1 and 3
And I agree this could be a struct instead.
c# comparative-review finance
edited 16 mins ago
200_success
125k14145406
asked 6 hours ago
Konrad
1386
 |Â
show 2 more comments
up vote
4
down vote
favorite
1
I have a class that contains 2 properties of the same type. decimal NetAmount and decimal GrossAmount
I would like to initialize it using either GrossAmount or NetAmount and based on the one specified calculate the second one.
Which way is the most elegant and why? (parameter validation is omitted for brevity)
1
public class TaxedPrice
private TaxedPrice()
public decimal NetAmount get; private set;
public decimal GrossAmount get; private set;
public static TaxedPrice FromNet(decimal netAmount, decimal taxRate)
return new TaxedPrice
NetAmount = decimal.Round(netAmount, 2, MidpointRounding.AwayFromZero),
GrossAmount = decimal.Round(netAmount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero)
;
public static TaxedPrice FromGross(decimal grossAmount, decimal taxRate)
return new TaxedPrice
GrossAmount = decimal.Round(grossAmount, 2, MidpointRounding.AwayFromZero),
NetAmount = decimal.Round(grossAmount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero)
;
2
public class TaxedPrice
public TaxedPrice(decimal netAmount, decimal grossAmount, decimal taxRate)
if (grossAmount != default)
GrossAmount = decimal.Round(grossAmount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(grossAmount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else if (netAmount != default)
NetAmount = decimal.Round(netAmount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(netAmount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
else
throw new InvalidOperationException($"Either nameof(netAmount) or grossAmount must be set.");
public decimal NetAmount get;
public decimal GrossAmount get;
3
public class TaxedPrice
public enum Type
Gross,
Net
public TaxedPrice(decimal amount, Type type, decimal taxRate)
if (type == Type.Gross)
GrossAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(amount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else if (type == Type.Net)
NetAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(amount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
public decimal NetAmount get;
public decimal GrossAmount get;
4
public class TaxedPrice
public TaxedPrice(decimal amount, bool fromGross, decimal taxRate)
if (fromGross)
GrossAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(amount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else
NetAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(amount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
public decimal NetAmount get;
public decimal GrossAmount get;
How it looks like from caller's side:
// 1
var taxedPrice = TaxedPrice.FromNet(2.123m, 0.23m);
// 2
var taxedPrice = new TaxedPrice(2.123m, default, 0.23m); // uses the first one to calculate the second one
var taxedPrice2 = new TaxedPrice(2.123m, 1.11m, 0.23m); // uses the first one to calculate the second one
var taxedPrice3 = new TaxedPrice(default, 1.11m, 0.23m); // uses the second one to calculate the first one
// 3
var taxedPrice = new TaxedPrice(2.123m, TaxedPrice.Type.Net, 0.23m);
// 4
var taxedPrice = new TaxedPrice(2.123m, false, 0.23m);
Extensions for tax:
public static class TaxExtensions
public static decimal ApplyTax(this decimal netPrice, decimal taxRate)
return netPrice * (taxRate + 1);
public static decimal RemoveTax(this decimal grossPrice, decimal taxRate)
return grossPrice / (taxRate + 1);
Mapping perspective
In my upper layer I pass those prices in POCOs/DTOs like:
public class PriceDTO
public decimal NetAmount get; set;
public decimal GrossAmount get; set;
And I have to check there as well which one was passed to decide from which to calculate. So in case of 1 mapping would look like:
if (priceDto.GrossAmount != default)
return TaxedPrice.FromGross(priceDto.GrossAmount, taxRate);
else if (priceDto.NetAmount != default)
return TaxedPrice.FromNet(priceDto.NetAmount, taxRate);
else
// error
In case of 2 (no need to check in the mapping code)
return new TaxedPrice(priceDto.NetAmount, priceDto.GrossAmount, taxRate)
3 - there's a check as well
4 - same like 1 and 3
And I agree this could be a struct instead.
c# comparative-review finance
edited 16 mins ago
200_success
125k14145406
asked 6 hours ago
Konrad
1386
There are some methods missing:RemoveTaxandApplyTax. Are these methods extensions?
– t3chb0t
6 hours ago
@t3chb0t simple extensions fordecimaltype that add/remove tax from the price.
– Konrad
6 hours ago
ok, it'd be great if you could add them too so that people can copy/paste your code into an IDE and test it if they like ;-)
– t3chb0t
6 hours ago
1
@t3chb0t added code
– Konrad
5 hours ago
2
Now the first answer has been posted, please refrain from further editing the question. You're at risk of invalidating the existing answer, which is simply not done on Code Review. Please see what you may and may not do after receiving answers.
– Mast
3 hours ago
 |Â
show 2 more comments
up vote
4
down vote
favorite
1
up vote
4
down vote
favorite
1
1
I have a class that contains 2 properties of the same type. decimal NetAmount and decimal GrossAmount
I would like to initialize it using either GrossAmount or NetAmount and based on the one specified calculate the second one.
Which way is the most elegant and why? (parameter validation is omitted for brevity)
1
public class TaxedPrice
private TaxedPrice()
public decimal NetAmount get; private set;
public decimal GrossAmount get; private set;
public static TaxedPrice FromNet(decimal netAmount, decimal taxRate)
return new TaxedPrice
NetAmount = decimal.Round(netAmount, 2, MidpointRounding.AwayFromZero),
GrossAmount = decimal.Round(netAmount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero)
;
public static TaxedPrice FromGross(decimal grossAmount, decimal taxRate)
return new TaxedPrice
GrossAmount = decimal.Round(grossAmount, 2, MidpointRounding.AwayFromZero),
NetAmount = decimal.Round(grossAmount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero)
;
2
public class TaxedPrice
public TaxedPrice(decimal netAmount, decimal grossAmount, decimal taxRate)
if (grossAmount != default)
GrossAmount = decimal.Round(grossAmount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(grossAmount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else if (netAmount != default)
NetAmount = decimal.Round(netAmount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(netAmount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
else
throw new InvalidOperationException($"Either nameof(netAmount) or grossAmount must be set.");
public decimal NetAmount get;
public decimal GrossAmount get;
3
public class TaxedPrice
public enum Type
Gross,
Net
public TaxedPrice(decimal amount, Type type, decimal taxRate)
if (type == Type.Gross)
GrossAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(amount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else if (type == Type.Net)
NetAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(amount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
public decimal NetAmount get;
public decimal GrossAmount get;
4
public class TaxedPrice
public TaxedPrice(decimal amount, bool fromGross, decimal taxRate)
if (fromGross)
GrossAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(amount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else
NetAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(amount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
public decimal NetAmount get;
public decimal GrossAmount get;
How it looks like from caller's side:
// 1
var taxedPrice = TaxedPrice.FromNet(2.123m, 0.23m);
// 2
var taxedPrice = new TaxedPrice(2.123m, default, 0.23m); // uses the first one to calculate the second one
var taxedPrice2 = new TaxedPrice(2.123m, 1.11m, 0.23m); // uses the first one to calculate the second one
var taxedPrice3 = new TaxedPrice(default, 1.11m, 0.23m); // uses the second one to calculate the first one
// 3
var taxedPrice = new TaxedPrice(2.123m, TaxedPrice.Type.Net, 0.23m);
// 4
var taxedPrice = new TaxedPrice(2.123m, false, 0.23m);
Extensions for tax:
public static class TaxExtensions
public static decimal ApplyTax(this decimal netPrice, decimal taxRate)
return netPrice * (taxRate + 1);
public static decimal RemoveTax(this decimal grossPrice, decimal taxRate)
return grossPrice / (taxRate + 1);
Mapping perspective
In my upper layer I pass those prices in POCOs/DTOs like:
public class PriceDTO
public decimal NetAmount get; set;
public decimal GrossAmount get; set;
And I have to check there as well which one was passed to decide from which to calculate. So in case of 1 mapping would look like:
if (priceDto.GrossAmount != default)
return TaxedPrice.FromGross(priceDto.GrossAmount, taxRate);
else if (priceDto.NetAmount != default)
return TaxedPrice.FromNet(priceDto.NetAmount, taxRate);
else
// error
In case of 2 (no need to check in the mapping code)
return new TaxedPrice(priceDto.NetAmount, priceDto.GrossAmount, taxRate)
3 - there's a check as well
4 - same like 1 and 3
And I agree this could be a struct instead.
c# comparative-review finance
edited 16 mins ago
200_success
125k14145406
asked 6 hours ago
Konrad
1386
I have a class that contains 2 properties of the same type. decimal NetAmount and decimal GrossAmount
I would like to initialize it using either GrossAmount or NetAmount and based on the one specified calculate the second one.
Which way is the most elegant and why? (parameter validation is omitted for brevity)
1
public class TaxedPrice
private TaxedPrice()
public decimal NetAmount get; private set;
public decimal GrossAmount get; private set;
public static TaxedPrice FromNet(decimal netAmount, decimal taxRate)
return new TaxedPrice
NetAmount = decimal.Round(netAmount, 2, MidpointRounding.AwayFromZero),
GrossAmount = decimal.Round(netAmount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero)
;
public static TaxedPrice FromGross(decimal grossAmount, decimal taxRate)
return new TaxedPrice
GrossAmount = decimal.Round(grossAmount, 2, MidpointRounding.AwayFromZero),
NetAmount = decimal.Round(grossAmount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero)
;
2
public class TaxedPrice
public TaxedPrice(decimal netAmount, decimal grossAmount, decimal taxRate)
if (grossAmount != default)
GrossAmount = decimal.Round(grossAmount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(grossAmount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else if (netAmount != default)
NetAmount = decimal.Round(netAmount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(netAmount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
else
throw new InvalidOperationException($"Either nameof(netAmount) or grossAmount must be set.");
public decimal NetAmount get;
public decimal GrossAmount get;
3
public class TaxedPrice
public enum Type
Gross,
Net
public TaxedPrice(decimal amount, Type type, decimal taxRate)
if (type == Type.Gross)
GrossAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(amount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else if (type == Type.Net)
NetAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(amount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
public decimal NetAmount get;
public decimal GrossAmount get;
4
public class TaxedPrice
public TaxedPrice(decimal amount, bool fromGross, decimal taxRate)
if (fromGross)
GrossAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
NetAmount = decimal.Round(amount.RemoveTax(taxRate), 2, MidpointRounding.AwayFromZero);
else
NetAmount = decimal.Round(amount, 2, MidpointRounding.AwayFromZero);
GrossAmount = decimal.Round(amount.ApplyTax(taxRate), 2, MidpointRounding.AwayFromZero);
public decimal NetAmount get;
public decimal GrossAmount get;
How it looks like from caller's side:
// 1
var taxedPrice = TaxedPrice.FromNet(2.123m, 0.23m);
// 2
var taxedPrice = new TaxedPrice(2.123m, default, 0.23m); // uses the first one to calculate the second one
var taxedPrice2 = new TaxedPrice(2.123m, 1.11m, 0.23m); // uses the first one to calculate the second one
var taxedPrice3 = new TaxedPrice(default, 1.11m, 0.23m); // uses the second one to calculate the first one
// 3
var taxedPrice = new TaxedPrice(2.123m, TaxedPrice.Type.Net, 0.23m);
// 4
var taxedPrice = new TaxedPrice(2.123m, false, 0.23m);
Extensions for tax:
public static class TaxExtensions
public static decimal ApplyTax(this decimal netPrice, decimal taxRate)
return netPrice * (taxRate + 1);
public static decimal RemoveTax(this decimal grossPrice, decimal taxRate)
return grossPrice / (taxRate + 1);
Mapping perspective
In my upper layer I pass those prices in POCOs/DTOs like:
public class PriceDTO
public decimal NetAmount get; set;
public decimal GrossAmount get; set;
And I have to check there as well which one was passed to decide from which to calculate. So in case of 1 mapping would look like:
if (priceDto.GrossAmount != default)
return TaxedPrice.FromGross(priceDto.GrossAmount, taxRate);
else if (priceDto.NetAmount != default)
return TaxedPrice.FromNet(priceDto.NetAmount, taxRate);
else
// error
In case of 2 (no need to check in the mapping code)
return new TaxedPrice(priceDto.NetAmount, priceDto.GrossAmount, taxRate)
3 - there's a check as well
4 - same like 1 and 3
And I agree this could be a struct instead.
c# comparative-review finance
c# comparative-review finance
edited 16 mins ago
200_success
125k14145406
asked 6 hours ago
Konrad
1386
edited 16 mins ago
200_success
125k14145406
asked 6 hours ago
Konrad
1386
edited 16 mins ago
200_success
125k14145406
edited 16 mins ago
200_success
125k14145406
edited 16 mins ago
200_success
125k14145406
125k14145406
asked 6 hours ago
Konrad
1386
asked 6 hours ago
Konrad
1386
asked 6 hours ago
Konrad
1386
1386
There are some methods missing:RemoveTaxandApplyTax. Are these methods extensions?
– t3chb0t
6 hours ago
@t3chb0t simple extensions fordecimaltype that add/remove tax from the price.
– Konrad
6 hours ago
ok, it'd be great if you could add them too so that people can copy/paste your code into an IDE and test it if they like ;-)
– t3chb0t
6 hours ago
1
@t3chb0t added code
– Konrad
5 hours ago
2
Now the first answer has been posted, please refrain from further editing the question. You're at risk of invalidating the existing answer, which is simply not done on Code Review. Please see what you may and may not do after receiving answers.
– Mast
3 hours ago
 |Â
show 2 more comments
There are some methods missing:RemoveTaxandApplyTax. Are these methods extensions?
– t3chb0t
6 hours ago
@t3chb0t simple extensions fordecimaltype that add/remove tax from the price.
– Konrad
6 hours ago
ok, it'd be great if you could add them too so that people can copy/paste your code into an IDE and test it if they like ;-)
– t3chb0t
6 hours ago
1
@t3chb0t added code
– Konrad
5 hours ago
2
Now the first answer has been posted, please refrain from further editing the question. You're at risk of invalidating the existing answer, which is simply not done on Code Review. Please see what you may and may not do after receiving answers.
– Mast
3 hours ago
There are some methods missing:
RemoveTax and ApplyTax. Are these methods extensions?– t3chb0t
6 hours ago
There are some methods missing:
RemoveTax and ApplyTax. Are these methods extensions?– t3chb0t
6 hours ago
@t3chb0t simple extensions for
decimal type that add/remove tax from the price.– Konrad
6 hours ago
@t3chb0t simple extensions for
decimal type that add/remove tax from the price.– Konrad
6 hours ago
ok, it'd be great if you could add them too so that people can copy/paste your code into an IDE and test it if they like ;-)
– t3chb0t
6 hours ago
ok, it'd be great if you could add them too so that people can copy/paste your code into an IDE and test it if they like ;-)
– t3chb0t
6 hours ago
1
1
@t3chb0t added code
– Konrad
5 hours ago
@t3chb0t added code
– Konrad
5 hours ago
2
2
Now the first answer has been posted, please refrain from further editing the question. You're at risk of invalidating the existing answer, which is simply not done on Code Review. Please see what you may and may not do after receiving answers.
– Mast
3 hours ago
Now the first answer has been posted, please refrain from further editing the question. You're at risk of invalidating the existing answer, which is simply not done on Code Review. Please see what you may and may not do after receiving answers.
– Mast
3 hours ago
 |Â
show 2 more comments
2 Answers
2
active
oldest
votes
up vote
4
down vote
In my opinion, you should go for number 1 (with one small change).
Reasons for choosing this one:
- Easy to modify - you can add new
FromXmethod - Clear - it states exactly what do you calculate
- Doesn't require explanation beforehand to do some checking by consumer (as in number 2)
Drawback:
(big one) It might disrupt testing, mocking a static method is a nightmare (unless you can afford moles).
Taking this all into account, I'd do a little bit of overengeneering and create a factory(-ish) class to build you a TaxedPrice object. For example:
public class TaxedPriceFactory: ITaxedPriceFactory
public TaxedPrice CreateFromNet(decimal netAmount, decimal taxRate)
//body
public TaxedPrice CreateFromGross(decimal grossAmount, decimal taxRate)
// body
This should have all three of advantages I mentioned earlier and it is easy to use in tests now.
Addendum:
Consider making your TaxedPrice a immutable struct instead of a class with two read-only properties.
EDIT: class and method names changed as suggested by @t3chb0t
answered 5 hours ago
MaLiN2223
60519
1
I don't think this is overengineered. If you named itTaxedPriceFactoryandCreateFromNetit'd be exactly as books suggest it ;-)
– t3chb0t
5 hours ago
1
I agree that option 1 is the best option. Your comments about mocking statics is nonsense though. Both methods are deterministic and thus do not need mocking. Introducing a factory is therefore pure over-engineering in this case.
– David Arno
4 hours ago
@DavidArno I disagree, you wouldn't test theTaxPriceclass, you might need to test a class that depends on it. In static scenario you would need to figure out how calculate a specific output value, in mine you would just mock the output.
– MaLiN2223
1 hour ago
"you wouldn't test theTaxPriceclass". Quite the contrary: I'd definitely test that class. "In static scenario you would need to figure out how calculate a specific output value...". No figuring out required. AsTaxPriceis deterministic, it can be used to provide the reference values. That's the beauty of deterministic code: no mocking required.
– David Arno
1 hour ago
@DavidArno yes, I misspoke let me rephrase: The problem with static method is not about testing this specific class. About your other sentence: how would you go about a test to see how your UI reacts to a Gross value of 0.35163453 ? You would need to input the correct values for the static method to get 0.35163453, isn't that right?
– MaLiN2223
1 hour ago
 |Â
show 1 more comment
up vote
3
down vote
I'd personally go for option one as it's by far the simplest, as the names FromNet and FromGross give a clear indication of what the code is doing. That clarity is lost in the other examples.
However, there is room to improve things. Both methods perform the same basic calculation, resulting in four near-dentical expressions. So DRY can be applied here:
public class TaxedPrice
private TaxedPrice()
public decimal NetAmount get; private set;
public decimal GrossAmount get; private set;
public static TaxedPrice FromNet(decimal netAmount, decimal taxRate)
return new TaxedPrice
NetAmount = ApplyRounding(netAmount),
GrossAmount = ApplyRounding(netAmount.ApplyTax(taxRate))
;
public static TaxedPrice FromGross(decimal grossAmount, decimal taxRate)
return new TaxedPrice
GrossAmount = ApplyRounding(grossAmount),
NetAmount = ApplyRounding(grossAmount.RemoveTax(taxRate))
;
private decimal ApplyRounding(decimal rawValue)
=> decimal.Round(rawValue, 2, MidpointRounding.AwayFromZero)
answered 57 mins ago
David Arno
63849
To follow OP's pattern you could also makeApplyRoundingan extension method.
– MaLiN2223
47 mins ago
@MaLiN2223, if that same rounding formula is used throughout the code, then yes, I'd certainly do that.
– David Arno
41 mins ago
@MaLiN2223 in my project I actually have an extension method forRoundso I agree
– Konrad
41 mins ago
add a comment |Â
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
4
down vote
In my opinion, you should go for number 1 (with one small change).
Reasons for choosing this one:
- Easy to modify - you can add new
FromXmethod - Clear - it states exactly what do you calculate
- Doesn't require explanation beforehand to do some checking by consumer (as in number 2)
Drawback:
(big one) It might disrupt testing, mocking a static method is a nightmare (unless you can afford moles).
Taking this all into account, I'd do a little bit of overengeneering and create a factory(-ish) class to build you a TaxedPrice object. For example:
public class TaxedPriceFactory: ITaxedPriceFactory
public TaxedPrice CreateFromNet(decimal netAmount, decimal taxRate)
//body
public TaxedPrice CreateFromGross(decimal grossAmount, decimal taxRate)
// body
This should have all three of advantages I mentioned earlier and it is easy to use in tests now.
Addendum:
Consider making your TaxedPrice a immutable struct instead of a class with two read-only properties.
EDIT: class and method names changed as suggested by @t3chb0t
answered 5 hours ago
MaLiN2223
60519
1
I don't think this is overengineered. If you named itTaxedPriceFactoryandCreateFromNetit'd be exactly as books suggest it ;-)
– t3chb0t
5 hours ago
1
I agree that option 1 is the best option. Your comments about mocking statics is nonsense though. Both methods are deterministic and thus do not need mocking. Introducing a factory is therefore pure over-engineering in this case.
– David Arno
4 hours ago
@DavidArno I disagree, you wouldn't test theTaxPriceclass, you might need to test a class that depends on it. In static scenario you would need to figure out how calculate a specific output value, in mine you would just mock the output.
– MaLiN2223
1 hour ago
"you wouldn't test theTaxPriceclass". Quite the contrary: I'd definitely test that class. "In static scenario you would need to figure out how calculate a specific output value...". No figuring out required. AsTaxPriceis deterministic, it can be used to provide the reference values. That's the beauty of deterministic code: no mocking required.
– David Arno
1 hour ago
@DavidArno yes, I misspoke let me rephrase: The problem with static method is not about testing this specific class. About your other sentence: how would you go about a test to see how your UI reacts to a Gross value of 0.35163453 ? You would need to input the correct values for the static method to get 0.35163453, isn't that right?
– MaLiN2223
1 hour ago
 |Â
show 1 more comment
up vote
4
down vote
In my opinion, you should go for number 1 (with one small change).
Reasons for choosing this one:
- Easy to modify - you can add new
FromXmethod - Clear - it states exactly what do you calculate
- Doesn't require explanation beforehand to do some checking by consumer (as in number 2)
Drawback:
(big one) It might disrupt testing, mocking a static method is a nightmare (unless you can afford moles).
Taking this all into account, I'd do a little bit of overengeneering and create a factory(-ish) class to build you a TaxedPrice object. For example:
public class TaxedPriceFactory: ITaxedPriceFactory
public TaxedPrice CreateFromNet(decimal netAmount, decimal taxRate)
//body
public TaxedPrice CreateFromGross(decimal grossAmount, decimal taxRate)
// body
This should have all three of advantages I mentioned earlier and it is easy to use in tests now.
Addendum:
Consider making your TaxedPrice a immutable struct instead of a class with two read-only properties.
EDIT: class and method names changed as suggested by @t3chb0t
answered 5 hours ago
MaLiN2223
60519
1
I don't think this is overengineered. If you named itTaxedPriceFactoryandCreateFromNetit'd be exactly as books suggest it ;-)
– t3chb0t
5 hours ago
1
I agree that option 1 is the best option. Your comments about mocking statics is nonsense though. Both methods are deterministic and thus do not need mocking. Introducing a factory is therefore pure over-engineering in this case.
– David Arno
4 hours ago
@DavidArno I disagree, you wouldn't test theTaxPriceclass, you might need to test a class that depends on it. In static scenario you would need to figure out how calculate a specific output value, in mine you would just mock the output.
– MaLiN2223
1 hour ago
"you wouldn't test theTaxPriceclass". Quite the contrary: I'd definitely test that class. "In static scenario you would need to figure out how calculate a specific output value...". No figuring out required. AsTaxPriceis deterministic, it can be used to provide the reference values. That's the beauty of deterministic code: no mocking required.
– David Arno
1 hour ago
@DavidArno yes, I misspoke let me rephrase: The problem with static method is not about testing this specific class. About your other sentence: how would you go about a test to see how your UI reacts to a Gross value of 0.35163453 ? You would need to input the correct values for the static method to get 0.35163453, isn't that right?
– MaLiN2223
1 hour ago
 |Â
show 1 more comment
up vote
4
down vote
up vote
4
down vote
In my opinion, you should go for number 1 (with one small change).
Reasons for choosing this one:
- Easy to modify - you can add new
FromXmethod - Clear - it states exactly what do you calculate
- Doesn't require explanation beforehand to do some checking by consumer (as in number 2)
Drawback:
(big one) It might disrupt testing, mocking a static method is a nightmare (unless you can afford moles).
Taking this all into account, I'd do a little bit of overengeneering and create a factory(-ish) class to build you a TaxedPrice object. For example:
public class TaxedPriceFactory: ITaxedPriceFactory
public TaxedPrice CreateFromNet(decimal netAmount, decimal taxRate)
//body
public TaxedPrice CreateFromGross(decimal grossAmount, decimal taxRate)
// body
This should have all three of advantages I mentioned earlier and it is easy to use in tests now.
Addendum:
Consider making your TaxedPrice a immutable struct instead of a class with two read-only properties.
EDIT: class and method names changed as suggested by @t3chb0t
answered 5 hours ago
MaLiN2223
60519
In my opinion, you should go for number 1 (with one small change).
Reasons for choosing this one:
- Easy to modify - you can add new
FromXmethod - Clear - it states exactly what do you calculate
- Doesn't require explanation beforehand to do some checking by consumer (as in number 2)
Drawback:
(big one) It might disrupt testing, mocking a static method is a nightmare (unless you can afford moles).
Taking this all into account, I'd do a little bit of overengeneering and create a factory(-ish) class to build you a TaxedPrice object. For example:
public class TaxedPriceFactory: ITaxedPriceFactory
public TaxedPrice CreateFromNet(decimal netAmount, decimal taxRate)
//body
public TaxedPrice CreateFromGross(decimal grossAmount, decimal taxRate)
// body
This should have all three of advantages I mentioned earlier and it is easy to use in tests now.
Addendum:
Consider making your TaxedPrice a immutable struct instead of a class with two read-only properties.
EDIT: class and method names changed as suggested by @t3chb0t
answered 5 hours ago
MaLiN2223
60519
edited 5 hours ago
answered 5 hours ago
MaLiN2223
60519
answered 5 hours ago
MaLiN2223
60519
answered 5 hours ago
MaLiN2223
60519
60519
1
I don't think this is overengineered. If you named itTaxedPriceFactoryandCreateFromNetit'd be exactly as books suggest it ;-)
– t3chb0t
5 hours ago
1
I agree that option 1 is the best option. Your comments about mocking statics is nonsense though. Both methods are deterministic and thus do not need mocking. Introducing a factory is therefore pure over-engineering in this case.
– David Arno
4 hours ago
@DavidArno I disagree, you wouldn't test theTaxPriceclass, you might need to test a class that depends on it. In static scenario you would need to figure out how calculate a specific output value, in mine you would just mock the output.
– MaLiN2223
1 hour ago
"you wouldn't test theTaxPriceclass". Quite the contrary: I'd definitely test that class. "In static scenario you would need to figure out how calculate a specific output value...". No figuring out required. AsTaxPriceis deterministic, it can be used to provide the reference values. That's the beauty of deterministic code: no mocking required.
– David Arno
1 hour ago
@DavidArno yes, I misspoke let me rephrase: The problem with static method is not about testing this specific class. About your other sentence: how would you go about a test to see how your UI reacts to a Gross value of 0.35163453 ? You would need to input the correct values for the static method to get 0.35163453, isn't that right?
– MaLiN2223
1 hour ago
 |Â
show 1 more comment
1
I don't think this is overengineered. If you named itTaxedPriceFactoryandCreateFromNetit'd be exactly as books suggest it ;-)
– t3chb0t
5 hours ago
1
I agree that option 1 is the best option. Your comments about mocking statics is nonsense though. Both methods are deterministic and thus do not need mocking. Introducing a factory is therefore pure over-engineering in this case.
– David Arno
4 hours ago
@DavidArno I disagree, you wouldn't test theTaxPriceclass, you might need to test a class that depends on it. In static scenario you would need to figure out how calculate a specific output value, in mine you would just mock the output.
– MaLiN2223
1 hour ago
"you wouldn't test theTaxPriceclass". Quite the contrary: I'd definitely test that class. "In static scenario you would need to figure out how calculate a specific output value...". No figuring out required. AsTaxPriceis deterministic, it can be used to provide the reference values. That's the beauty of deterministic code: no mocking required.
– David Arno
1 hour ago
@DavidArno yes, I misspoke let me rephrase: The problem with static method is not about testing this specific class. About your other sentence: how would you go about a test to see how your UI reacts to a Gross value of 0.35163453 ? You would need to input the correct values for the static method to get 0.35163453, isn't that right?
– MaLiN2223
1 hour ago
1
1
I don't think this is overengineered. If you named it
TaxedPriceFactory and CreateFromNet it'd be exactly as books suggest it ;-)– t3chb0t
5 hours ago
I don't think this is overengineered. If you named it
TaxedPriceFactory and CreateFromNet it'd be exactly as books suggest it ;-)– t3chb0t
5 hours ago
1
1
I agree that option 1 is the best option. Your comments about mocking statics is nonsense though. Both methods are deterministic and thus do not need mocking. Introducing a factory is therefore pure over-engineering in this case.
– David Arno
4 hours ago
I agree that option 1 is the best option. Your comments about mocking statics is nonsense though. Both methods are deterministic and thus do not need mocking. Introducing a factory is therefore pure over-engineering in this case.
– David Arno
4 hours ago
@DavidArno I disagree, you wouldn't test the
TaxPrice class, you might need to test a class that depends on it. In static scenario you would need to figure out how calculate a specific output value, in mine you would just mock the output.– MaLiN2223
1 hour ago
@DavidArno I disagree, you wouldn't test the
TaxPrice class, you might need to test a class that depends on it. In static scenario you would need to figure out how calculate a specific output value, in mine you would just mock the output.– MaLiN2223
1 hour ago
"you wouldn't test the
TaxPrice class". Quite the contrary: I'd definitely test that class. "In static scenario you would need to figure out how calculate a specific output value...". No figuring out required. As TaxPrice is deterministic, it can be used to provide the reference values. That's the beauty of deterministic code: no mocking required.– David Arno
1 hour ago
"you wouldn't test the
TaxPrice class". Quite the contrary: I'd definitely test that class. "In static scenario you would need to figure out how calculate a specific output value...". No figuring out required. As TaxPrice is deterministic, it can be used to provide the reference values. That's the beauty of deterministic code: no mocking required.– David Arno
1 hour ago
@DavidArno yes, I misspoke let me rephrase: The problem with static method is not about testing this specific class. About your other sentence: how would you go about a test to see how your UI reacts to a Gross value of 0.35163453 ? You would need to input the correct values for the static method to get 0.35163453, isn't that right?
– MaLiN2223
1 hour ago
@DavidArno yes, I misspoke let me rephrase: The problem with static method is not about testing this specific class. About your other sentence: how would you go about a test to see how your UI reacts to a Gross value of 0.35163453 ? You would need to input the correct values for the static method to get 0.35163453, isn't that right?
– MaLiN2223
1 hour ago
 |Â
show 1 more comment
up vote
3
down vote
I'd personally go for option one as it's by far the simplest, as the names FromNet and FromGross give a clear indication of what the code is doing. That clarity is lost in the other examples.
However, there is room to improve things. Both methods perform the same basic calculation, resulting in four near-dentical expressions. So DRY can be applied here:
public class TaxedPrice
private TaxedPrice()
public decimal NetAmount get; private set;
public decimal GrossAmount get; private set;
public static TaxedPrice FromNet(decimal netAmount, decimal taxRate)
return new TaxedPrice
NetAmount = ApplyRounding(netAmount),
GrossAmount = ApplyRounding(netAmount.ApplyTax(taxRate))
;
public static TaxedPrice FromGross(decimal grossAmount, decimal taxRate)
return new TaxedPrice
GrossAmount = ApplyRounding(grossAmount),
NetAmount = ApplyRounding(grossAmount.RemoveTax(taxRate))
;
private decimal ApplyRounding(decimal rawValue)
=> decimal.Round(rawValue, 2, MidpointRounding.AwayFromZero)
answered 57 mins ago
David Arno
63849
To follow OP's pattern you could also makeApplyRoundingan extension method.
– MaLiN2223
47 mins ago
@MaLiN2223, if that same rounding formula is used throughout the code, then yes, I'd certainly do that.
– David Arno
41 mins ago
@MaLiN2223 in my project I actually have an extension method forRoundso I agree
– Konrad
41 mins ago
add a comment |Â
up vote
3
down vote
I'd personally go for option one as it's by far the simplest, as the names FromNet and FromGross give a clear indication of what the code is doing. That clarity is lost in the other examples.
However, there is room to improve things. Both methods perform the same basic calculation, resulting in four near-dentical expressions. So DRY can be applied here:
public class TaxedPrice
private TaxedPrice()
public decimal NetAmount get; private set;
public decimal GrossAmount get; private set;
public static TaxedPrice FromNet(decimal netAmount, decimal taxRate)
return new TaxedPrice
NetAmount = ApplyRounding(netAmount),
GrossAmount = ApplyRounding(netAmount.ApplyTax(taxRate))
;
public static TaxedPrice FromGross(decimal grossAmount, decimal taxRate)
return new TaxedPrice
GrossAmount = ApplyRounding(grossAmount),
NetAmount = ApplyRounding(grossAmount.RemoveTax(taxRate))
;
private decimal ApplyRounding(decimal rawValue)
=> decimal.Round(rawValue, 2, MidpointRounding.AwayFromZero)
answered 57 mins ago
David Arno
63849
To follow OP's pattern you could also makeApplyRoundingan extension method.
– MaLiN2223
47 mins ago
@MaLiN2223, if that same rounding formula is used throughout the code, then yes, I'd certainly do that.
– David Arno
41 mins ago
@MaLiN2223 in my project I actually have an extension method forRoundso I agree
– Konrad
41 mins ago
add a comment |Â
up vote
3
down vote
up vote
3
down vote
I'd personally go for option one as it's by far the simplest, as the names FromNet and FromGross give a clear indication of what the code is doing. That clarity is lost in the other examples.
However, there is room to improve things. Both methods perform the same basic calculation, resulting in four near-dentical expressions. So DRY can be applied here:
public class TaxedPrice
private TaxedPrice()
public decimal NetAmount get; private set;
public decimal GrossAmount get; private set;
public static TaxedPrice FromNet(decimal netAmount, decimal taxRate)
return new TaxedPrice
NetAmount = ApplyRounding(netAmount),
GrossAmount = ApplyRounding(netAmount.ApplyTax(taxRate))
;
public static TaxedPrice FromGross(decimal grossAmount, decimal taxRate)
return new TaxedPrice
GrossAmount = ApplyRounding(grossAmount),
NetAmount = ApplyRounding(grossAmount.RemoveTax(taxRate))
;
private decimal ApplyRounding(decimal rawValue)
=> decimal.Round(rawValue, 2, MidpointRounding.AwayFromZero)
answered 57 mins ago
David Arno
63849
I'd personally go for option one as it's by far the simplest, as the names FromNet and FromGross give a clear indication of what the code is doing. That clarity is lost in the other examples.
However, there is room to improve things. Both methods perform the same basic calculation, resulting in four near-dentical expressions. So DRY can be applied here:
public class TaxedPrice
private TaxedPrice()
public decimal NetAmount get; private set;
public decimal GrossAmount get; private set;
public static TaxedPrice FromNet(decimal netAmount, decimal taxRate)
return new TaxedPrice
NetAmount = ApplyRounding(netAmount),
GrossAmount = ApplyRounding(netAmount.ApplyTax(taxRate))
;
public static TaxedPrice FromGross(decimal grossAmount, decimal taxRate)
return new TaxedPrice
GrossAmount = ApplyRounding(grossAmount),
NetAmount = ApplyRounding(grossAmount.RemoveTax(taxRate))
;
private decimal ApplyRounding(decimal rawValue)
=> decimal.Round(rawValue, 2, MidpointRounding.AwayFromZero)
answered 57 mins ago
David Arno
63849
answered 57 mins ago
David Arno
63849
answered 57 mins ago
David Arno
63849
answered 57 mins ago
David Arno
63849
63849
To follow OP's pattern you could also makeApplyRoundingan extension method.
– MaLiN2223
47 mins ago
@MaLiN2223, if that same rounding formula is used throughout the code, then yes, I'd certainly do that.
– David Arno
41 mins ago
@MaLiN2223 in my project I actually have an extension method forRoundso I agree
– Konrad
41 mins ago
add a comment |Â
To follow OP's pattern you could also makeApplyRoundingan extension method.
– MaLiN2223
47 mins ago
@MaLiN2223, if that same rounding formula is used throughout the code, then yes, I'd certainly do that.
– David Arno
41 mins ago
@MaLiN2223 in my project I actually have an extension method forRoundso I agree
– Konrad
41 mins ago
To follow OP's pattern you could also make
ApplyRounding an extension method.– MaLiN2223
47 mins ago
To follow OP's pattern you could also make
ApplyRounding an extension method.– MaLiN2223
47 mins ago
@MaLiN2223, if that same rounding formula is used throughout the code, then yes, I'd certainly do that.
– David Arno
41 mins ago
@MaLiN2223, if that same rounding formula is used throughout the code, then yes, I'd certainly do that.
– David Arno
41 mins ago
@MaLiN2223 in my project I actually have an extension method for
Round so I agree– Konrad
41 mins ago
@MaLiN2223 in my project I actually have an extension method for
Round so I agree– Konrad
41 mins ago
add a comment |Â
Â
draft saved
draft discarded
Â
draft saved
draft discarded
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f204832%2freversible-tax-calculator-class%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
There are some methods missing:
RemoveTaxandApplyTax. Are these methods extensions?– t3chb0t
6 hours ago
@t3chb0t simple extensions for
decimaltype that add/remove tax from the price.– Konrad
6 hours ago
ok, it'd be great if you could add them too so that people can copy/paste your code into an IDE and test it if they like ;-)
– t3chb0t
6 hours ago
1
@t3chb0t added code
– Konrad
5 hours ago
2
Now the first answer has been posted, please refrain from further editing the question. You're at risk of invalidating the existing answer, which is simply not done on Code Review. Please see what you may and may not do after receiving answers.
– Mast
3 hours ago