Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm looking for some feedback on my first Javascript app. I've build an app that calculates tax for Australian tax payers. Would be great if someone could have a look over the code that I have produced. Specific feedback on areas that could be object orientated and any errors I have made would be great.

//Inputs
var income = 250000;
var superAnnuationPercentage = 9.25;
var superAnnuationTaxRate = 15;


//Defines the tax brackets
var taxBracket = [
    {from: 0, to: 18200, percentage: 0, amount: 0},
    {from: 18201, to: 37000, percentage: 19, over: 18200, amount: 0},
    {from: 37001, to: 80000, percentage: 32.5, over: 37000, amount: 3752},
    {from: 80001, to: 180000, percentage: 37, over: 80000, amount: 17547},
    {from: 180001, to: Infinity, percentage: 45, over: 180000, amount: 54547}
];


//Defines HECS/HELP repayment brackets
var hecsHelp = [
{from:0,     to:53345, percentage:4},
{from:53344, to:59421, percentage:4},
{from:59422, to:65497, percentage:4.5},
{from:65498, to:68939, percentage:5.0},
{from:68940, to:74105, percentage:5.5},
{from:74106, to:80527, percentage:6.0},
{from:80258, to:84481, percentage:6.5},
{from:84482, to:92970, percentage:7.0},
{from:99070, to:99069, percentage:7.5},
{from:99070, to:Infinity, percentage:8.0},
];

//Calculates superannuation based on a percentage
var superannuationCalc = function (int) {
    var percent = superAnnuationPercentage/100;
    return  int * percent;
};

//define income after removing superannuation element. Superannuation is taxed at a lower rate.
var grossIncome = income - superannuationCalc(income);

//The end result of alll the code.
var workoutTax = taxCalc(grossIncome);

//bringing it all together
function workOutSuperTax(){
var a = superannuationCalc(income);
var b = superAnnuationTaxRate;
var c = superannuationTaxCalc(a,b);
return c;
}

var superTax = workOutSuperTax();

var workOutHECS = hecsHelpCalc(income);

totalTax = workoutTax + superTax + workOutHECS;

//The end result!
console.log(totalTax);

//Loops through taxBracket object and finds the tax bracket for the post deduction gross income amount
function taxCalc(grossSuperIncome){
    for(var x = 0; x < taxBracket.length; x++){
        if(grossSuperIncome <= taxBracket[x].to){
            var amountOver = grossSuperIncome - taxBracket[x].over;
            var percent = taxBracket[x].percentage / 100;
            return taxBracket[x].amount + (amountOver * percent);

        }

    };
};

//calculates super tax amount by usuing superTaxRate and superAnnuation variables
function superannuationTaxCalc(superAmount,superTaxRate){
    var percent = superTaxRate / 100;
    return superAmount * percent;
};

//calculates HECS bracket
function hecsHelpCalc(hecsIncome){
        for (var x =0; x < hecsHelp.length; x++){
            if(hecsIncome <= hecsHelp[x].to){
            var percentage = hecsHelp[x].percentage / 100;
            return hecsIncome * percentage;
            }
        };
};
share|improve this question
    
As others have pointed out hecsHelp has the wrong values: 0-53k should be percentage 0 and the 7.5 percentage should have range: 92971-99069. This is all on the assumption of the 2014-15 year. –  James Khoury Sep 3 at 3:44

4 Answers 4

Ranges

You use

var hecsHelp = [
{from:0,     to:53345, percentage:4},
{from:53344, to:59421, percentage:4},
{from:59422, to:65497, percentage:4.5},
{from:65498, to:68939, percentage:5.0},
{from:68940, to:74105, percentage:5.5},
{from:74106, to:80527, percentage:6.0},
{from:80258, to:84481, percentage:6.5},
{from:84482, to:92970, percentage:7.0},
{from:99070, to:99069, percentage:7.5},
{from:99070, to:Infinity, percentage:8.0},
];

to define ranges with different percentages. A few things to say about this :

  • it does not respect the DRY principle : this makes things easier to get wrong (and I believe that either the first or the second range is indeed wrong as it does not follow the same pattern where range[n].to + 1 == range[n+1].from)
  • it does not define what happens when the number you want to handle happens to be between range[n].to and range[n+1].from which can happen if you are handling non-integers values.

I think things would be easier if you were to define it like :

var hecsHelp = [
{from:0,     percentage:4},
{from:53344, percentage:4}, # By the way, is this line useful ?
{from:59422, percentage:4.5},
{from:65498, percentage:5.0},
{from:68940, percentage:5.5},
{from:74106, percentage:6.0},
{from:80258, percentage:6.5},
{from:84482, percentage:7.0},
{from:99070, percentage:7.5}, # This one seems pretty weird too.
{from:99070, percentage:8.0},
];

Then, iterating backward (or forward if you define your array as the reverse of what I have written), everything should be easier to maintain.

share|improve this answer

The terminology for the limits of the brackets should be made more consistent. You never use the from values in taxBracket or hecsHelp. Curiously, you do use taxBracket[x].over, which is almost the same thing. I suggest renaming .amount to .baseAmount for clarity.

Take care to indent your code consistently, for example, in hecsHelp, workOutSuperTax(), and hecsHelpCalc().

It's odd that the superannuationCalc function is defined as using the var superannuationCalc = function(int) { … } syntax.

You should group all of your constants together (e.g. taxBracket), followed by all of the functions, followed by the user-calculation. A good convention to follow is to name the constants using UPPER_CASE.

share|improve this answer

I looked at your reference tables and immediately thought "Oh, gappy ranges, there's a bug, what if the value is 18200.5?"

So I thought I would have a quick answer about how JavaScript automatically does floating-point calculations, so floating-point numbers happen more often than you may think.

But then, when I looked at it more, I realized that your tables have two issues:

  1. You have the from value in your tables, but you never use it. Instead you assume the tables are in order, and you use <= ...[x].to. Your from values are essentially dead and misleading data.

  2. You have duplicate and overlapping values in your table here:

    var hecsHelp = [
    {from:0,     to:53345, percentage:4},
    {from:53344, to:59421, percentage:4},
    

    Since your code only uses the to value, it works, but the data makes it confusing.

share|improve this answer
    
Do I have to recode this deal with floating point numbers? Thanks –  andrew_b Sep 2 at 21:44
    
@andrew_b - not necessarily, because you use <= comparisons with to. But, you need to correct your data tables, and rationalize the need for the from column. Additionally, one of the other answers suggests using from, and not to, as the boundary. That solution would have some small benefits too. –  rolfl Sep 2 at 21:58
    
Floating point values shouldn't be a problem because Australian tax is on income that is rounded down to the nearest dollar. –  James Khoury Sep 3 at 3:41

This is only a sample on single object approach using prototype. I have not fixed the brackets as noted by others. There is also some bad naming, but leave it be as I have not checked out what actually happens here ... ;)

Knowing what is what etc. I guess some re-factoring could be done to make it more streamlined.

Anyhow; only meant as a dirty mashup as a starter on a little different approach.

Edit: Added another re-write at bottom of post.

The tax calc

// Base function.
// This could hold a "default" config etc.
// In this code _all_ conf fields are both required and used.
// No error check.
function TaxCalc(conf) {
    this.taxBracket      = conf.taxBracket;
    this.hecsHelp        = conf.hecsHelp;
    this.superAnnPercent = conf.superAnnPercent;
    this.superAnnTaxRate = conf.superAnnTaxRate;
}

// Calculates superannuation based on a percentage.
// AKA superannuationCalc = function (int)
TaxCalc.prototype.superAnnPst = function (amount) {
    return  amount * this.superAnnPercent / 100;
};

// Calculates super tax amount by using superAnnuation and superTaxRate.
// AKA superannuationTaxCalc()
TaxCalc.prototype.superAnnTax = function (amount) {
    return amount * this.superAnnTaxRate / 100;
};

// Not used, but should perhaps be ...
TaxCalc.prototype.grossIncome = function (income) {
    return income - this.superAnnPst(income);
};

// Loops through taxBracket object and finds the tax bracket for the
// post deduction gross income amount.
TaxCalc.prototype.taxCalc = function (grossIncome) {
    for (var x = 0; x < this.taxBracket.length; ++x) {
        if (grossIncome <= this.taxBracket[x].to) {
            var amountOver = grossIncome - this.taxBracket[x].over;
            var percent = this.taxBracket[x].percentage / 100;
            return this.taxBracket[x].amount + (amountOver * percent);
        }
    }
};

// Calculates HECS bracket.
TaxCalc.prototype.hecsHelpCalc = function (hecsIncome) {
    for (var x = 0; x < this.hecsHelp.length; ++x) {
        if (hecsIncome <= this.hecsHelp[x].to) {
            var percentage = this.hecsHelp[x].percentage / 100;
            return hecsIncome * percentage;
        }
    }
};

// Simple debug log function of result object.    
TaxCalc.prototype.log = function (report) {
    console.log(
        '================================\n' +
        'Income      : ', report.income, '\n' +
        'Gross Income: ', report.grossIncome, '\n' +
        'Gross Ann   : ', report.superPst, '\n' +

        'Tax Base    : ', report.taxBase, '\n' +
        'Tax Super   : ', report.taxSuper, '\n' +
        'Tax HECS    : ', report.taxHECS, '\n' +
        '--------------------------------\n' +
        'Tax Total   : ', report.taxTotal, '\n' +
        '================================\n'
    );
};

// Generate a report based on income.
TaxCalc.prototype.genReport = function (income) {
    var report = {
        income: income
    };
    report.superPst    = this.superAnnPst(income);
    report.grossIncome = income - report.superPst;

    report.taxBase     = this.taxCalc(report.grossIncome);
    report.taxSuper    = this.superAnnTax(report.superPst);
    report.taxHECS     = this.hecsHelpCalc(income);

    report.taxTotal    = report.taxBase  +
                         report.taxSuper +
                         report.taxHECS;
    return report;
};

Sample usage:

Edit: Added some more corrections and comments based on:

Based on this, I guess taxBracket and hecsHelp might also better be named something like TAX_RES_2014_15 and HRI_2014_15. In the Calculator you would then use more generic names like TAX_RES and HRI.

/* ====================================================== */
 * SAMPLE CODE                                            *
 * ====================================================== */

// Defines the tax brackets
// With fixed error "amount: 3752", should be 3572.
var taxBracket = [
    {from:     0, to:   18200, percentage: 0.0,              amount:    0},
    {from: 18201, to:   37000, percentage:19.0, over: 18200, amount:    0},
    {from: 37001, to:   80000, percentage:32.5, over: 37000, amount: 3572}, /* Err. 3752 */
    {from: 80001, to:  180000, percentage:37.0, over: 80000, amount:17547},
    {from:180001, to:Infinity, percentage:45.0, over:180000, amount:54547}
];

// Defines HECS/HELP repayment brackets
// With fixed errors (Errors noted in comments.)
var hecsHelp = [
    {from:    0, to:   53344, percentage:0.0}, /* Err. to  :53345, 4.0 % */
    {from:53345, to:   59421, percentage:4.0}, /* Err. from:53344 */
    {from:59422, to:   65497, percentage:4.5}, /* OK. */
    {from:65498, to:   68939, percentage:5.0}, /* OK. */
    {from:68940, to:   74105, percentage:5.5}, /* OK. */
    {from:74106, to:   80257, percentage:6.0}, /* Err. to  :80527 */
    {from:80258, to:   84481, percentage:6.5}, /* OK. */
    {from:84482, to:   92970, percentage:7.0}, /* OK. */
    {from:92971, to:   99069, percentage:7.5}, /* Err. from:99070 */
    {from:99070, to:Infinity, percentage:8.0}, /* OK. */
];


// Create a new "Tax Calcualtor"
var tax = new TaxCalc({
    taxBracket      : taxBracket,
    hecsHelp        : hecsHelp,
    superAnnPercent : 9.25,
    superAnnTaxRate : 15
});

// Run a couple of tests and print result to log.
tax.log(tax.genReport(250000));
tax.log(tax.genReport(340000));

Yield in log:

/*
================================
Income      :  250000
Gross Income:  226875
Gross Ann   :  23125
Tax Base    :  75640.75
Tax Super   :  3468.75
Tax HECS    :  20000
--------------------------------
Tax Total   :  99109.5
================================

================================
Income      :  340000
Gross Income:  308550
Gross Ann   :  31450
Tax Base    :  112394.5
Tax Super   :  4717.5
Tax HECS    :  27200
--------------------------------
Tax Total   :  144312
================================
*/

Edit


Decided to have another look at this. Here is an example. There are still improvements to be made, some renaming is likely also in order. But; basics is built upon previous code. The brackets are simplified and a generic search function is used to find the bracket you want based on from and some amount. Here with the somewhat ambiguous/bad name _getFromBracket().

Post it as you might find some of it useful.

/***
 * Tax Calculating.
 *
 * Minor error checking.
 * */
function TaxCalc(conf) {
    this.TAX = conf.TAX;
    this.HRI = conf.HRI;
    this.SUPER_RATE = conf.superAnnRate / 100;
    this.SUPER_TAX  = conf.superAnnTax  / 100;

    if (this.TAX[0].from !== 0)
        throw new Error(
            'Tax bracket has to start from zero. Currently at ' +
            this.TAX[0].from
        );
    if (this.HRI[0].from !== 0)
        throw new Error(
            'HRI bracket has to start from zero. Currently at ' +
            this.HRI[0].from
        );
}

/***
 * Calculate superannuation amount.
 * */
TaxCalc.prototype.superAnnuation = function (incomeTotal) {
    if (incomeTotal < 1)
        return 0;
    return incomeTotal * this.SUPER_RATE;
};

/***
 * Calculate tax amount from superannuation amount.
 * */
TaxCalc.prototype.taxSuperAnnuation = function (amountSuper) {
    if (amountSuper < 1)
        return 0;
    return amountSuper * this.SUPER_TAX;
};

/***
 * Calculate gross income.
 * */
TaxCalc.prototype.incomeGross = function (incomeTotal) {
    if (incomeTotal < 1)
        return 0;
    return incomeTotal - this.superAnnuation(incomeTotal);
};

/***
 * Helper function for brackets search.
 *
 * Find object in array of objects where obj[x].from is within
 * range of lim. Starting at end of array; highest from-amount.
 * */
TaxCalc.prototype._getFromBracket = function (arr, lim) {
    var i;

    for (i = arr.length - 1; i >= 0; --i) {
        if (lim >= arr[i].from)
            break;
    }
    /* If no bracket found return null, which denotes error. */
    if (i < 0)
        return null;
    return arr[i];
};

/***
 * Calculate HRI amount.
 *
 * (HECS) Higher Education Contribution Scheme
 * (HELP) Higher Education Loan Program
 * (HRI)  HELP Rapayment Income bracket
 */
TaxCalc.prototype.taxHRI = function (incomeTotal) {
    var hri = this._getFromBracket(this.HRI, incomeTotal);
    /* !hri aka null/error. This should likely be reported somehow. */
    if (!hri)
        return 0;
    return incomeTotal * hri.rate;
};

/***
 * Calculate base income tax.
 *
 * @incomeGross Amount after deducting superannuation amount.
 * */
TaxCalc.prototype.taxGross = function (incomeGross) {
    var tax = this._getFromBracket(this.TAX, incomeGross);
    /* !tax aka null/error. This should likely be reported somehow. */
    if (!tax)
        return 0;
    var amountOver = incomeGross - (tax.from - 1);
    return tax.base + (amountOver * tax.rate);
};

/***
 * Debug; log report to console.
 * */
TaxCalc.prototype.log = function (report) {
    console.log(
        '================================',
        '\nIncome Total: ', report.incomeTotal,
        '\nIncome Gross: ', report.incomeGross,
        '\nAmount Super: ', report.amountSuper,

        '\nTax Gross   : ', report.taxGross,
        '\nTax Super   : ', report.taxSuper,
        '\nTax HRI     : ', report.taxHRI,
        '\n--------------------------------',
        '\nTax Total   : ', report.taxTotal,
        '\n================================\n'
    );
};

/***
 * Generate tax report object.
 * */
TaxCalc.prototype.genReport = function (incomeTotal) {
    var report = {
        incomeTotal: incomeTotal,
        amountSuper: this.superAnnuation(incomeTotal),
        incomeGross: this.incomeGross(incomeTotal),
        taxHRI     : this.taxHRI(incomeTotal)
    };

    report.taxGross = this.taxGross(report.incomeGross);
    report.taxSuper = this.taxSuperAnnuation(report.amountSuper);

    report.taxTotal = report.taxGross +
                      report.taxSuper +
                      report.taxHRI;
    return report;
};

/* ***************************************************************
 *
 *     APPLICATION / TEST CODE
 *
 * ************************************************************* */

function test() {
    /***
     * Individual income tax rates for residents.
     *
     * https://www.ato.gov.au/rates/individual-income-tax-rates/
     *
     * */
    var TAX_RES_2014_15 = [
        {from:     0, rate:0.000, base:    0},
        {from: 18201, rate:0.190, base:    0},
        {from: 37001, rate:0.325, base: 3572},
        {from: 80001, rate:0.370, base:17547},
        {from:180001, rate:0.450, base:54547}
    ];
    /***
    * (HECS) Higher Education Contribution Scheme
    * (HELP) Higher Education Loan Program
    *
    * HELP Rapayment Income (HRI) bracket
    *
    * https://www.ato.gov.au/rates/help-repayment-thresholds-and-rates/
    *
    * Percentage = 3.5 + Index * 0.5, but not always in history,
    * thus (likely) better to hardcode.
    * */
    var HRI_2014_15 = [
        {from:    0, rate:0.000},
        {from:53345, rate:0.040},
        {from:59422, rate:0.045},
        {from:65498, rate:0.050},
        {from:68940, rate:0.055},
        {from:74106, rate:0.060},
        {from:80258, rate:0.065},
        {from:84482, rate:0.070},
        {from:92971, rate:0.075},
        {from:99070, rate:0.080}
    ];
    /* Create a tax object. */
    var tax = new TaxCalc({
        TAX          : TAX_RES_2014_15,
        HRI          : HRI_2014_15,
        superAnnRate : 9.25,
        superAnnTax  : 15
    });
    /* Sample income array. */
    var income = [
        250000,
        340000
    ];

    var i, rep;

    for (i = 0; i < income.length; ++i) {
        rep = tax.genReport(income[i]);
        tax.log(rep);
    }

    /* If you want to continue to use it elsewhere. E.g. add a event driven
     * calculator in HTML. */
    return tax;
}

// Execute test.
test();
share|improve this answer
    
Exactly the feedback that I was after –  andrew_b Sep 4 at 0:37
    
@andrew_b note that the best "thank you" on Stack Exchange remains an upvote ;) –  Mat's Mug Sep 5 at 0:26
    
Thanks @user13500. This has really helped me get my head around object orientated javascript. –  andrew_b Sep 6 at 12:56

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.