Let’s use jsfiddle.net with the OP (original post) code. Using the tidy feature allows us to fix the indenting, which helps us to understand the code better.
The first issue is with single/married - how is userStatus
going to be translated in to true/false for single/married?
One suggestion is to use a separate while loop for the prompt, so that you can keep asking until a correct answer is given.
The two separate variables are not be required at all either, for you can just check for one of them and use the else clause for the other. Because the code does single stuff before married, single
can be the status that we store.
var single = null;
var userStatus = "";
do {
userStatus = prompt("Are you Single or Married?");
userStatus = userStatus.toLowerCase();
if (userStatus === "single" || userStatus === "married") {
married = (userStatus === "single");
} else {
alert("Please enter single or married as your answer.");
}
} while (single = null);
For semicolon warriors, please note that the while part of a do-while loop also ends with a semicolon.
Also, variables should only be at the top of a code section like a function, so I’m moving variable declarations to the top for easier management.
Variable names should start with a lowercase letter, as an initial uppercase letter normally indicates that it’s a constructor function instead. The Total
and UserIncome
variables really should be total
and userIncome
instead.
Also, as you’ve use userIncome
elsewhere I think that you should assign total
to useIncome
after the loop completes.
The do/while loop is also easier to understand when the while clause is connected up with the closing brace.
Also, the prompt results in a string result, so that while (moreIncome === true)
condition is never going to end. The prompt though does give null when cancel is chosen, so you can just check for a truthy or falsy value instead.
By way of example null, undefined, false, 0, and “” are all falsey values. Beware though, for “0” and “false” as strings are considered to be a truthy value, not falsy ones.
do {
var UserIncome = prompt("Enter the income you received (W-2, Investments, 1099")
total += useIncome;
var moreIncome = prompt("Do you have any more additional income to add to your 1040 Tax Form?");
} while (moreIncome);
Because the code carries on to use userIncome
, I recommend that those further ones are not changed to total, and instead that the total from the do/while loop is used to replace the last userIncome
value.
do {
...
} while (moreIncome);
userIncome = total;
With the single/married deductions, it can help to decide how you want to group the code. Do you want all of the single stuff happening, with the married stuff occurring in the else clause?
if (single) {
// single stuff
// more single stuff
} else {
// married stuff
// more married stuff
}
Or do you want to separate them out so that each thing happens separately?
if (single) {
// single stuff
} else {
// married stuff
}
if (single) {
// more single stuff
} else {
// more married stuff
}
I’ll go with the latter structure for this example.
The single/married deduction prompt doesn’t seem to make any sense, because you already know if they are married. As a result, you can do without the prompt and take care of it immediately.
if (single) {
userIncome -= 6300;
} else {
userIncome -= 12600;
}
With the series of if/else if statements, it can really help to keep the braces properly lined up, to ensure that everything is organised correctly. It can also help to keep the single and married parts separate.
In fact, by putting the single and married tax rates in to separate functions, that can help the code to be easier to understand too.
Here’s the singleTax function for example:
function singleTax(userIncome) {
if (userIncome < 9275) {
tax = userIncome * 0.10;
} else if (userIncome < 37650) {
tax = userIncome * 0.15;
} else if (userIncome < 91150) {
tax = userIncome * 0.25;
} else if (userIncome < 190150) {
tax = userIncome * 0.28;
} else if (userIncome < 413350) {
tax = userIncome * 0.33;
} else if (userIncome < 415050) {
tax = userIncome * 0.35;
} else {
tax = userIncome * 0.396;
}
return tax;
}
Which along with a marriedTax function, results in code that ends up just being:
if (single) {
tax = singleTax(userIncome);
} else {
tax = marriedTax(userIncome);
}
You could then work on making the function easier to work with, such as by having a list of tax rates where you search through them until you come to a value that’s greater than the user’s income.
function singleTax(userIncome) {
var rates = [
{tax: 0.10, amount: 9275},
{tax: 0.15, amount: 37650},
{tax: 0.25, amount: 91150},
{tax: 0.28, amount: 190150},
{tax: 0.33, amount: 413350},
{tax: 0.35, amount: 415050},
{tax: 0.396, amount: Number.MAX_VALUE}
];
var taxRate = 0;
rates.find(function (rate) {
if (amount < userIncome) {
taxRate = rate.tax;
} else {
return true;
}
});
return userIncome * taxRate;
}
We can now see that the singleTax and marriedTax functions are near identical, all but for the rates themself, so that logic we can move on out to a separate findTaxRate()
function instead:
function findTaxRate(rates, userIncome) {
var taxRate = 0;
rates.find(function (rate) {
if (amount < userIncome) {
taxRate = rate.tax;
} else {
return true;
}
});
return taxRate;
}
Also, the tax rates shouldn’t be inside of the function. That’s data that can be stored elsewhere and given to the function instead.
var taxRates = {
single: [
{tax: 0.10, amount: 9275},
{tax: 0.15, amount: 37650},
{tax: 0.25, amount: 91150},
{tax: 0.28, amount: 190150},
{tax: 0.33, amount: 413350},
{tax: 0.35, amount: 415050},
{tax: 0.396, amount: Number.MAX_VALUE}
],
married: [
{tax: 0.10, amount: 18550},
{tax: 0.15, amount: 75300},
{tax: 0.25, amount: 151900},
{tax: 0.28, amount: 231450},
{tax: 0.33, amount: 413350},
{tax: 0.35, amount: 466950},
{tax: 0.396, amount: Number.MAX_VALUE}
]
};
So now we can get the appropriate tax rates with taxRates.single
, which leaves our singleTax()
and ‘marriedTax()’ functions looking identical to each other:
function singleTax(rates, userIncome) {
return userIncome * findTaxRate(rates, userIncome);
}
function marriedTax(rates, userIncome) {
return userIncome * findTaxRate(rates, userIncome);
}
...
if (single) {
tax = singleTax(taxRates.single, userIncome);
} else {
tax = marriedTax(taxRates.married, userIncome);
}
It’s not a loss though, for they have helped us to extract out useful behaviour. We cannot allow these two identical functions to remain though, so they can be combined in to a single function called getTax()
function getTax(rates, userIncome) {
return userIncome * findTaxRate(rates, userIncome);
}
...
if (single) {
tax = getTax(taxRates.single, userIncome);
} else {
tax = getTax(taxRates.married, userIncome);
}
let us now look at the variables that we have floating around the place:
var single = null;
var userStatus = "";
var userIncome = 0;
var moreIncome = 0;
var total = 0;
var tax = 0;
That’s a lot. We need single, and we need userIncome, and possibly even tax, but the others can be removed by moving their sections of code out to separate functions.
Asking for the marital status, that can be moved in to an askMaritalStatus()
function:
function askMaritalStatus() {
var userStatus = "";
do {
userStatus = prompt("Are you Single or Married?");
userStatus = userStatus.toLowerCase();
if (userStatus === "single" || userStatus === "married") {
return (userStatus === "single");
} else {
alert("Please enter single or married as your answer.");
}
} while (single = null);
}
...
single = askMaritalStatus();
When asking for the user income, we can put that all in to its own askUserIncome()
function:
function askUserIncome() {
var userIncome = 0;
var total = 0;
var moreIncome;
do {
userIncome = prompt("Enter the income you received (W-2, Investments, 1099");
total += userIncome;
moreIncome = prompt("Do you have any more additional income to add to your 1040 Tax Form?");
} while (moreIncome);
return total;
}
...
userIncome = askUserIncome();
The improvements can keep on occuring from there, but this is just a taste of how such improvements can be made.
The code after all of these improvements can be found at https://jsfiddle.net/qokphgqh/2/ and is as follows:
function askMaritalStatus() {
var userStatus = "";
do {
userStatus = prompt("Are you Single or Married?");
userStatus = userStatus.toLowerCase();
if (userStatus === "single" || userStatus === "married") {
return (userStatus === "single");
} else {
alert("Please enter single or married as your answer.");
}
} while (single = null);
}
function askUserIncome() {
var userIncome = 0;
var total = 0;
var moreIncome;
do {
userIncome = prompt("Enter the income you received (W-2, Investments, 1099");
total += userIncome;
moreIncome = prompt("Do you have any more additional income to add to your 1040 Tax Form?");
} while (moreIncome);
return total;
}
function findTaxRate(rates, userIncome) {
var taxRate = 0;
rates.find(function (rate) {
if (amount < userIncome) {
taxRate = rate.tax;
} else {
return true;
}
});
return taxRate;
}
function getTax(rates, userIncome) {
return userIncome * findTaxRate(rates, userIncome);
}
var single;
var userIncome = 0;
var tax = 0;
var taxRates = {
single: [
{tax: 0.10, amount: 9275},
{tax: 0.15, amount: 37650},
{tax: 0.25, amount: 91150},
{tax: 0.28, amount: 190150},
{tax: 0.33, amount: 413350},
{tax: 0.35, amount: 415050},
{tax: 0.396, amount: Number.MAX_VALUE}
],
married: [
{tax: 0.10, amount: 18550},
{tax: 0.15, amount: 75300},
{tax: 0.25, amount: 151900},
{tax: 0.28, amount: 231450},
{tax: 0.33, amount: 413350},
{tax: 0.35, amount: 466950},
{tax: 0.396, amount: Number.MAX_VALUE}
]
};
single = askMaritalStatus();
userIncome = askUserIncome();
if (single) {
userIncome -= 6300;
} else {
userIncome -= 12600;
}
if (single) {
tax = getTax(taxRates.single, userIncome);
} else {
tax = getTax(taxRates.married, userIncome);
}