From 788f79c1bbc57c7408f98969b27cd3778c687caa Mon Sep 17 00:00:00 2001 From: Paul Blacknell Date: Thu, 26 Sep 2019 14:58:28 +0100 Subject: [PATCH] Validate XIRR inputs and return correct error values Fix: Return #NUM! if values and dates contain a different number of values Fix: Return #NUM! if there is not at least one positive cash flow and one negative cash flow Fix: Return #NUM! if any number in dates precedes the starting date Fix: Return #NUM! if a result that works cannot be found after max iteration tries Fix: Correct DocBlocks for XIRR & XNPV Add: Validate XIRR with unit tests Closes #1177 --- CHANGELOG.md | 1 + src/PhpSpreadsheet/Calculation/Financial.php | 46 +++++++- .../Calculation/FinancialTest.php | 7 +- tests/data/Calculation/Financial/XIRR.php | 111 ++++++++---------- 4 files changed, 95 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d8cbad3..a113848d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Fix branch pruning handling of non boolean conditions [#1167](https://github.com/PHPOffice/PhpSpreadsheet/pull/1167) - Fix ODS Reader when no DC namespace are defined [#1182](https://github.com/PHPOffice/PhpSpreadsheet/pull/1182) - Fixed Functions->ifCondition for allowing <> and empty condition [#1206](https://github.com/PHPOffice/PhpSpreadsheet/pull/1206) +- Validate XIRR inputs and return correct error values [#1120](https://github.com/PHPOffice/PhpSpreadsheet/issues/1120) ## [1.9.0] - 2019-08-17 diff --git a/src/PhpSpreadsheet/Calculation/Financial.php b/src/PhpSpreadsheet/Calculation/Financial.php index dde87c1c..90bb36df 100644 --- a/src/PhpSpreadsheet/Calculation/Financial.php +++ b/src/PhpSpreadsheet/Calculation/Financial.php @@ -2148,7 +2148,7 @@ class Financial * The maturity date is the date when the Treasury bill expires. * @param int $price The Treasury bill's price per $100 face value * - * @return float + * @return float|mixed|string */ public static function TBILLYIELD($settlement, $maturity, $price) { @@ -2183,6 +2183,23 @@ class Financial return Functions::VALUE(); } + /** + * XIRR. + * + * Returns the internal rate of return for a schedule of cash flows that is not necessarily periodic. + * + * Excel Function: + * =XIRR(values,dates,guess) + * + * @param float[] $values A series of cash flow payments + * The series of values must contain at least one positive value & one negative value + * @param mixed[] $dates A series of payment dates + * The first payment date indicates the beginning of the schedule of payments + * All other dates must be later than this date, but they may occur in any order + * @param float $guess An optional guess at the expected answer + * + * @return float|mixed|string + */ public static function XIRR($values, $dates, $guess = 0.1) { if ((!is_array($values)) && (!is_array($dates))) { @@ -2195,11 +2212,28 @@ class Financial return Functions::NAN(); } + $datesCount = count($dates); + for ($i = 0; $i < $datesCount; ++$i) { + $dates[$i] = DateTime::getDateValue($dates[$i]); + if (!is_numeric($dates[$i])) { + return Functions::VALUE(); + } + } + if (min($dates) != $dates[0]) { + return Functions::NAN(); + } + // create an initial range, with a root somewhere between 0 and guess $x1 = 0.0; $x2 = $guess; $f1 = self::XNPV($x1, $values, $dates); + if (!is_numeric($f1)) { + return $f1; + } $f2 = self::XNPV($x2, $values, $dates); + if (!is_numeric($f2)) { + return $f2; + } for ($i = 0; $i < self::FINANCIAL_MAX_ITERATIONS; ++$i) { if (($f1 * $f2) < 0.0) { break; @@ -2210,7 +2244,7 @@ class Financial } } if (($f1 * $f2) > 0.0) { - return Functions::VALUE(); + return Functions::NAN(); } $f = self::XNPV($x1, $values, $dates); @@ -2247,15 +2281,15 @@ class Financial * =XNPV(rate,values,dates) * * @param float $rate the discount rate to apply to the cash flows - * @param array of float $values A series of cash flows that corresponds to a schedule of payments in dates. + * @param float[] $values A series of cash flows that corresponds to a schedule of payments in dates. * The first payment is optional and corresponds to a cost or payment that occurs at the beginning of the investment. * If the first value is a cost or payment, it must be a negative value. All succeeding payments are discounted based on a 365-day year. * The series of values must contain at least one positive value and one negative value. - * @param array of mixed $dates A schedule of payment dates that corresponds to the cash flow payments. + * @param mixed[] $dates A schedule of payment dates that corresponds to the cash flow payments. * The first payment date indicates the beginning of the schedule of payments. * All other dates must be later than this date, but they may occur in any order. * - * @return float + * @return float|mixed|string */ public static function XNPV($rate, $values, $dates) { @@ -2273,7 +2307,7 @@ class Financial return Functions::NAN(); } if ((min($values) > 0) || (max($values) < 0)) { - return Functions::VALUE(); + return Functions::NAN(); } $xnpv = 0.0; diff --git a/tests/PhpSpreadsheetTests/Calculation/FinancialTest.php b/tests/PhpSpreadsheetTests/Calculation/FinancialTest.php index d98d91c5..e9418454 100644 --- a/tests/PhpSpreadsheetTests/Calculation/FinancialTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/FinancialTest.php @@ -501,13 +501,12 @@ class FinancialTest extends TestCase * @dataProvider providerXIRR * * @param mixed $expectedResult + * @param mixed $message */ - public function testXIRR($expectedResult, ...$args) + public function testXIRR($expectedResult, $message, ...$args) { - $this->markTestIncomplete('TODO: This test should be fixed'); - $result = Financial::XIRR(...$args); - self::assertEquals($expectedResult, $result, '', 1E-8); + self::assertEquals($expectedResult, $result, $message, Financial::FINANCIAL_PRECISION); } public function providerXIRR() diff --git a/tests/data/Calculation/Financial/XIRR.php b/tests/data/Calculation/Financial/XIRR.php index b4727c57..8c254131 100644 --- a/tests/data/Calculation/Financial/XIRR.php +++ b/tests/data/Calculation/Financial/XIRR.php @@ -1,71 +1,62 @@