patternphpMinor
UK tax calculator
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;
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:
I'd use the same indentation level for both comments and functions since they're connected to each other:
You should also use consistent indentation in other places too.
-
Instead of the following function you could use PHP's built-in
-
Don't use floating point varibles for currency, they are not precise.
-
to a more type-safe version:
It would also reduce duplication of array keys though the code and would be easier to read.
-
-
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.