5
\$\begingroup\$

I'm very much on the beginner end of the PHP scale but I have read plenty of the entry level books and completed the online training at teamtreehouse.com so I know the basics.

I'm at a point where I've built a tax calculator in PHP (UK income tax) and it works and is accurate but I have no clue if the code follows best practice. Most likely, it doesn't!

I need help to see how I go from a beginner programmer to an intermediate. I want to make sure I'm on the right track before I start using a database for this project and converting it to an API.

My main class is below. I learn by doing and then having people smarter than me tell me the areas I suck in so I can improve. Hopefully you folks will be able to give me some pointers on areas to focus on.

I'm really trying to follow OOP practices and I know I have a lot to learn in that respect. Feel free to let me have it with both barrels.

<?php

require_once('tax_codes.php');
require_once('national_insurance.php');


class TaxCalculation {

    public $persona;
    public $taxRates;
    public $taxBand;
    public $taxFreeAllowance;
    public $personalAllowance;
    public $totalTaxableAmount;

/*
 * Sets the default values we need when the class is instantiated.
 * @param   array   $persona            User submitted inputs
 * @param   array   $income_tax_rates   Raw data for all tax years           
 */

    public function __construct($persona) {
        include('data/income_tax_rates.php');
        include('data/national_insurance_rates.php');
        include('data/student_loan_rates.php');
        include('data/childcare_voucher_rates.php');
        $this->persona = $persona;
        $this->taxRates = $income_tax_rates;
        $this->niRates = $national_insurance_rates;
        $this->taxYear = $this->persona["tax_year_is"];
        $this->taxBand = $this->taxRates[$this->taxYear]["rates"];
        $this->taxFreeAllowance = $this->taxRates[$this->taxYear]["allowances"];
        $this->studentRates = $student_loan_rates[$this->taxYear];
        $this->childCareVoucher = $annual_childcare_voucher_rates;
    }

/*
 * Takes two numbers and determines which is the lower figure.
 * @param   integer   $a,$b   Used to compare integers in other functions
 * @return  integer           The lowest value of the two checked                     
 */

    public function get_lower_figure($a, $b) {
        if ($a <= $b) {
            return $a;
        } else {
            return $b;
        }
    }

/*
 * Gets the personal allowance figure based on the users age.
 * @return  integer  The personal allowance for chosen tax year, by age               
 */

    public function get_personal_allowance() {
        if ($this->persona["age_is"] === "65_74") {
            $allowance = $this->taxFreeAllowance["personal_for_people_aged_65_74"];
            return $allowance;

        } elseif ($this->persona["age_is"] === "over_75") {
            $allowance = $this->taxFreeAllowance["personal_for_people_aged_75_and_over"];
            return $allowance;
        } else {
            $allowance = $this->taxFreeAllowance["personal"];
            return $allowance;
        }

    }

/*
 * Find and set the income allowance limit
 * @return  integer   The income limit for chosen tax year, by age           
 */

    public function get_income_allowance_limit() {

            if ($this->persona["age_is"] === "65_74" || $this->persona["age_is"] === "over_75") {
                $allowanceLimit = $this->taxFreeAllowance["income_limit_for_age_related"];
                return $allowanceLimit;
            } else {
                $allowanceLimit = $this->taxFreeAllowance["income_limit_for_personal"];
                return $allowanceLimit;
            }
    }

/*
 * Calculate the tax free amount that user is entitled to
 * @return  integer   The tax free allowance for chosen tax year           
 */

    public function get_tax_free_allowance() {
        $personalAllowance = $this->get_personal_allowance();
        $incomeAllowanceLimit = $this->get_income_allowance_limit();

        if ($this->persona["gross_annual_income"] > $incomeAllowanceLimit) {
            $deductFromAllowance = ($this->persona["gross_annual_income"] - $incomeAllowanceLimit) / 2;
            $personalAllowance = $personalAllowance - $deductFromAllowance;



            if ($this->persona["age_is"] === "65_74" || $this->persona["age_is"] === "over_75" ) {
                if ($personalAllowance <= $this->taxFreeAllowance["personal"]) {
                    $personalAllowance = $this->taxFreeAllowance["personal"];
                    $incomeAllowanceLimit = $this->taxFreeAllowance["income_limit_for_personal"];

                    if ($this->persona["gross_annual_income"] > $incomeAllowanceLimit) {

                    $deductFromAllowance = ($this->persona["gross_annual_income"] - $incomeAllowanceLimit) / 2;
                    $personalAllowance = $personalAllowance - $deductFromAllowance;
                }

                }
            }

        }

        if (is_numeric($this->persona["other_allowance_is"])) {
                $personalAllowance += $this->persona["other_allowance_is"];
            }

            if ($personalAllowance < 0) {
                $personalAllowance = 0;
            } 

        return $personalAllowance;

    }

/*
 * Set gross income to a float
 * @return  integer   Gross annual income         
 */

    public function set_gross_income() {
        $this->grossIncome = floatval($this->persona["gross_annual_income"]);
        return $this->grossIncome;
    }



/*
 * Finds the blind allowance for the chosen tax year
 * @return  integer   Blind persons allowance          
 */

    public function get_blind_persons_allowance() {
        $blind_persons_allowance = $this->taxFreeAllowance["blind_persons"];
        return $blind_persons_allowance;
    }

/*
 * Determines whether user is eligible for married couples allowance
 * @return  integer   Married couples allowance (10% of the allowance)          
 */

    public function get_married_couples_allowance() {
            $marriedAllowance = ($this->taxFreeAllowance["married_couples_over_75"] / 100) * 10;
            return $marriedAllowance;
        }

/*
 * Determines the personal allowance based on entered tax code
 * Replaces tax free allowance with calculated amount if the code isn't K
 * Adds the calculated amount to the total taxable amount if it is K
 * @return  integer   Personal allowance by tax code          
 */

    public function get_tax_code_personal_allowance() {
            $taxCodeCalculator = new TaxCodeCalculator($this->persona["tax_code_is"]);

            $this->taxCodePersonalAllowance = $taxCodeCalculator->get_personal_allowance_from_code();
            $this->taxCodeLetter = $taxCodeCalculator->taxCodeLetter;

            if (is_numeric($this->taxCodePersonalAllowance) && $this->taxCodeLetter === "K") {
                $this->totalTaxableAmount = $this->showGrossIncome + $this->taxCodePersonalAllowance;
                $this->showTaxFreeAllowance = 0;
            } elseif (is_numeric($this->taxCodePersonalAllowance) && $this->taxCodeLetter !== "K") {
                $this->showTaxFreeAllowance = $this->taxCodePersonalAllowance;
                $this->totalTaxableAmount = $this->showGrossIncome - $this->showTaxFreeAllowance;
            } 
    }

/*
 * Checks if the tax code is one of the special codes to work out
 * Compares the total taxable amount against the tax bands for chosen year
 * and works out the value of tax for each banding
 * @return     integer   Personal allowance by tax code          
 */

    public function calculate_tax_bands() {

        unset($this->taxBand["savings"]);

        if (isset($this->persona["tax_code_is"])) {
            $output = array();
            switch($this->persona["tax_code_is"]) {
                case 'BR':
                    // Basic Rate percentage
                    $this->showTaxFreeAllowance = 0;
                    $this->totalTaxableAmount = $this->showGrossIncome;
                    $bandPercentage = $this->taxBand["basic"]["rate"];
                    $percentageAmount = ($this->totalTaxableAmount / 100) * $bandPercentage;
                    $output["basic"] = round($percentageAmount);
                    $output["higher"] = 0;
                    $output["additional"] = 0;
                    return $output;

                case 'D0':
                    // Higher Band percentage
                    $this->showTaxFreeAllowance = 0;
                    $this->totalTaxableAmount = $this->showGrossIncome;
                    $bandPercentage = $this->taxBand["higher"]["rate"];
                    $percentageAmount = ($this->totalTaxableAmount / 100) * $bandPercentage;
                    $output["basic"] = 0;
                    $output["higher"] = round($percentageAmount);
                    $output["additional"] = 0;
                    return $output;
                case 'D1':
                    // Additional Band percentage
                    $this->showTaxFreeAllowance = 0;
                    $this->totalTaxableAmount = $this->showGrossIncome;
                    $bandPercentage = $this->taxBand["additional"]["rate"];
                    $percentageAmount = ($this->totalTaxableAmount / 100) * $bandPercentage;
                    $output["basic"] = 0;
                    $output["higher"] = 0;
                    $output["additional"] = round($percentageAmount);
                    $this->showTaxFreeAllowance = 0;
                    return $output;
                case 'NT':
                    // No Tax
                    return 0;

            }
        }


        $values = array();
        foreach ($this->taxBand as $key => $band) {

                    if ($band["end"] !== null || $band["end"] > 0) {
                        $band["amount"] = $this->get_lower_figure($this->totalTaxableAmount, $band["end"]) - $band["start"];
                    } else {
                        $band["amount"] = $this->totalTaxableAmount - $band["start"];
                    }

                $band["percentage_amount"] = ($band["amount"] / 100) * $band["rate"];
                $totalDeduction = $band["percentage_amount"];

                if ($totalDeduction < 0) {
                    $totalDeduction = 0;
                }

                $values[$key] = $totalDeduction;

        }
        return $values;

    }

/*
 * Takes total weekly income less deductions and works out the national
 * insurance contributions for the primary and upper bandings.
 * @return     integer   Annual national insurance contributions 
 */

    public function get_national_insurance_contribution() {
        $nationalInsuranceCalculator = new nationalInsuranceCalculator(
                                        ($this->persona["gross_annual_income"] - $this->showChildCareVouchers) / 52, $this->taxYear, $this->niRates);

        $totalNIContribution = $nationalInsuranceCalculator->get_ni_contributions();

        return $totalNIContribution;
    }

/*
 * Takes gross income less deductions and works out whether the income
 * is over the start amount before calculating the repayment amount
 * Student loans are also rounded down to the nearest pound.
 * @return     integer   Annual student loan repayment amount    
 */

    public function get_student_loan_repayment() {
        if ($this->persona["gross_annual_income"] >= $this->studentRates["start"]) {
            $deductableAmount = $this->persona["gross_annual_income"] - $this->studentRates["start"];

            if (isset($this->showChildCareVouchers)) {
                $deductableAmount -= $this->showChildCareVouchers;
            }

            $deduction = ($deductableAmount / 100) * $this->studentRates["rate"];
        }

        return floor($deduction);
    }

/*
 * Checks the pension amount for a % symbol and if found calculates the
 * percentage based on the annual income.  If there is no %, the entered
 * amount will be used instead.
 * @return     integer   Annual pension amount    
 */

    public function get_employers_pension_amount() {
        preg_match('/[%]/', $this->persona["pension_contribution_is"], $pensionPercentage);

        if (!empty($pensionPercentage) && $pensionPercentage[0] === "%") {
            $pensionPercentageAmount = preg_replace('/\D/', '', $this->persona["pension_contribution_is"]);

            if ($this->persona["pension_every_x"] === "month") {
                $monthlyIncome = $this->persona["gross_annual_income"] / 52;

                $pensionAmount = ($monthlyIncome / 100) * $pensionPercentageAmount;
                $annualAmount = $pensionAmount * 52;

                return $annualAmount;
            } else {
                $annualAmount = ($this->persona["gross_annual_income"] / 100) * $pensionPercentageAmount;

                return $annualAmount;
            }
        } else {
            if ($this->persona["pension_every_x"] === "month") {
                $monthlyIncome = $this->persona["gross_annual_income"] / 52;

                $pensionAmount = $this->persona["pension_contribution_is"];
                $annualAmount = $pensionAmount * 52;

                return $annualAmount;
            } else {
                $annualAmount = $this->persona["pension_contribution_is"];

                return $annualAmount;
            }
        }

    }

/*
 * Checks tax banding to see whether income is in a higher or additional band
 * If so, calculates the pension relief amount by multiplying the pension
 * amount by the tax band rate.
 * @return     integer   Annual HMRC pension relief    
 */

    public function get_hmrc_employers_pension_amount($pensionAmount) {
        $taxBands = $this->calculate_tax_bands();

        if ($taxBands["higher"] > 0 && $taxBands["additional"] === 0) {
            $pensionHMRC = ($pensionAmount / 100) * $this->taxBand["higher"]["rate"];

            return $pensionHMRC;

        } elseif ($taxBands["additional"] > 0) {
            $pensionHMRC = ($pensionAmount / 100) * $this->taxBand["additional"]["rate"];

            return $pensionHMRC;
        } else {
            $pensionHMRC = ($pensionAmount / 100) * $this->taxBand["basic"]["rate"];

            return $pensionHMRC;
        }

    }

/*
 * Checks whether the childcare voucher amount is within the limits allowed and 
 * if too high, returns the maximum allowed amount.  If the amount is in a higher
 * or additional tax band, a lower amount will be used.
 * @return     integer   Annual childcare voucher amount   
 */

    public function get_childcare_voucher_amount() {
        $income = $this->persona["gross_annual_income"];
        $taxBands = $this->taxBand;
        $annualAmount = $this->persona["annual_childcare_vouchers"];
        $rates = $this->childCareVoucher;
        $pre2011 = $this->persona["is_childcare_pre2011"];

        if ($annualAmount > $rates["basic"]) {
            $annualAmount = $rates["basic"];
        }

        if ($income >= $taxBands["higher"]["start"] && $annualAmount > $rates["higher"] && $pre2011 === "") {
            $annualAmount = $rates["higher"];

        } 

        if ($income >= $taxBands["additional"]["start"] && $annualAmount > $rates["additional"] && $pre2011 === "") {
            if ($this->persona["tax_year_is"] === "year2013_14" || $this->persona["tax_year_is"] === "year2014_15") {
                $rates["additional"] = 1320;

                $annualAmount = $rates["additional"];
            }

            $annualAmount = $rates["additional"];
        } 

        return $annualAmount;

    }

/*
 * Calculate the taxes for user and pull all figures together
 * @return  mixed   Return everything we need to populate the tax calculation table         
 */

    public function calculate_taxes() {

        $this->showGrossIncome = $this->persona["gross_annual_income"];
        $this->showTaxFreeAllowance = $this->get_tax_free_allowance();
        $this->totalTaxableAmount = $this->showGrossIncome - $this->showTaxFreeAllowance;
        $this->showTotalDeduction = 0;

        if ($this->persona["is_married"] === "on" && $this->persona["age_is"] === "over_75") {
            $this->showMarriedAllowance = $this->get_married_couples_allowance();
        }

        if ($this->persona["is_blind"] === "on") {
            $this->showBlindAllowance = $this->get_blind_persons_allowance();
            $this->showTaxFreeAllowance += $this->showBlindAllowance;
            $this->totalTaxableAmount -= $this->showBlindAllowance;
        }

        if (isset($this->persona["tax_code_is"])) {
            $this->get_tax_code_personal_allowance();
        }

        if (isset($this->persona["pension_contribution_is"])) {
            $this->showEmployerPension = $this->get_employers_pension_amount();
            $this->showPensionHMRC = $this->get_hmrc_employers_pension_amount($this->showEmployerPension);
            $this->totalTaxableAmount -= $this->showEmployerPension;
            $this->showTotalDeduction += $this->showEmployerPension;
        }

        if (isset($this->persona["annual_childcare_vouchers"])) {
            $this->showChildCareVouchers = $this->get_childcare_voucher_amount();
            $this->totalTaxableAmount -= $this->showChildCareVouchers;
            $this->showTotalDeduction += $this->showChildCareVouchers;
        }

        if ($this->persona["has_student_loan"] === "on") {
            $this->showStudentLoanAmount = $this->get_student_loan_repayment();
            $this->showTotalDeduction += $this->showStudentLoanAmount;
        }

        if ($this->persona["exclude_ni"] === "on" || $this->persona["age_is"] === "over_75" || $this->persona["age_is"] === "65_74") {
            $this->showNIContribution = 0;
        } else {
            $this->showNIContribution = $this->get_national_insurance_contribution();
            $this->showTotalDeduction += $this->showNIContribution;
        }

        if ($this->showGrossIncome <= $this->showTaxFreeAllowance) {
            $this->totalTaxableAmount = 0;
            $this->totalTaxDue = 0;
        } else {
            $this->deduction = $this->calculate_tax_bands();
            $this->totalTaxDue = $this->deduction["basic"] + $this->deduction["higher"] + $this->deduction["additional"];
            $this->showTotalDeduction += $this->totalTaxDue;
        } 

        $this->showNetIncome = $this->showGrossIncome - $this->showTotalDeduction;
    }

}
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Just a quick comment, before I take the time for a more thorough review Coding standards are very important. Try to adhere to them as much as you can. In this particular instance, you'll soon find that that will make all those require_once and include statements utterly redundant \$\endgroup\$ Apr 14, 2014 at 9:36
  • \$\begingroup\$ I looked through the site you linked to but wasn't really sure what I was looking at. Care to elaborate for a newb? Nevermind, I figured it out. Looking at autoload now :) \$\endgroup\$
    – Ian
    Apr 15, 2014 at 13:06
  • 1
    \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Vogel612
    Jan 9, 2017 at 22:11
  • \$\begingroup\$ Hi lan is it possible for you to share the code \$\endgroup\$
    – nilesh
    May 27, 2019 at 19:33

2 Answers 2

3
\$\begingroup\$

There is a lot that could be improved so at least a couple of points. In case that you need further explanation I will gladly explain it in comments.

  1. Having public class attributes is usually sign of bad design:

    public $persona;
    public $taxRates;
    public $taxBand;
    public $taxFreeAllowance;
    public $personalAllowance;
    public $totalTaxableAmount;
    

    You should initialize attributes in constructor or with setAttributeName($attributeName) methods:

    private $persona;
    
    public function setPersona($persona) { $this->persona = $persona; }
    

    If you need to access attribute outside of the class then you should have getAttributeName() methods:

    public function getPersona() { return $this->persona; }
    

    Inside class it is ok to access attribute directly using $this->persona.

  2. Do not include files inside class method:

    public function __construct($persona) {
        include('data/income_tax_rates.php');
        include('data/national_insurance_rates.php');
        include('data/student_loan_rates.php');
        include('data/childcare_voucher_rates.php');
        $this->persona = $persona;
        $this->taxRates = $income_tax_rates;
        $this->niRates = $national_insurance_rates;
        $this->taxYear = $this->persona["tax_year_is"];
        $this->taxBand = $this->taxRates[$this->taxYear]["rates"];
        $this->taxFreeAllowance = $this->taxRates[$this->taxYear]["allowances"];
        $this->studentRates = $student_loan_rates[$this->taxYear];
        $this->childCareVoucher = $annual_childcare_voucher_rates;
    }
    

    It should be like this:

    public function __construct($persona, $taxRates, $niRates, $taxYear, $taxBand, $taxFreeAllowance, $studentRates, $childCareVoucher) {
        $this->persona = $persona;
        $this->taxRates = $taxRates;
        $this->niRates = $niRates;
        $this->taxYear = $taxYear;
        $this->taxBand = $taxBand;
        $this->taxFreeAllowance = $taxFreeAllowance;
        $this->studentRates = $studentRates;
        $this->childCareVoucher = $childCareVoucher;
    }
    
    • Constructor or setAttributeName($attributeName) methods should be used for passing variables from outside of the class to class.
    • What if you decide to use the class in some other project? It would always mean to move it together with other files.
    • Design like this is not easy to be tested by test frameworks.
    • You can find a lot of good articles about OOP, for example here.
  3. Do not write method documentation like this:

    /*
     * Sets the default values we need when the class is instantiated.
     * @param   array   $persona            User submitted inputs
     * @param   array   $income_tax_rates   Raw data for all tax years
     */
    

    Better to write it like this:

    /*
     * Sets the default values we need when the class is instantiated.
     *
     * @param array $persona User submitted inputs
     * @param array $income_tax_rates Raw data for all tax years
     */
    
    • Imagine that you would have 10 params and then you would need to add 11th with way longer name than other parameters, that would require indenting all other parameters.
    • Readability is the same without indenting.
    • Indenting might cause problems displaying documentation hints in some IDEs.
  4. Do not use empty line between method documentation and method definition:

    /*
     * Takes two numbers and determines which is the lower figure.
     * @param   integer   $a,$b   Used to compare integers in other functions
     * @return  integer           The lowest value of the two checked
     */
    
    public function get_lower_figure($a, $b) {
        if ($a <= $b) {
            return $a;
        } else {
            return $b;
        }
    }
    

    It should be like this:

    /*
     * Sets the default values we need when the class is instantiated.
     *
     * @param array $persona User submitted inputs
     * @param array $income_tax_rates Raw data for all tax years
     */
    public function get_lower_figure($a, $b) {
        if ($a <= $b) {
            return $a;
        } else {
            return $b;
        }
    }
    
  5. There is built in method for this in PHP, the method is called "min":

    public function get_lower_figure($a, $b) {
        if ($a <= $b) {
            return $a;
        } else {
            return $b;
        }
    }
    
  6. This can be simplified (applies to other issues like this as well):

    if ($personalAllowance < 0) {
        $personalAllowance = 0;
    }
    
    return $personalAllowance;
    

    It should be like this:

    return ($personalAllowance < 0) ? 0 : $personalAllowance;
    
  7. This can be simplified (applies to other issues like this as well):

    public function get_blind_persons_allowance() {
        $blind_persons_allowance = $this->taxFreeAllowance["blind_persons"];
        return $blind_persons_allowance;
    }
    

    It should be like this:

    public function get_blind_persons_allowance() {
        return $this->taxFreeAllowance["blind_persons"];
    }    
    
  8. It is better to have single exit point in a method, that means not multiple return statements in a method.

  9. Variable $monthlyIncome not used in method get_employers_pension_amount(), comment in code:

    if ($this->persona["pension_every_x"] === "month") {
        // This variable is not used anywhere before return statement
        $monthlyIncome = $this->persona["gross_annual_income"] / 52;
    
        $pensionAmount = $this->persona["pension_contribution_is"];
        $annualAmount = $pensionAmount * 52;
    
        return $annualAmount;
    } else {
        $annualAmount = $this->persona["pension_contribution_is"];
    
        return $annualAmount;
    }
    
  10. Value in variable $annualAmount is reassigned right after it is assigned, method get_children_voucher_amount(), comment in code:

    if ($income >= $taxBands["additional"]["start"] && $annualAmount > $rates["additional"] && $pre2011 === "") {
        if ($this->persona["tax_year_is"] === "year2013_14" || $this->persona["tax_year_is"] === "year2014_15") {
            $rates["additional"] = 1320;
            // You assign value here, you do not use it for any operation
            $annualAmount = $rates["additional"];
        }
        // You reassign the value here, assignment above is not needed
        $annualAmount = $rates["additional"];
    }
    
  11. isset vs array_key_exists:

    if (isset($this->persona["annual_childcare_vouchers"])) {
        $this->showChildCareVouchers = $this->get_childcare_voucher_amount();
        $this->totalTaxableAmount -= $this->showChildCareVouchers;
        $this->showTotalDeduction += $this->showChildCareVouchers;
    }
    

    You are using the isset method for checking if array key exists. It works, but there is special method for that called array_key_exists.

  12. "" vs '':

    If string does not contain any variables it is generally better to use '', because PHP does not need to check if there is variable inside the string and the interpretation of script is a bit faster.

  13. Naming your class TaxCalculator would be more suitable, taxCalculation (or better calculateTax) would be more appropriate for a method.

  14. Use consistent naming of your variables and methods. Use myMethodName / myVariableName or my_method_name / my_variable_name, not both.

\$\endgroup\$
6
\$\begingroup\$

I'm not too familiar with PHP, so just a few generic notes:

  1. For this:

    /*
     * Sets the default values we need when the class is instantiated.
     * @param   array   $persona            User submitted inputs
     * @param   array   $income_tax_rates   Raw data for all tax years           
     */
    
        public function __construct($persona) {
    

    I'd use the same indentation level for both comments and functions since they're connected to each other:

    /*
    * Sets the default values we need when the class is instantiated.
    * @param   array   $persona            User submitted inputs
    * @param   array   $income_tax_rates   Raw data for all tax years           
    */
    public function __construct($persona) {
    

    You should also use consistent indentation in other places too.

  2. Instead of the following function you could use PHP's built-in min function:

    public function get_lower_figure($a, $b) {
        if ($a <= $b) {
            return $a;
        } else {
            return $b;
        }
    }
    
  3. Don't use floating point varibles for currency, they are not precise.

  4. persona seems to be an array. I would consider creating an object for it as well as for its keys. With an Age object you could change this:

    if ($this->persona["age_is"] === "65_74" || $this->persona["age_is"] === "over_75") {
    

    to a more type-safe version:

    if ($this->persona->age->isOver65()) { 
    

    It would also reduce duplication of array keys though the code and would be easier to read.

  5. 52 is used multiple times. You should create a named constant for it with a descriptive name.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Thanks for the pointers. I've taken your advice and started check my indentation, replaced that function with min() and I'm looking into creating a persona object. I've not done that before so I'm doing some reading first. \$\endgroup\$
    – Ian
    Apr 15, 2014 at 13:03
  • \$\begingroup\$ @ian: I'm glad that you find it helpful. The Persona object should be similar to you TaxCalculation object, probably with setter/getter methods. \$\endgroup\$
    – palacsint
    Apr 15, 2014 at 14:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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