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
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
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
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.)
const where Appropriate
You have a number of variables that could be static single assignments, or even compile-time constants, but aren't. Declaring
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:
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]
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,
taxableIncome <= taxBracket[i],
taxableIncome >= taxBracket[i+1], or
(taxableIncome - taxBracket[i])*marginalRate[i] if
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.