Skip to content

Commit

Permalink
SUMIFS sum values only once
Browse files Browse the repository at this point in the history
Values were summed multiple times if it matched several conditions
whereas it should only be summed once.

Fixes #704
Fixes #710
  • Loading branch information
marcusblevin authored and PowerKiKi committed Oct 28, 2018
1 parent ed6a3a0 commit 98d1047
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 9 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
26 changes: 18 additions & 8 deletions src/PhpSpreadsheet/Calculation/MathTrig.php
Original file line number Diff line number Diff line change
Expand Up @@ -1256,27 +1256,37 @@ public static function SUMIFS(...$args)
$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
Expand Down
16 changes: 16 additions & 0 deletions tests/PhpSpreadsheetTests/Calculation/MathTrigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,22 @@ public function providerSUMIF()
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
*
Expand Down
44 changes: 44 additions & 0 deletions tests/data/Calculation/MathTrig/SUMIFS.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

return [
[
2,
[
[1],
[1],
[1],
],
[
['Y'],
['Y'],
['N'],
],
'=Y',
[
['H'],
['H'],
['H'],
],
'=H',
],
[
1,
[
[1],
[1],
[1],
],
[
['A'],
['B'],
['C'],
],
'=B',
[
['C'],
['B'],
['A'],
],
'=B',
],
];

0 comments on commit 98d1047

Please sign in to comment.