From 98d10475f2f272d7b593960e209107977e8b70b6 Mon Sep 17 00:00:00 2001 From: marcusblevin Date: Mon, 8 Oct 2018 15:38:50 -0400 Subject: [PATCH] SUMIFS sum values only once Values were summed multiple times if it matched several conditions whereas it should only be summed once. Fixes #704 Fixes #710 --- CHANGELOG.md | 3 +- src/PhpSpreadsheet/Calculation/MathTrig.php | 26 +++++++---- .../Calculation/MathTrigTest.php | 16 +++++++ tests/data/Calculation/MathTrig/SUMIFS.php | 44 +++++++++++++++++++ 4 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 tests/data/Calculation/MathTrig/SUMIFS.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 62c95615..a12da59b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed -- Support numeric condition in SUMIF, SUMIFS, AVERAGEIF, COUNTIF, MAXIF and MINIF [#683](https://github.com/PHPOffice/PhpSpreadsheet/issues/683) +- Support numeric condition in SUMIF, SUMIFS, AVERAGEIF, COUNTIF, MAXIF and MINIF - [#683](https://github.com/PHPOffice/PhpSpreadsheet/issues/683) +- SUMIFS containing multiple conditions - [#704](https://github.com/PHPOffice/PhpSpreadsheet/issues/704) ## [1.5.0] - 2018-10-21 diff --git a/src/PhpSpreadsheet/Calculation/MathTrig.php b/src/PhpSpreadsheet/Calculation/MathTrig.php index e8a0397e..666ca798 100644 --- a/src/PhpSpreadsheet/Calculation/MathTrig.php +++ b/src/PhpSpreadsheet/Calculation/MathTrig.php @@ -1256,27 +1256,37 @@ class MathTrig $returnValue = 0; $sumArgs = Functions::flattenArray(array_shift($arrayList)); + $aArgsArray = []; + $conditions = []; while (count($arrayList) > 0) { $aArgsArray[] = Functions::flattenArray(array_shift($arrayList)); $conditions[] = Functions::ifCondition(array_shift($arrayList)); } - // Loop through each set of arguments and conditions - foreach ($conditions as $index => $condition) { - $aArgs = $aArgsArray[$index]; + // Loop through each sum and see if arguments and conditions are true + foreach ($sumArgs as $index => $value) { + $valid = true; - // Loop through arguments - foreach ($aArgs as $key => $arg) { + foreach ($conditions as $cidx => $condition) { + $arg = $aArgsArray[$cidx][$index]; + + // Loop through arguments if (!is_numeric($arg)) { $arg = Calculation::wrapResult(strtoupper($arg)); } $testCondition = '=' . $arg . $condition; - if (Calculation::getInstance()->_calculateFormulaValue($testCondition)) { - // Is it a value within our criteria - $returnValue += $sumArgs[$key]; + if (!Calculation::getInstance()->_calculateFormulaValue($testCondition)) { + // Is not a value within our criteria + $valid = false; + + break; // if false found, don't need to check other conditions } } + + if ($valid) { + $returnValue += $value; + } } // Return diff --git a/tests/PhpSpreadsheetTests/Calculation/MathTrigTest.php b/tests/PhpSpreadsheetTests/Calculation/MathTrigTest.php index 11c958fb..8179c367 100644 --- a/tests/PhpSpreadsheetTests/Calculation/MathTrigTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/MathTrigTest.php @@ -568,6 +568,22 @@ class MathTrigTest extends TestCase return require 'data/Calculation/MathTrig/SUMIF.php'; } + /** + * @dataProvider providerSUMIFS + * + * @param mixed $expectedResult + */ + public function testSUMIFS($expectedResult, ...$args) + { + $result = MathTrig::SUMIFS(...$args); + self::assertEquals($expectedResult, $result, '', 1E-12); + } + + public function providerSUMIFS() + { + return require 'data/Calculation/MathTrig/SUMIFS.php'; + } + /** * @dataProvider providerSUBTOTAL * diff --git a/tests/data/Calculation/MathTrig/SUMIFS.php b/tests/data/Calculation/MathTrig/SUMIFS.php new file mode 100644 index 00000000..a374dd14 --- /dev/null +++ b/tests/data/Calculation/MathTrig/SUMIFS.php @@ -0,0 +1,44 @@ +