Income Tax Calculator using 1913 tax standards

Question

I am a beginner in a class and recently given a problem where I have to design a income tax calculator but using the 1913 tax standards. I was able to get it down I think but I was just wondering if it at all looks sloppy or if there is any productive criticism people are willing to give.

Have I over complicated things for myself?

#include <iostream>
using namespace std; 
int main() {

double income;
cout << "what is your income?: ";
cin >> income;

//tax brackets for 1913. each number aside tax in the variable tax is the perecntage 
that number gets taxed at 
double taxBracket1 = 20000;
double taxBracket2 = 50000;
double taxBracket3 = 75000;
double taxBracket4 = 100000;
double taxBracket5 = 250000;
double taxBracket6 = 500000;
double TaxTotal = 0.0;
double TaxTotal2 = 0.0;
double TaxTotal3 = 0.0;
double TaxTotal4 = 0.0;
double TaxTotal5 = 0.0;
double TaxTotal6 = 0.0;
double TaxTotalBracket2;

if (income <= taxBracket1){
    TaxTotal = income * 0.01;
}
    else if (income <= taxBracket2){ //for each time you move up a tax bracket you are 
taxed the same as the previous brakcet. 
        TaxTotalBracket2 = (income - taxBracket1) * 0.02; 
        TaxTotal2 = taxBracket1 * 0.01;
        TaxTotal = TaxTotal2 + TaxTotalBracket2;
    }
    else if ((income <= taxBracket3)  & (income > taxBracket2)){ // the step happens for 
every tax bracket you fall into.
        TaxTotalBracket2 = (income - taxBracket2) * 0.03;
        TaxTotal3 = (taxBracket2 - taxBracket1) * 0.02;
        TaxTotal2 = taxBracket1 * 0.01;
        TaxTotal = TaxTotal2 + TaxTotalBracket2  + TaxTotal3; 
    }
    else if ((income <= taxBracket4) & (income >taxBracket3)) {
        TaxTotalBracket2 = (income - taxBracket3) * 0.04;
        TaxTotal4 = (taxBracket3 - taxBracket2) * 0.03;
        TaxTotal3 = (taxBracket2 - taxBracket1) * 0.02;
        TaxTotal2 = taxBracket1 * 0.01;
        TaxTotal =  TaxTotal4 + TaxTotal3 + TaxTotal2 + TaxTotalBracket2;
    }
    else if ((income <= taxBracket5) & (income > taxBracket4)) {
        TaxTotalBracket2 = (income - taxBracket4) * 0.05;
        TaxTotal5 = (taxBracket4 - taxBracket3) * 0.04;
        TaxTotal4 = (taxBracket3 - taxBracket2) * 0.03;
        TaxTotal3 = (taxBracket2 - taxBracket1) * 0.02;
        TaxTotal2 = taxBracket1 * 0.01; 
        TaxTotal = TaxTotal5 + TaxTotal4 + TaxTotal3 + TaxTotal2 + TaxTotalBracket2;
    }
    
    else if ((income <= taxBracket6) & (income > taxBracket5)) {
        TaxTotalBracket2 = (income - taxBracket5) * 0.06;
        TaxTotal6 = (taxBracket5 - taxBracket4) * 0.05;
        TaxTotal5 = (taxBracket4 - taxBracket3) * 0.04;
        TaxTotal4 = (taxBracket3 - taxBracket2) * 0.03;
        TaxTotal3 = (taxBracket2 - taxBracket1) * 0.02;
        TaxTotal2 = taxBracket1 * 0.01; 
        TaxTotal = TaxTotal6 + TaxTotal5 + TaxTotal4 + TaxTotal3 + TaxTotal2 + 
TaxTotalBracket2;
    }
cout << TaxTotal;
}

Answer 1

Use a data-driven design

You have lots of code duplication. You can remove that and simplify your code by using a more data-driven design. First, create a struct that groups together a bracket's limit and the tax fraction:

struct Bracket {
    double limit;
    double fraction;
};

Then create an array for the tax brackets:

static const Bracket taxBrackets[] = {
    { 20000, 0.01},
    { 50000, 0.02},
    { 75000, 0.03},
    {100000, 0.04},
    {250000, 0.05},
    {500000, 0.06},
};

Now you can create a loop that goes over all the brackets:

double remaining = income;
double taxTotal = 0;
double previousLimit = 0;

for (auto& bracket: taxBrackets) {
    double taxableAmount = std::min(remaining, bracket.limit - previousLimit);
    taxTotal += taxableAmount * bracket.fraction;
    remaining -= taxableAmount;
    previousLimit = bracket.limit;
}

Notice how you can now easily modify, add and remove tax brackets by changing the contents of the array taxBrackets[], without having to change any other line of code.

Answer 2

Make Sure this Meets the Requirements

You don’t give a specification for what the algorithm is supposed to do, or any test cases, so it’s impossible to tell whether it has any bugs or not. However, it does not appear to match the actual income tax code of 1913.

Don’t Import All of namespace std

This comes up here a lot, and every time, everybody goes over the reasons why you shouldn’t.. One is: if you import someone else’s namespace, and someone else then adds something to it that has the same name as something in your code, your program could suddenly break. If you’re lucky, the compiler might be able to figure out why, and a maintainer trying to fix it will be able to tell when foo means std::foo and when it means ::foo. On the other hand, if you import only what you need, like using std::cin, std::cout;, or if you always write out std::, you’re safe.

The whole reason namespace std was created in the first place was that it would otherwise be impossible to write a program that conformed to Standard C++: a programmer would have no idea what names might be added to the system headers in the future. If you just dump all the names from std::, you put yourself back in that situation and defeat the entire purpose.

(Tiny little solo projects you’ll never actually need to maintain or use again can get away with this—and therefore, doing it makes you look like an amateur.)

Use Fixed-Point for Financial Math

This might be overkill for an exercise like this, but double-precision floating-point numbers start getting round-off errors past the fifteenth digit. That’s not quite enough to calculate the national debt to the penny.

A more-robust way to store the number would be in fixed-point, that is, as the numerator of a fraction with a constant denominator. That would be like storing $1,234,567.89 in cents as 123456789LL, except you need to store at least one more digit of precision, so that calculations round off correctly to the nearest cent. So you might actually store it in mills, or thousandths of a dollar, as 1234567890LL. A few real-world financial transactions are even denominated in ten-thousandths of a dollar (decimills or milrays). Whatever denominator you choose, can use exact, and fast, integer math on the numbers.

It’s good practice to make this explicit, by declaring the denomination as a type, and maybe even giving your variables names like taxBracketMills that contain the denomination:

using mills = long long int;
constexpr mills oneMillionDollarsAndChange = 1234567890LL; // $1,234,567.89

Format Your Output Correctly

Tf you send the number 123.40 to cout, it will display as 123.4. That’s not correct for dollars and cents.

If you’re storing the number as a double, the easiest way to format it is std::locale::money_put. (This program is simple enough that you don’t need to worry about switching locales at runtime.) Otherwise, you could calculate the correct value of std::ostream::setprecision.

If you’re storing it as a fixed-point number, and don’t want to round it off by converting to floating-point, you’d want to use something like:

constexpr mills millsPerDollar = 1000;
constexpr mills millsPerCent = 10;

/* ... */
{
    const mills dollars = x/millsPerDollar;
    const mills centsRounded = x >= 0 ? (x%millsPerDollar + millsPerCent/2)/millsPerCent
                                      : -(x%millsPerDollar - millsPerCent/2)/millsPerCent;

And then manipulate the state of your ostream to display the dollar part of the amount (which might be negative), followed by a point, and the cents (normalized above to always be positive), in a field of width two left-padded with zeroes.

(Note that the calculation of centsRounded is only guaranteed to work on C++11 or later. Until 2008, the behavior of % on a negative operand was implementation-defined.)

Use constexpr and const where Appropriate

You have a number of variables that could be static single assignments, or even compile-time constants, but aren't. Declaring const, constexpr or consteval when possible helps the compiler catch bugs and optimize.

Streamline Your Algorithm

First, you have six or seven tax brackets that could be stored in a constexpr array, and another six or seven marginal rates (which your comment describes in a very confusing way) that could be stored the same way.

At that point, you want to write the code one of a few ways:

An if/else if/else structure: this would find which bracket the taxpayer is in and apply the correct calculation. It always has the form:

(taxableIncome - taxBracket[i])*marginalRate[i] + baseTax[i]

where baseTax is a running sum of the base tax paid at the bottom of each tax bracket.

A Loop: You could also calculate the portion of taxable income within each tax bracket (that is, 0 if taxableIncome <= taxBracket[i], (taxBracket[i+1]-taxBracket[i])*marginalRate[i] if taxableIncome >= taxBracket[i+1], or (taxableIncome - taxBracket[i])*marginalRate[i] if taxBracket[i+1] > taxableIncome > taxBracket[i]). Then add them together. This seemingly is more work, but if you do it right, the compiler can turn the algorithm into branchless SIMD code.

A Map-Reduction: If you factor out the calculation of the tax due for each bracket into a helper function, which is probably more readable anyway, you could turn the entire calculation into a call to std::transform_reduce. This is a bit more abstract and complex, but it’s also closer to what you hope the code will actually do. If your code is supposed to vectorize, but actually doesn’t, you’re also more likely to get an error with the higher-level API.