Ayende @ Rahien

It's a girl

The tax calculation challenge

People seems to be more interested in answering the question than the code that solved it. Actually, people seemed to be more interested in outdoing one another in creating answers to that. What I found most interesting is that a large percentage of the answers (both in the blog post and in the interviews) got a lot of that wrong.

So here is the question in full. The following table is the current tax rates in Israel:

  Tax Rate
Up to 5,070 10%
5,071 up to 8,660 14%
8,661 up to 14,070 23%
14,071 up to 21,240 30%
21,241 up to 40,230 33%
Higher than 40,230 45%

Here are some example answers:

  • 5,000 –> 500
  • 5,800 –> 609.2
  • 9,000 –> 1087.8
  • 15,000 –> 2532.9
  • 50,000 –> 15,068.1

This problem is a bit tricky because the tax rate doesn’t apply to the whole sum, only to the part that is within the current rate.

Comments

Ayende Rahien
09/23/2011 09:44 AM by
Ayende Rahien

Thomas, Did it got cut off?

Thomas Levesque
09/23/2011 09:44 AM by
Thomas Levesque

Oops, my code was truncated because of the '<' sign... here it is on pastebin http://pastebin.com/NCmjVJE0

Patrick Huizinga
09/23/2011 09:56 AM by
Patrick Huizinga

Hmm, everything on this page is monospaced now...

Ayende, I hereby file a bug. :-)

Jonty
09/23/2011 10:10 AM by
Jonty

What's the tax rate for 5,070.5?

Daniel Lang
09/23/2011 10:11 AM by
Daniel Lang

Wow, tax rates in Isreal are freakin' high!

hazzik
09/23/2011 10:11 AM by
hazzik

https://gist.github.com/1237076 tem minutes

Timvw
09/23/2011 10:21 AM by
Timvw

At least the submitted answers use decimal instead of int..

@Daniel Lang: Stay away from Belgium then..

        0 to 7 900 euro 25 %
 7 900 to 11 240 euro   30 %

11 240 to 18 730 euro 40 % 18 730 to 34 330 euro 45 %

34 330 euro 50 %

Johan
09/23/2011 11:06 AM by
Johan

Stay away from Sweden too... :) Our taxes starts at 30 % and the sky is the limit...

Geert Baeyaert
09/23/2011 11:06 AM by
Geert Baeyaert

decimal GetTax(decimal amount) { var brackets = new[] { new TaxBracket(0, 5070, 0.1m), new TaxBracket(5070, 8660, 0.14m), new TaxBracket(8660, 14070, 0.23m), new TaxBracket(14070, 21240, 0.3m), new TaxBracket(21240, 40230, 0.33m), new TaxBracket(40230, decimal.MaxValue, 0.45m) };

return brackets.Sum(r => r.GetTaxInBracket(amount)); }

public class TaxBracket { public TaxBracket(decimal lower, decimal upper, decimal rate) { Lower = lower; Upper = upper; Rate = rate; }

public readonly decimal Lower; public readonly decimal Upper; public readonly decimal Rate;

public decimal GetTaxInBracket(int amount) { if (Lower > amount) return 0;

var taxable = Math.Min(amount - Lower, Upper - Lower);
return taxable * Rate;

} }

Stuart Cam
09/23/2011 11:22 AM by
Stuart Cam

I think Geert wins in terms of readability...

Andre Hühn
09/23/2011 11:25 AM by
Andre Hühn

You should stay away from germany. You would need a hole army of developers to calculate the tax rate because the laws are so freakin complex... :)

Diogo Mafra
09/23/2011 11:34 AM by
Diogo Mafra

The trick is: don't calculate the value for each rate, just calculate the value for the bigger rate and apply a fixed discount.

For example: 5800 -> 5800 * 0,14 - 202,84 (202,84 is the fixed discount) 9000 -> 9000 * 0,23 - 982,33

 Neil Barnwell
09/23/2011 12:01 PM by
Neil Barnwell

I know I didn't use any fancy linq tricks, which I'd probably do to make it more elegant, but this seems to pass the tests listed above: http://pastebin.com/zJADQGdx

I'm quite pleased, actually. I can never normally do this kind of thing; my brain isn't geared up for it.

Linkgoron
09/23/2011 12:10 PM by
Linkgoron

Well, my solution... Basically the same as all the others...

https://gist.github.com/1237185

Josh Schwartzberg
09/23/2011 12:21 PM by
Josh Schwartzberg

It's pretty funny how similar the ayende blog readers are writing their code. here's mine: https://gist.github.com/1237210

Arnon Rotem-Gal-Oz
09/23/2011 12:25 PM by
Arnon Rotem-Gal-Oz

Here's a Scala version http://mysticpaste.com/view/10034

Olg
09/23/2011 01:19 PM by
Olg

Here's mine : http://pastebin.com/A5fzvFqt

Steve Py
09/23/2011 01:26 PM by
Steve Py

Hmm, after reading the comments mine is similar but I didn't need the Lower bounds recorded. Ugliest bit was updating the upper boundary taxable amounts based on their previous amounts but it works (incl. income of 0 and decimal.max (I wish:)

https://gist.github.com/1237303

Spent 20 min on it, mostly trying to find a cleaner way to generate/update the tax brackets.

Abdelmoumen BOUABDALLAH
09/23/2011 01:41 PM by
Abdelmoumen BOUABDALLAH

Hi, How about this Python program: taxRates=[ { 'min':40230 , 'rate':0.45 }, { 'min':21240 , 'rate':0.33 }, { 'min':14070 , 'rate':0.3 }, { 'min':8660 , 'rate':0.23 }, { 'min':5070 , 'rate':0.14 }, { 'min':0 , 'rate':0.1 } ]

def taxRate(amount): tax=0; for taxRate in taxRates: if amount > taxRate['min']: tax = tax + round((amount - taxRate['min']) * taxRate['rate'],1) amount = taxRate['min'] return round(tax,1)

mike
09/23/2011 01:41 PM by
mike

my using components:

http://pastebin.com/VCqiwbuk

Avish
09/23/2011 01:45 PM by
Avish

My ruby implementation is naive and assumes you enter the tax steps' boundaries with relation to each other (instead of absolute upper bounds), but a proper design would use some kind of builder, where you can add steps one after the other and finish with a "final step" (I don't like this decimal.MaxValue thing, it's smelly). The resulting calculator would keep precalculated discounts like in Diogo's suggestion to avoid running through all the steps all the time.

mike
09/23/2011 01:53 PM by
mike

Are the solutions breaking o/c principle ? what if a new tax comes into play ?

Damian Powell
09/23/2011 02:00 PM by
Damian Powell

Wow, the Scala version is nice.

Got it working relatively neatly in C# after working out that I was moving the wrong bound to account for the inter-rate gap.

If this was real-life though, I'd be going to the local tax expert and asking them what happens with the unspecified shekels rather than trying to reverse engineer it.

Phil Bolduc
09/23/2011 02:03 PM by
Phil Bolduc

Ayende,

Is there a error in the problem statement? They way it is written states that if income falls between two tax rates, for example 5070.50, then no tax rate would apply. Does income get rounded to the nearest whole number before calculation? Or should the problem statement be changed to take into consideration fractional units of income?

Damian Powell
09/23/2011 02:03 PM by
Damian Powell

Wow! The scala version is nice.

Managed to get it working in C# relatively neatly after I realised that I was moving the wrong bound in order to account for the inter-rate gap.

http://pastebin.com/4qL8NHi9

If this was real life though, I'd probably just go to the local tax expert and ask them how the unspecified shekels should be accounted for, rather than trying to reverse engineer the rules.

Peter Paperno
09/23/2011 02:23 PM by
Peter Paperno

Here is my solution:

http://pastebin.com/ZtMd4gzJ

Any comments would be appreciated.

larrys
09/23/2011 02:23 PM by
larrys

use the Force, Luke...from LinqPAD:

int GetSlice(int salary, int startRange, int endRange) { if (salary <= startRange) return 0; if (salary >= endRange) return endRange - startRange; return salary - startRange; }

IEnumerable TaxTable(int salary) { yield return GetSlice(salary, 0, 5070) * .1m; yield return GetSlice(salary, 5070, 8660) * .14m; yield return GetSlice(salary, 8660, 14070) * .23m; yield return GetSlice(salary, 14070, 21240) * .30m; yield return GetSlice(salary, 21240, 40230) * .33m; yield return GetSlice(salary, 40230, Int32.MaxValue) * .45m; }

void Main() { TaxTable(5000).Aggregate((a,b) => a + b).Dump(); TaxTable(5800).Aggregate((a,b) => a + b).Dump(); TaxTable(9000).Aggregate((a,b) => a + b).Dump(); TaxTable(15000).Aggregate((a,b) => a + b).Dump(); TaxTable(50000).Aggregate((a,b) => a + b).Dump(); }

Valera Kolupaev
09/23/2011 02:40 PM by
Valera Kolupaev

Solution with nice small DSL :) http://jsfiddle.net/val2048/pY7Vm/1/

Ayende Rahien
09/23/2011 02:45 PM by
Ayende Rahien

Phil, That is how it is define by law. In practice, it means that you can safely ignore this. That is, you can say that if you are getting 5,070.5 that half a shekel isn't taxed.

Steve Py
09/23/2011 02:53 PM by
Steve Py

@Mike: "Are the solutions breaking o/c principle ?" Yes & No.

Most of the solutions are focused on building a reasonable code segment to solve a specific code problem.

Even given an application that had a similar requirement with fixed boundaries and no matching requirement to make boundary to add/remove/change boundaries (even if it looked plausable that this is something they'd want in the future) I personally would still implement something clean and simple like many of these solutions. Otherwise you're having to write code and unit tests (and/or include test cases) for non-requirements. If the customer never asks for it, it's a waste. If the customer needs it in the future then it should be kept easy to re-factor and they'd pay for the new requirements.

Phil Bolduc
09/23/2011 03:11 PM by
Phil Bolduc

Interesting, in Canda they use the wording "If amount is X or less then Y, If the amount is more than X but not more than W then Z"

Dmitry Ornatsky
09/23/2011 03:23 PM by
Dmitry Ornatsky

Ok, here you go. https://gist.github.com/1237614

Keith
09/23/2011 03:32 PM by
Keith

F#: http://pastebin.com/g9UDu5vF

alun
09/23/2011 03:36 PM by
alun

This would make an excellent codegolf challenge. Semi readable using linq: http://pastebin.com/vduGhdJY

Dmitry Ornatsky
09/23/2011 03:17 PM by
Dmitry Ornatsky

Here you go.

https://gist.github.com/1237614

alexl
09/23/2011 03:47 PM by
alexl

https://gist.github.com/1237707

another one

Dmitry Ornatsky
09/23/2011 04:37 PM by
Dmitry Ornatsky

Sorry for the duplicate comment. I wish there was a way to delete one.

David Fauber
09/23/2011 04:42 PM by
David Fauber
    private List<TaxBracket> _brackets = 
        new List<TaxBracket>
            {
                new TaxBracket{Min=int.MinValue,Max=5070,Rate=.1m},
                new TaxBracket{Min=5071,Max=8660,Rate=.14m},
                new TaxBracket{Min=8661,Max=14070,Rate=.23m},
                new TaxBracket{Min=14071,Max=21240,Rate=.3m},
                new TaxBracket{Min=21241,Max=40230,Rate=.33m},
                new TaxBracket{Min=40231,Max=int.MaxValue,Rate=.45m}
            };

    /// for crazy reuse potential uncomment this guy and set your own brackets!
    /// public List<TaxBracket> Brackets { get { return _brackets; } set { _brackets = value; } }

    public decimal CalculateTax (int income)
    {
        decimal tax = 0m;
        for (int i = 1; i <= income; i++)
        {
            tax += _brackets.Single(s => s.Max >= i && s.Min <= i).Rate;
        }
        return tax;
    }
}

public class TaxBracket
{
    public int Min { get; set; }
    public int Max { get; set; }
    public decimal Rate { get; set; }
}
David Fauber
09/23/2011 04:43 PM by
David Fauber

Apparently I lost some lines in my cut/pasting...this should be at the top:

public class TaxResolver
{
David Fauber
09/23/2011 04:50 PM by
David Fauber

called like this in case its not obvious, sorry for triple-posting,should've just paste-bin'd this...

    static void Main()
    {
        var resolver = new TaxResolver();
        Console.WriteLine(resolver.CalculateTax(5000));
        Console.WriteLine(resolver.CalculateTax(5800));
        Console.WriteLine(resolver.CalculateTax(9000));
        Console.WriteLine(resolver.CalculateTax(15000));
        Console.WriteLine(resolver.CalculateTax(50000));
    }

(yes I'm lame and did a console app instead of wiring up tests)

Demis Bellot
09/23/2011 05:00 PM by
Demis Bellot

F# solution from yesterday: Gist at: https://gist.github.com/1236106

let taxOf salary taxRates = 
	((0m,0)::taxRates, taxRates) 
		||> Seq.zip 
		 |> Seq.map(fun ((_, prevBand),(rate, band)) -> (prevBand, rate, band))
		 |> Seq.sumBy(fun (prevBand, rate, band) -> 
			match salary with 
				| x when x < prevBand -> 0m
				| x when x > band -> decimal(band - prevBand) * rate
				| x -> decimal(x - prevBand) * rate
			)

//define custom tax bands and rates
let israelTaxRates = [
	0.10m, 5070; 
	0.14m, 8660; 
	0.23m, 14070; 
	0.30m, 21240; 
	0.33m, 40230; 
	0.45m, System.Int32.MaxValue]

taxOfIsrael 5800 = 609.20

Demis Bellot
09/23/2011 05:02 PM by
Demis Bellot

with missing important piece included :)

//use currying to build a higher order function to calculate US Tax Rates
let taxOfIsrael salary = israelTaxRates |> taxOf salary
James
09/23/2011 06:39 PM by
James

Here's a fairly straightforward option in c#:

static readonly Dictionary<decimal, decimal> tiers = new Dictionary<decimal, decimal> { {40230, .45m},
{21240, .33m}, {14070, .30m}, {8660, .23m}, {5070, .14m}, {0, .10m} };

static decimal CalculateTax(decimal salary) { decimal salaryRemaining = salary; decimal tax = 0; while (salaryRemaining > 0) { var tier = tiers.Where(t => salaryRemaining > t.Key).First(); var rate = tier.Value; var amountToTax = (salaryRemaining - tier.Key); tax += rate * amountToTax; salaryRemaining -= amountToTax; } return tax; }

Jamie
09/23/2011 07:01 PM by
Jamie

Every single one of these answers is wrong. Where are the tests?

Josh
09/23/2011 07:39 PM by
Josh

I wrote a solution using TDD. http://technofattie.blogspot.com/2011/09/solving-ayendes-tax-woes.html

Now that I am looking over the other solutions it is clear that a lot of people solved it in a very similar way. Oh well, it was a nice break from normal work for a little bit.

tobi
09/23/2011 09:18 PM by
tobi

"Hmm, everything on this page is monospaced now..." That is because this blog uses HTML blacklisting sanitization instead of encoding it. Blacklisting is nearly always wrong (because buggy), encoding is nearly always the right solution. Sry for banging on this point but this issue drives me nuts. I event went through whole SO and commented on every wrong sanitization code example. It is just wrong.

To make this more concrete: Your regex based blacklisting does not catch imbalanced tags or singe lt/gt chars. It does not catch unclosed tags either. That is because it is humanly impossible to write correct blacklisting code ;-)

Eric
09/23/2011 09:19 PM by
Eric

As someone who considers themselves a novice and definitely in the "would be applying to beginner positions", how is this code? http://pastebin.com/VirBFdtB

I don't know, seemed like a fun problem. My solution isn't as elegant as many of the others, but I'm curious if it would be considered a turnoff to an employer.

Rich
09/23/2011 11:28 PM by
Rich

@Jamie -- every single one eh? Did you even look at them all? The second one added (hazzik's) had tests. So did Damien Powell's example. Get off your high horse and look at them before making a blanket statement like that.

Steve Sheldon
09/24/2011 12:19 AM by
Steve Sheldon

Another way to look at this problem. If I have a salary of 50,000... The answer is calculated as:

(50000 - 40230) * .45 + 10671.6 = 15068.1

So I wrote this is a series of rules, check for a match and calculate tax. Could even convert this into a Boo DSL.

http://pastebin.com/yzxRqSx0

I've been working for a big payroll house the past two years. We didn't calculate tax, as theire are third party libraries for that sort of thing, but I did enough excel spreadsheets for creating test data to know how tax is calculated.

Steve Py
09/24/2011 01:29 AM by
Steve Py

Readability is a personal preference thing, but I decided to take a bunch of the C# samples and benchmark their performance and test that they actually worked... Here are the results:

Timed tests...

Steve Py x 5000: 9.0005ms Alexander x 5000: 7.0004ms Damien x 5000: 23.0013ms Dmitry x 5000: 8.0004ms Eric x 5000: 8.0005ms Geert x 5000: 13.0007ms Link Goron x 5000: 9.0005ms Peter x 5000: 12.0007ms Thomas x 5000: 9.0005ms Steve Py x 100000: 103.0059ms Alexander x 100000: 97.0056ms Damien x 100000: 375.0215ms Dmitry x 100000: 128.0073ms Eric x 100000: 146.0083ms Geert x 100000: 225.0129ms Link Goron x 100000: 140.008ms Peter x 100000: 173.0099ms Thomas x 100000: 150.0085ms Steve Py x 1000000: 1014.058ms Alexander x 1000000: 972.0556ms Damien x 1000000: 3782.2164ms Dmitry x 1000000: 1321.0755ms Eric x 1000000: 1459.0835ms Geert x 1000000: 2281.1304ms Link Goron x 1000000: 1411.0808ms Peter x 1000000: 1714.098ms Thomas x 1000000: 1504.086ms

Unit tests...

Steve Py... Steve Py x Passed! Alexander... Alexander x Passed! Damien... Damien x Passed! Dmitry... Dmitry x Passed! Eric... Eric x Passed! Geert... Geert x Passed! Goron... Goron x Passed! Peter... Peter x Passed! Thomas... Thomas x Passed!

Sorry to anyone that had c# examples that I missed...

The unit tests section just ran through Ayende's examples plus $0. Since most of the samples used decimal, I converted the double/int versions to decimal for consistency of test sets. This was just measured with DateTime & Timespan checks not a precision timer so it's not going to be terribly accurate. There were swings between test runs but the performance comparison was pretty-much the same.

Notables: Alexander's was noticably better performing than the rest, and all passed the test scenarios.

Damien's sample nearly ended up with an all-fail by a factor of 100 until I spotted he chose to pass tax rates as percentages.

Geert's sample didn't compile as provided. 2 minor adjustments needed. Additionally Geert's sample was improved a bit by extracting out the set of taxbrackets as a member variable instead of each time the method was called: Geert x 1000000: 2281.1304ms to Geert x 1000000: 1944.1112ms

This brought him a bit closer in-line with the other samples.

Demis Bellot
09/24/2011 04:27 AM by
Demis Bellot

I think calculating benchmarks for something like this is a bit pointless as the fastest algorithm is also likely to be the hardest to maintain. So just for kicks I created a fast version of in C# - which funnily enough is similar in spirit to the interviewee's submission :)

Full Gist at: https://gist.github.com/1238962

public static decimal CalcTaxFast(int salary) { const int band1 = 5070; const decimal rate1 = 0.10m; const int band2 = 8660; const decimal rate2 = 0.14m; const int band3 = 14070; const decimal rate3 = 0.23m; const int band4 = 21240; const decimal rate4 = 0.30m; const int band5 = 40230; const decimal rate5 = 0.33m;

const decimal rate6 = 0.45m;

const decimal bandTax1 = band1 * rate1;
const decimal bandTax2 = (band2 - band1) * rate2 + bandTax1;
const decimal bandTax3 = (band3 - band2) * rate3 + bandTax2;
const decimal bandTax4 = (band4 - band3) * rate4 + bandTax3;
const decimal bandTax5 = (band5 - band4) * rate5 + bandTax4;

if (salary &gt; band5) 
    return (salary - band5) * rate6 + bandTax5;
if (salary &gt; band4) 
    return (salary - band4) * rate5 + bandTax4;
if (salary &gt; band3) 
    return (salary - band3) * rate4 + bandTax3;
if (salary &gt; band2) 
    return (salary - band2) * rate3 + bandTax2;
if (salary &gt; band1) 
    return (salary - band1) * rate2 + bandTax1;

return salary * rate1;

}

P.S: For ultimate perf, someone should rewrite this in ASM :)

Steve Py
09/24/2011 05:29 AM by
Steve Py

Correction: Oddly enough Damien's test failed when working with decimals and passing the decimal rate (I.e. .23) while removing the / 100.

Damien x Failed. : $5800, expected $609.2, actual $609.34| $9000, expected $1087.8, actual $1088.17| $15000, expected $2532.9, actual $2533.57| $50000, expected $15068.1, actual $15069.55

It seems the rounding may have been concealing a bit of a bug. (or I may have lost something in translation.) One thing that did look a bit odd was that there was overlap between ranges. (5070-8660, 8660-14070) This may be throwing off the calculation.

For fun when I was looking over Peter's example (which is quite a bit of code to maintain) it got me thinking perhaps a double - linked list might work, but the best I could get out of it without it looking obfuscated was still slightly slower than my original.

Additionally I made one pretty obvious mistake in the test sets: I had it generating a random range of values from $0-3M. This is going to result in a very disproportionate # of values falling into the upper tax bracket. I revised the generation to between $0-100k and the results changed a bit. Apparently Alexander's solution handles a lot of rich people better than mine. The extra load I had at start up though pays off in the larger # of requests.

I also incorporated Frank's sample from the previous post since that was one lean looking piece of code. It covered all of the test cases however the performance was surprisingly unimpressive:

Steve Py x 5000: 8.0004ms Steve Py 2 x 5000: 9.0006ms Alexander x 5000: 7.0004ms Damien x 5000: 17.0009ms Dmitry x 5000: 7.0004ms Eric x 5000: 7.0004ms Geert x 5000: 12.0007ms Link Goron x 5000: 8.0005ms Peter x 5000: 12.0007ms Thomas x 5000: 9.0005ms Frank x 5000: 8.0004ms Frank x 5000: 7.0004ms Steve Py x 100000: 95.0054ms Steve Py 2 x 100000: 95.0054ms Alexander x 100000: 101.0058ms Damien x 100000: 237.0135ms Dmitry x 100000: 114.0065ms Eric x 100000: 127.0073ms Geert x 100000: 204.0117ms Link Goron x 100000: 129.0074ms Peter x 100000: 163.0093ms Thomas x 100000: 144.0083ms Frank x 100000: 150.0085ms Frank x 100000: 129.0074ms Steve Py x 1000000: 921.0527ms Steve Py 2 x 1000000: 935.0534ms Alexander x 1000000: 1076.0616ms Damien x 1000000: 2413.138ms Dmitry x 1000000: 1145.0655ms Eric x 1000000: 1278.0731ms Geert x 1000000: 2079.1189ms Link Goron x 1000000: 1298.0742ms Peter x 1000000: 1637.0937ms Thomas x 1000000: 1381.079ms Frank x 1000000: 1464.0837ms

but the original was declaring the arrays in each call, a quick tweak turned: Frank x 1000000: 1464.0837ms to: Frank x 1000000: 1261.0721ms

In any case, not a concern for such a slick little bit of code.

Demis Bellot
09/24/2011 09:46 AM by
Demis Bellot

Hi @Steve Py

Out of curiosity what numbers do you get if you plug CalcTaxFast from: https://gist.github.com/1238962 in?

Roberto
09/24/2011 11:06 AM by
Roberto

Just another C# solution: https://gist.github.com/9cbe9ab68f75f52eebfb

Bruno
09/24/2011 04:53 PM by
Bruno

Java solution. It's slightly different from others. I used a factory to create calculators and some parameter validations (like a precondition). I think that practices are important, even in naive implementations. https://gist.github.com/1239534

Daniel Lidström
09/24/2011 06:28 PM by
Daniel Lidström

Here's the simplest solution I could come up with:

private double TaxFor(double amount)
{
    if (amount > 40230)
        return 0.45 * (amount - 40230) + 10671.6 /*TaxFor(40230)*/;
    else if (amount > 21240)
        return 0.33 * (amount - 21240) + 4404.9 /*TaxFor(21240)*/;
    else if (amount > 14070)
        return 0.30 * (amount - 14070) + 2253.9 /*TaxFor(14070)*/;
    else if (amount > 8660)
        return 0.23 * (amount - 8660) + 1009.6 /*TaxFor(8660)*/;
    else if (amount > 5070)
        return 0.14 * (amount - 5070) + 507 /*TaxFor(5070)*/;
    else
        return 0.10 * amount;
}
Steve Py
09/25/2011 12:54 AM by
Steve Py

Regarding the benchmarking: The idea was not to try and come up with the fastest solution, it was to measure the potential performance impact of what people thought were good solutions. The goal of the test should be that

1. (Obviously) the code compiles and runs.

2. The code passes a set of test scenarios.

3. The code is reasonably well structured and easy to follow.

Depending on the whims of the project lead they are bound to look for other things such as it being easy to extend or swap out interest rates, validates itself robustly, is unit tested effectively, is efficient, etc. The idea of a benchmark isn't always to make things faster, it can be to measure the potential trade off in performance to try making something better.

If anyone wants to play around with the benchmark and add their own sample that I missed, it is available for download at: http://www.mediafire.com/?d91x97jvzx14jd6

Mark
09/26/2011 02:45 AM by
Mark

In Australia, progressive tax is usually specified by the tax office as:

(Between {min} and {max}, {base amount} plus {% on each $ above min}.)+

Which has the nice side effect of the primary source material easily translating into a ordered list which makes a LINQ query a readable SkipWhile, First.

Timothy Walters
09/26/2011 03:35 AM by
Timothy Walters

While most of the answers are quite "clever" and show an understanding of some nice coding methods, I would pick Daniel Lidström's answer if I was hiring.

It's short, to the point, simple, easy to follow, and shows a greater understanding of the customer's needs than the programmer's needs.

Tax for the portion below each bracket can be pre-calculated, it's static, you simply have to tax the remainder.

Of course it would probably load the rates from a database with pre-calculated values (that should be calculated on data entry instead of on use), but there was no requirement for that in the "test" above. This could be easily refactored later to allow for this.

Alex
09/25/2011 09:12 PM by
Alex

I compulsively order from small to large, hence the Reverse :D Not the most efficient but nice and small. https://gist.github.com/1241154

Simon Cousins
09/26/2011 11:01 AM by
Simon Cousins

Most things turn out to be a fold:


let taxes salary =
    let rates = [40230.0m,0.45m;21240.0m,0.33m;14070.0m,0.3m;8660.0m,0.23m;5070.0m,0.14m;0.0m,0.1m]
    let taxAtRate (tax,remainder) (threshold,rate) =
        let taxable = remainder - threshold
        if taxable > 0.0m 
        then tax + rate * taxable, remainder - taxable 
        else tax,remainder
    List.fold taxAtRate (0.0m, salary) rates |> fst
Colin Bull
09/26/2011 11:18 AM by
Colin Bull

@Simon,

This function can actually be generalised to deal with any ranges of objects without any code changes the only thing required is that the objects passed in as rates support comparison (IComparable) and the addition operator. It is only the naming that makes this function specific to taxes.

Of couse this could be done in C# aswell through generics but the ceremony is far higher.

Robert
09/26/2011 12:06 PM by
Robert

@Bruno, you're fired.

Joe
09/26/2011 05:51 PM by
Joe

public class TaxCalc { public static decimal Calculate(int salary) { var brackets = new List{ new TaxBracket(0, 5070, 10), new TaxBracket(5000, 6000, 10), new TaxBracket(5071, 8660, 14), new TaxBracket(8661, 14070, 23), new TaxBracket(14071, 21240, 30), new TaxBracket(21241, 40230, 33), new TaxBracket(40231, Decimal.MaxValue, 45) };

    return brackets.Sum (b => b.GetTax(salary));

}

}

public class TaxBracket { public decimal MinValue {get;set;} public decimal MaxValue {get;set;} public decimal Rate {get;set;}

public TaxBracket(decimal min, decimal max, decimal rate)
{
    MinValue = min;
    MaxValue = max;
    Rate = rate;
}

public decimal GetTax(int salary) 
{
    var nums = Enumerable.Range(1, salary);
    var taxableDollars = nums.Where(n => n >= MinValue &&  n <= MaxValue);
    return taxableDollars.Count() * (Rate /  100);
}

}

Adam Langley
09/26/2011 09:42 PM by
Adam Langley
  1. I'd avoid using a class to encapsulate such simple data unless I was returning/consuming that class... (I might use an anonymous class though, purely for readability).
  2. Linq-be-gone... I personally feel it reduces the readability of something so simple...
  3. Sure, some iterations are unnecessary given your tax bracket, and they could be skipped using conditions, but whats the point? A couple of extra CPU cycles and the code is much simpler. Of course, this isnt written with the intention of calculating a million samples per second....

readability<--->performance. Just my 2c.

http://pastebin.com/Ezi5BL7u

Ken Tong
09/27/2011 09:33 AM by
Ken Tong

Let me join the party. My approach is to allow more trivial tax rate setting. But the implementation is a bit tricky.

http://pastebin.com/YkUB8RvG

Nicola Baldi
09/27/2011 12:42 PM by
Nicola Baldi

And now, my super-over-engineered (and quite useless) solution!

http://nbaldi.codeplex.com/SourceControl/list/changesets

David Cuccia
09/27/2011 07:39 PM by
David Cuccia

To be the gadfly: there is no question. Just saying.

ZyZ
09/27/2011 08:45 PM by
ZyZ
    public static decimal CalculateTax(decimal amount)
    {
        var rates = new Dictionary<decimal, int> { { 40230, 45 }, { 21240, 33 }, { 14070, 30 }, { 8660, 23 }, { 5070, 14 }, { 0, 10 } };
        return rates.Where(rate => rate.Key <= amount).Sum(rate =>
        {
            var tmp = amount;
            amount = rate.Key;
            return (tmp - rate.Key) * rate.Value * 0.01m;
        });
    }
Yan Cui
09/28/2011 12:11 PM by
Yan Cui

F#: https://gist.github.com/1247782

Andrew
09/28/2011 05:16 PM by
Andrew

From C# Program in LINQPad

http://pastebin.com/gmwKbwws

Keith Baker
09/28/2011 07:36 PM by
Keith Baker

My solution - https://gist.github.com/1248932

Bruno
09/29/2011 12:26 PM by
Bruno

@Robert Thanks for read my implementation. Please, give me a more detailed feedback. It's important to my learning.

JJoos
10/07/2011 11:39 AM by
JJoos

I over engineered a C# solution: http://pastebin.com/VDQ6VFhY . It's based on Geert Baeyaert solution.

jurkow
10/18/2011 08:54 PM by
jurkow
    private static readonly double[] TaxTable = new double[] { 0, 0.1, 0.14, 0.23, 0.3, 0.33, 0.45 };
    private static readonly double[] T = new double[] { 0, 5070, 8660, 14070, 21240, 40230 };
    public static double Tax(double value, int t = 0, double tax = 0)
    {
        return (++t < T.Count() && value > T[t] - T[t - 1]) ? Tax(value - T[t] + T[t - 1], t, tax + (T[t] - T[t - 1]) * TaxTable[t]) : tax + value * TaxTable[t];
    }
Comments have been closed on this topic.