From cf0d988c7302de12ad53882216bc9a6ced1344cf Mon Sep 17 00:00:00 2001 From: Joseph Van der Wee Date: Tue, 2 Jun 2020 12:15:26 +0100 Subject: [PATCH] HMA-3006 fix for InvalidMutabilityException (#44) * HMA-3006 fix for InvalidMutabilityException * HMA-3006 moved CalculatorTests into commonTest --- README.md | 2 +- .../uk/gov/hmrc/calculator/Calculator.kt | 51 +++---- .../hmrc/calculator/model/bands/TaxBands.kt | 53 ++++--- .../uk/gov/hmrc/calculator/CalculatorTests.kt | 14 +- .../calculator/model/bands/TaxBandsTests.kt | 137 ++++++++++++++++-- 5 files changed, 183 insertions(+), 74 deletions(-) rename src/{jvmTest => commonTest}/kotlin/uk/gov/hmrc/calculator/CalculatorTests.kt (76%) diff --git a/README.md b/README.md index a18565e..57cf24b 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ [![Build Status](https://app.bitrise.io/app/cd7fb52c258b9273/status.svg?token=lntO8o4xz5AUEvLwVzbo3A&branch=master)](https://app.bitrise.io/app/cd7fb52c258b9273) ![LINE](https://img.shields.io/badge/line--coverage-98%25-brightgreen.svg) ![BRANCH](https://img.shields.io/badge/branch--coverage-93%25-brightgreen.svg) -![COMPLEXITY](https://img.shields.io/badge/complexity-1.51-brightgreen.svg) +![COMPLEXITY](https://img.shields.io/badge/complexity-1.52-brightgreen.svg) [ ![Download](https://api.bintray.com/packages/hmrc/mobile-releases/tax-kalculator/images/download.svg) ](https://bintray.com/hmrc/mobile-releases/tax-kalculator/_latestVersion) ## Calculate take-home pay diff --git a/src/commonMain/kotlin/uk/gov/hmrc/calculator/Calculator.kt b/src/commonMain/kotlin/uk/gov/hmrc/calculator/Calculator.kt index e65994b..182030b 100644 --- a/src/commonMain/kotlin/uk/gov/hmrc/calculator/Calculator.kt +++ b/src/commonMain/kotlin/uk/gov/hmrc/calculator/Calculator.kt @@ -26,7 +26,6 @@ import uk.gov.hmrc.calculator.exception.InvalidWagesException import uk.gov.hmrc.calculator.model.BandBreakdown import uk.gov.hmrc.calculator.model.CalculatorResponse import uk.gov.hmrc.calculator.model.CalculatorResponsePayPeriod -import uk.gov.hmrc.calculator.model.Country import uk.gov.hmrc.calculator.model.Country.ENGLAND import uk.gov.hmrc.calculator.model.PayPeriod import uk.gov.hmrc.calculator.model.PayPeriod.FOUR_WEEKLY @@ -75,12 +74,13 @@ class Calculator @JvmOverloads constructor( val taxCode = this.taxCode.toTaxCode() - val taxBands = TaxBands.getBands(taxYear, taxCode.country) + val taxBands = TaxBands.getAdjustedBands(taxYear, taxCode) - val taxFreeAmount = adjustTaxBands(taxBands, taxCode)[0].upper + val taxFreeAmount = taxBands[0].upper val amountToAddToWages = if (taxCode is KTaxCode) taxCode.amountToAddToWages else null return createResponse( + taxCode, yearlyWages, taxFreeAmount, amountToAddToWages @@ -88,11 +88,11 @@ class Calculator @JvmOverloads constructor( } private fun createResponse( + taxCode: TaxCode, yearlyWages: Double, taxFreeAmount: Double, amountToAddToWages: Double? ): CalculatorResponse { - val taxCode = this.taxCode.toTaxCode() val taxPayable = taxToPay(yearlyWages, taxCode) val employeesNI = employeeNIToPay(yearlyWages) val employersNI = employerNIToPay(yearlyWages) @@ -143,15 +143,20 @@ class Calculator @JvmOverloads constructor( } private fun taxToPay(yearlyWages: Double, taxCode: TaxCode): Double { - val taxBands = TaxBands.getBands(taxYear, taxCode.country) - return when (taxCode) { - is StandardTaxCode, is AdjustedTaxFreeTCode, is EmergencyTaxCode, is MarriageTaxCodes -> - getTotalFromBands(adjustTaxBands(taxBands, taxCode), yearlyWages) + is StandardTaxCode, is AdjustedTaxFreeTCode, is EmergencyTaxCode, is MarriageTaxCodes -> { + val taxBands = TaxBands.getAdjustedBands(taxYear, taxCode) + getTotalFromBands(taxBands, yearlyWages) + } is NoTaxTaxCode -> getTotalFromSingleBand(yearlyWages, taxCode.taxFreeAmount) - is SingleBandTax -> getTotalFromSingleBand(yearlyWages, taxBands[taxCode.taxAllAtBand].percentageAsDecimal) - is KTaxCode -> - getTotalFromBands(adjustTaxBands(taxBands, taxCode), yearlyWages + taxCode.amountToAddToWages) + is SingleBandTax -> { + val taxBands = TaxBands.getBands(taxYear, taxCode.country) + getTotalFromSingleBand(yearlyWages, taxBands[taxCode.taxAllAtBand].percentageAsDecimal) + } + is KTaxCode -> { + val taxBands = TaxBands.getAdjustedBands(taxYear, taxCode) + getTotalFromBands(taxBands, yearlyWages + taxCode.amountToAddToWages) + } else -> throw InvalidTaxCodeException("$this is an invalid tax code") } } @@ -162,23 +167,6 @@ class Calculator @JvmOverloads constructor( return taxToPayForSingleBand } - private fun adjustTaxBands(taxBands: List, taxCode: TaxCode): List { - // The full tax free amount e.g. 12509 - val bandAdjuster = getDefaultTaxAllowance(taxYear, taxCode.country) - - taxBands[0].upper = taxCode.taxFreeAmount - taxBands[1].lower = taxCode.taxFreeAmount - taxBands[1].upper = taxBands[1].upper + taxCode.taxFreeAmount - bandAdjuster - - for (bandNumber in 2 until taxBands.size) { - taxBands[bandNumber].lower = taxBands[bandNumber].lower + taxCode.taxFreeAmount - bandAdjuster - if (taxBands[bandNumber].upper != -1.0) { - taxBands[bandNumber].upper = taxBands[bandNumber].upper + taxCode.taxFreeAmount - bandAdjuster - } - } - return taxBands - } - private fun employerNIToPay(yearlyWages: Double) = if (isPensionAge) 0.0 else getTotalFromBands(EmployerNIBands(taxYear).bands, yearlyWages) @@ -212,13 +200,8 @@ class Calculator @JvmOverloads constructor( fun getDefaultTaxCode(): String { val taxYear = TaxYear().currentTaxYear() - val defaultTaxAllowance = getDefaultTaxAllowance(taxYear) + val defaultTaxAllowance = TaxBands.getBands(taxYear, ENGLAND)[0].upper.toInt() return "${(defaultTaxAllowance / 10)}L" } - - internal fun getDefaultTaxAllowance( - taxYear: Int, - country: Country = ENGLAND - ) = TaxBands.getBands(taxYear, country)[0].upper.toInt() } } diff --git a/src/commonMain/kotlin/uk/gov/hmrc/calculator/model/bands/TaxBands.kt b/src/commonMain/kotlin/uk/gov/hmrc/calculator/model/bands/TaxBands.kt index 5649c81..760f611 100644 --- a/src/commonMain/kotlin/uk/gov/hmrc/calculator/model/bands/TaxBands.kt +++ b/src/commonMain/kotlin/uk/gov/hmrc/calculator/model/bands/TaxBands.kt @@ -18,31 +18,46 @@ package uk.gov.hmrc.calculator.model.bands import uk.gov.hmrc.calculator.exception.InvalidTaxYearException import uk.gov.hmrc.calculator.model.Country import uk.gov.hmrc.calculator.model.Country.SCOTLAND +import uk.gov.hmrc.calculator.model.taxcodes.TaxCode internal object TaxBands { - private object Year2020 { - val scotland: List = listOf( - TaxBand(0.0, 12509.00, 0.0), - TaxBand(12509.00, 14585.00, 0.19), - TaxBand(14585.00, 25158.00, 0.20), - TaxBand(25158.00, 43430.00, 0.21), - TaxBand(43430.00, 150000.00, 0.41), - TaxBand(150000.0, -1.0, 0.46) - ) - val other: List = listOf( - TaxBand(0.0, 12509.00, 0.0), - TaxBand(12509.0, 50000.00, 0.2), - TaxBand(50000.0, 150000.00, 0.4), - TaxBand(150000.0, -1.0, 0.45) - ) - } - fun getBands(taxYear: Int, country: Country) = when (taxYear) { 2020 -> when (country) { - SCOTLAND -> Year2020.scotland - else -> Year2020.other + SCOTLAND -> listOf( + TaxBand(0.0, 12509.00, 0.0), + TaxBand(12509.00, 14585.00, 0.19), + TaxBand(14585.00, 25158.00, 0.20), + TaxBand(25158.00, 43430.00, 0.21), + TaxBand(43430.00, 150000.00, 0.41), + TaxBand(150000.0, -1.0, 0.46) + ) + else -> listOf( + TaxBand(0.0, 12509.00, 0.0), + TaxBand(12509.0, 50000.00, 0.2), + TaxBand(50000.0, 150000.00, 0.4), + TaxBand(150000.0, -1.0, 0.45) + ) } else -> throw InvalidTaxYearException("$taxYear") } + + fun getAdjustedBands(taxYear: Int, taxCode: TaxCode): List { + val taxBands = getBands(taxYear, taxCode.country).toMutableList() + + // The full tax free amount e.g. 12509 + val bandAdjuster = taxBands[0].upper.toInt() + + taxBands[0].upper = taxCode.taxFreeAmount + taxBands[1].lower = taxCode.taxFreeAmount + taxBands[1].upper = taxBands[1].upper + taxCode.taxFreeAmount - bandAdjuster + + for (bandNumber in 2 until taxBands.size) { + taxBands[bandNumber].lower = taxBands[bandNumber].lower + taxCode.taxFreeAmount - bandAdjuster + if (taxBands[bandNumber].upper != -1.0) { + taxBands[bandNumber].upper = taxBands[bandNumber].upper + taxCode.taxFreeAmount - bandAdjuster + } + } + return taxBands + } } diff --git a/src/jvmTest/kotlin/uk/gov/hmrc/calculator/CalculatorTests.kt b/src/commonTest/kotlin/uk/gov/hmrc/calculator/CalculatorTests.kt similarity index 76% rename from src/jvmTest/kotlin/uk/gov/hmrc/calculator/CalculatorTests.kt rename to src/commonTest/kotlin/uk/gov/hmrc/calculator/CalculatorTests.kt index 7adc0be..608090e 100644 --- a/src/jvmTest/kotlin/uk/gov/hmrc/calculator/CalculatorTests.kt +++ b/src/commonTest/kotlin/uk/gov/hmrc/calculator/CalculatorTests.kt @@ -15,9 +15,9 @@ */ package uk.gov.hmrc.calculator +import kotlin.test.Test +import kotlin.test.assertEquals import kotlin.test.assertFailsWith -import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Test import uk.gov.hmrc.calculator.exception.InvalidHoursException import uk.gov.hmrc.calculator.exception.InvalidWagesException import uk.gov.hmrc.calculator.model.PayPeriod @@ -27,35 +27,35 @@ internal class CalculatorTests { @Test fun `GIVEN hours is zero and pay period hour WHEN calculate THEN exception`() { assertFailsWith { - Calculator("1250L", 20.0, payPeriod = PayPeriod.HOURLY, howManyAWeek = 0.0).run() + Calculator(taxCode = "1250L", wages = 20.0, payPeriod = PayPeriod.HOURLY, howManyAWeek = 0.0).run() } } @Test fun `GIVEN hours is null and pay period hour WHEN calculate THEN exception`() { assertFailsWith { - Calculator("1250L", 20.0, payPeriod = PayPeriod.HOURLY).run() + Calculator(taxCode = "1250L", wages = 20.0, payPeriod = PayPeriod.HOURLY).run() } } @Test fun `GIVEN wages is below zero WHEN calculate THEN exception`() { assertFailsWith { - Calculator("1250L", -190.0, payPeriod = PayPeriod.WEEKLY).run() + Calculator(taxCode = "1250L", wages = -190.0, payPeriod = PayPeriod.WEEKLY).run() } } @Test fun `GIVEN wages is zero WHEN calculate THEN exception`() { assertFailsWith { - Calculator("1250L", 0.0, payPeriod = PayPeriod.YEARLY).run() + Calculator(taxCode = "1250L", wages = 0.0, payPeriod = PayPeriod.YEARLY).run() } } @Test fun `GIVEN wages too high WHEN calculate THEN exception`() { assertFailsWith { - Calculator("1250L", 10000000.0, payPeriod = PayPeriod.YEARLY).run() + Calculator(taxCode = "1250L", wages = 10000000.0, payPeriod = PayPeriod.YEARLY).run() } } diff --git a/src/commonTest/kotlin/uk/gov/hmrc/calculator/model/bands/TaxBandsTests.kt b/src/commonTest/kotlin/uk/gov/hmrc/calculator/model/bands/TaxBandsTests.kt index 701efc0..742f565 100644 --- a/src/commonTest/kotlin/uk/gov/hmrc/calculator/model/bands/TaxBandsTests.kt +++ b/src/commonTest/kotlin/uk/gov/hmrc/calculator/model/bands/TaxBandsTests.kt @@ -21,11 +21,13 @@ import kotlin.test.assertFailsWith import uk.gov.hmrc.calculator.exception.InvalidTaxYearException import uk.gov.hmrc.calculator.model.Country.ENGLAND import uk.gov.hmrc.calculator.model.Country.SCOTLAND +import uk.gov.hmrc.calculator.model.Country.WALES +import uk.gov.hmrc.calculator.utils.taxcode.toTaxCode class TaxBandsTests { @Test - fun invalidYear() { + fun `GIVEN invalid year WHEN get bands THEN fail with exception`() { val exception = assertFailsWith { TaxBands.getBands( 2017, @@ -36,17 +38,126 @@ class TaxBandsTests { } @Test - fun bandsForScotland2020() { - val taxBand = TaxBands.getBands(2020, SCOTLAND)[1] - assertEquals(14585.00, taxBand.upper) - assertEquals(12509.00, taxBand.lower) - assertEquals(0.19, taxBand.percentageAsDecimal) - - assertEquals(false, taxBand.inBand(12509.00)) - assertEquals(false, taxBand.inBand(12508.00)) - assertEquals(true, taxBand.inBand(12510.00)) - assertEquals(true, taxBand.inBand(14585.00)) - assertEquals(true, taxBand.inBand(14584.00)) - assertEquals(false, taxBand.inBand(14586.00)) + fun `GIVEN invalid year WHEN get adjusted bands THEN fail with exception`() { + val exception = assertFailsWith { + TaxBands.getAdjustedBands( + 2017, + "1250L".toTaxCode() + ) + } + assertEquals(exception.message, "2017") + } + + @Test + fun `GIVEN year is 2020 WHEN get bands for Scotland THEN bands are as expected`() { + val taxBands = TaxBands.getBands(2020, SCOTLAND) + + assertEquals(0.0, taxBands[0].lower) + assertEquals(12509.00, taxBands[0].upper) + assertEquals(0.0, taxBands[0].percentageAsDecimal) + + assertEquals(12509.00, taxBands[1].lower) + assertEquals(14585.00, taxBands[1].upper) + assertEquals(0.19, taxBands[1].percentageAsDecimal) + + assertEquals(14585.00, taxBands[2].lower) + assertEquals(25158.00, taxBands[2].upper) + assertEquals(0.20, taxBands[2].percentageAsDecimal) + + assertEquals(25158.00, taxBands[3].lower) + assertEquals(43430.00, taxBands[3].upper) + assertEquals(0.21, taxBands[3].percentageAsDecimal) + + assertEquals(43430.00, taxBands[4].lower) + assertEquals(150000.00, taxBands[4].upper) + assertEquals(0.41, taxBands[4].percentageAsDecimal) + + assertEquals(150000.00, taxBands[5].lower) + assertEquals(-1.0, taxBands[5].upper) + assertEquals(0.46, taxBands[5].percentageAsDecimal) + } + + @Test + fun `GIVEN year is 2020 WHEN get bands for ENGLAND THEN bands are as expected`() { + val taxBands = TaxBands.getBands(2020, ENGLAND) + + assertEquals(0.0, taxBands[0].lower) + assertEquals(12509.00, taxBands[0].upper) + assertEquals(0.0, taxBands[0].percentageAsDecimal) + + assertEquals(12509.00, taxBands[1].lower) + assertEquals(50000.0, taxBands[1].upper) + assertEquals(0.2, taxBands[1].percentageAsDecimal) + + assertEquals(50000.0, taxBands[2].lower) + assertEquals(150000.0, taxBands[2].upper) + assertEquals(0.4, taxBands[2].percentageAsDecimal) + + assertEquals(150000.0, taxBands[3].lower) + assertEquals(-1.0, taxBands[3].upper) + assertEquals(0.45, taxBands[3].percentageAsDecimal) + } + + @Test + fun `GIVEN year is 2020 WHEN get bands for WALES THEN bands are as expected`() { + val taxBands = TaxBands.getBands(2020, WALES) + + assertEquals(0.0, taxBands[0].lower) + assertEquals(12509.00, taxBands[0].upper) + assertEquals(0.0, taxBands[0].percentageAsDecimal) + + assertEquals(12509.00, taxBands[1].lower) + assertEquals(50000.0, taxBands[1].upper) + assertEquals(0.2, taxBands[1].percentageAsDecimal) + + assertEquals(50000.0, taxBands[2].lower) + assertEquals(150000.0, taxBands[2].upper) + assertEquals(0.4, taxBands[2].percentageAsDecimal) + + assertEquals(150000.0, taxBands[3].lower) + assertEquals(-1.0, taxBands[3].upper) + assertEquals(0.45, taxBands[3].percentageAsDecimal) + } + + @Test + fun `GIVEN year is 2020 WHEN get adjusted bands for 1250L THEN bands are as expected`() { + val taxBands = TaxBands.getAdjustedBands(2020, "1250L".toTaxCode()) + + assertEquals(0.0, taxBands[0].lower) + assertEquals(12509.00, taxBands[0].upper) + assertEquals(0.0, taxBands[0].percentageAsDecimal) + + assertEquals(12509.00, taxBands[1].lower) + assertEquals(50000.0, taxBands[1].upper) + assertEquals(0.2, taxBands[1].percentageAsDecimal) + + assertEquals(50000.0, taxBands[2].lower) + assertEquals(150000.0, taxBands[2].upper) + assertEquals(0.4, taxBands[2].percentageAsDecimal) + + assertEquals(150000.0, taxBands[3].lower) + assertEquals(-1.0, taxBands[3].upper) + assertEquals(0.45, taxBands[3].percentageAsDecimal) + } + + @Test + fun `GIVEN year is 2020 WHEN get adjusted bands for BR THEN bands are as expected`() { + val taxBands = TaxBands.getAdjustedBands(2020, "BR".toTaxCode()) + + assertEquals(0.0, taxBands[0].lower) + assertEquals(0.0, taxBands[0].upper) + assertEquals(0.0, taxBands[0].percentageAsDecimal) + + assertEquals(0.0, taxBands[1].lower) + assertEquals(37491.0, taxBands[1].upper) + assertEquals(0.2, taxBands[1].percentageAsDecimal) + + assertEquals(37491.0, taxBands[2].lower) + assertEquals(137491.0, taxBands[2].upper) + assertEquals(0.4, taxBands[2].percentageAsDecimal) + + assertEquals(137491.0, taxBands[3].lower) + assertEquals(-1.0, taxBands[3].upper) + assertEquals(0.45, taxBands[3].percentageAsDecimal) } }