HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

UK tax calculator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
taxcalculatorstackoverflow

Problem

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.

```
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 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;

Solution

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

-
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.

-
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;
    }
}


-
Don't use floating point varibles for currency, they are not precise.

  • Why not use Double or Float to represent currency?



  • How to deal with strange rounding of floats in PHP



-
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.

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

Code Snippets

/*
 * 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) {
/*
* 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) {
public function get_lower_figure($a, $b) {
    if ($a <= $b) {
        return $a;
    } else {
        return $b;
    }
}
if ($this->persona["age_is"] === "65_74" || $this->persona["age_is"] === "over_75") {
if ($this->persona->age->isOver65()) {

Context

StackExchange Code Review Q#47073, answer score: 6

Revisions (0)

No revisions yet.