aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--src/java/org/apache/poi/ss/formula/functions/Baseifs.java183
-rw-r--r--src/java/org/apache/poi/ss/formula/functions/Countifs.java42
-rw-r--r--src/java/org/apache/poi/ss/formula/functions/Sumifs.java136
-rw-r--r--src/ooxml/testcases/org/apache/poi/ss/formula/functions/CountifsTests.java (renamed from src/testcases/org/apache/poi/ss/formula/functions/CountifsTests.java)63
-rw-r--r--test-data/spreadsheet/56822-Countifs.xlsxbin0 -> 6686 bytes
5 files changed, 262 insertions, 162 deletions
diff --git a/src/java/org/apache/poi/ss/formula/functions/Baseifs.java b/src/java/org/apache/poi/ss/formula/functions/Baseifs.java
new file mode 100644
index 0000000000..2fcc3c668b
--- /dev/null
+++ b/src/java/org/apache/poi/ss/formula/functions/Baseifs.java
@@ -0,0 +1,183 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ====================================================================
+ */
+
+package org.apache.poi.ss.formula.functions;
+
+import org.apache.poi.ss.formula.OperationEvaluationContext;
+import org.apache.poi.ss.formula.eval.AreaEval;
+import org.apache.poi.ss.formula.eval.ErrorEval;
+import org.apache.poi.ss.formula.eval.EvaluationException;
+import org.apache.poi.ss.formula.eval.NumberEval;
+import org.apache.poi.ss.formula.eval.RefEval;
+import org.apache.poi.ss.formula.eval.ValueEval;
+import org.apache.poi.ss.formula.functions.CountUtils.I_MatchPredicate;
+import org.apache.poi.ss.formula.functions.Countif.ErrorMatcher;
+
+/**
+ * Base class for SUMIFS() and COUNTIFS() functions, as they share much of the same logic,
+ * the difference being the source of the totals.
+ */
+/*package*/ abstract class Baseifs implements FreeRefFunction {
+
+ /**
+ * Implementations must be stateless.
+ * @return true if there should be a range argument before the criteria pairs
+ */
+ protected abstract boolean hasInitialRange();
+
+ public ValueEval evaluate(ValueEval[] args, OperationEvaluationContext ec) {
+ final boolean hasInitialRange = hasInitialRange();
+ final int firstCriteria = hasInitialRange ? 1 : 0;
+
+ if( args.length < (2+firstCriteria) || args.length % 2 != firstCriteria ) {
+ return ErrorEval.VALUE_INVALID;
+ }
+
+ try {
+ AreaEval sumRange = null;
+ if (hasInitialRange) {
+ sumRange = convertRangeArg(args[0]);
+ }
+
+ // collect pairs of ranges and criteria
+ AreaEval[] ae = new AreaEval[(args.length - firstCriteria)/2];
+ I_MatchPredicate[] mp = new I_MatchPredicate[ae.length];
+ for(int i = firstCriteria, k=0; i < args.length; i += 2, k++){
+ ae[k] = convertRangeArg(args[i]);
+
+ mp[k] = Countif.createCriteriaPredicate(args[i+1], ec.getRowIndex(), ec.getColumnIndex());
+ }
+
+ validateCriteriaRanges(sumRange, ae);
+ validateCriteria(mp);
+
+ double result = aggregateMatchingCells(sumRange, ae, mp);
+ return new NumberEval(result);
+ } catch (EvaluationException e) {
+ return e.getErrorEval();
+ }
+ }
+
+ /**
+ * Verify that each <code>criteriaRanges</code> argument contains the same number of rows and columns
+ * including the <code>sumRange</code> argument if present
+ * @param sumRange if used, it must match the shape of the criteriaRanges
+ * @param criteriaRanges to check
+ * @throws EvaluationException if the ranges do not match.
+ */
+ private static void validateCriteriaRanges(AreaEval sumRange, AreaEval[] criteriaRanges) throws EvaluationException {
+ int h = criteriaRanges[0].getHeight();
+ int w = criteriaRanges[0].getWidth();
+
+ if (sumRange != null
+ && (sumRange.getHeight() != h
+ || sumRange.getWidth() != w) ) {
+ throw EvaluationException.invalidValue();
+ }
+
+ for(AreaEval r : criteriaRanges){
+ if(r.getHeight() != h ||
+ r.getWidth() != w ) {
+ throw EvaluationException.invalidValue();
+ }
+ }
+ }
+
+ /**
+ * Verify that each <code>criteria</code> predicate is valid, i.e. not an error
+ * @param criteria to check
+ *
+ * @throws EvaluationException if there are criteria which resulted in Errors.
+ */
+ private static void validateCriteria(I_MatchPredicate[] criteria) throws EvaluationException {
+ for(I_MatchPredicate predicate : criteria) {
+
+ // check for errors in predicate and return immediately using this error code
+ if(predicate instanceof ErrorMatcher) {
+ throw new EvaluationException(ErrorEval.valueOf(((ErrorMatcher)predicate).getValue()));
+ }
+ }
+ }
+
+
+ /**
+ * @param sumRange the range to sum, if used (uses 1 for each match if not present)
+ * @param ranges criteria ranges
+ * @param predicates array of predicates, a predicate for each value in <code>ranges</code>
+ * @return the computed value
+ */
+ private static double aggregateMatchingCells(AreaEval sumRange, AreaEval[] ranges, I_MatchPredicate[] predicates) {
+ int height = ranges[0].getHeight();
+ int width = ranges[0].getWidth();
+
+ double result = 0.0;
+ for (int r = 0; r < height; r++) {
+ for (int c = 0; c < width; c++) {
+
+ boolean matches = true;
+ for(int i = 0; i < ranges.length; i++){
+ AreaEval aeRange = ranges[i];
+ I_MatchPredicate mp = predicates[i];
+
+ if (!mp.matches(aeRange.getRelativeValue(r, c))) {
+ matches = false;
+ break;
+ }
+
+ }
+
+ if(matches) { // sum only if all of the corresponding criteria specified are true for that cell.
+ result += accumulate(sumRange, r, c);
+ }
+ }
+ }
+ return result;
+ }
+
+ /**
+ * For counts, this would return 1, for sums it returns a cell value or zero.
+ * This is only called after all the criteria are confirmed true for the coordinates.
+ * @param sumRange if used
+ * @param relRowIndex
+ * @param relColIndex
+ * @return the aggregate input value corresponding to the given range coordinates
+ */
+ private static double accumulate(AreaEval sumRange, int relRowIndex, int relColIndex) {
+ if (sumRange == null) return 1.0; // count
+
+ ValueEval addend = sumRange.getRelativeValue(relRowIndex, relColIndex);
+ if (addend instanceof NumberEval) {
+ return ((NumberEval)addend).getNumberValue();
+ }
+ // everything else (including string and boolean values) counts as zero
+ return 0.0;
+
+ }
+
+ protected static AreaEval convertRangeArg(ValueEval eval) throws EvaluationException {
+ if (eval instanceof AreaEval) {
+ return (AreaEval) eval;
+ }
+ if (eval instanceof RefEval) {
+ return ((RefEval)eval).offset(0, 0, 0, 0);
+ }
+ throw new EvaluationException(ErrorEval.VALUE_INVALID);
+ }
+
+}
diff --git a/src/java/org/apache/poi/ss/formula/functions/Countifs.java b/src/java/org/apache/poi/ss/formula/functions/Countifs.java
index 15e6a1521e..0ac50635aa 100644
--- a/src/java/org/apache/poi/ss/formula/functions/Countifs.java
+++ b/src/java/org/apache/poi/ss/formula/functions/Countifs.java
@@ -18,11 +18,6 @@
package org.apache.poi.ss.formula.functions;
-import org.apache.poi.ss.formula.OperationEvaluationContext;
-import org.apache.poi.ss.formula.eval.ErrorEval;
-import org.apache.poi.ss.formula.eval.NumberEval;
-import org.apache.poi.ss.formula.eval.ValueEval;
-
/**
* Implementation for the function COUNTIFS
* <p>
@@ -30,33 +25,20 @@ import org.apache.poi.ss.formula.eval.ValueEval;
* </p>
*/
-public class Countifs implements FreeRefFunction {
+public class Countifs extends Baseifs {
+ /**
+ * Singleton
+ */
public static final FreeRefFunction instance = new Countifs();
- public ValueEval evaluate(ValueEval[] args, OperationEvaluationContext ec) {
- // https://support.office.com/en-us/article/COUNTIFS-function-dda3dc6e-f74e-4aee-88bc-aa8c2a866842?ui=en-US&rs=en-US&ad=US
- // COUNTIFS(criteria_range1, criteria1, [criteria_range2, criteria2]...)
- // need at least 2 arguments and need to have an even number of arguments (criteria_range1, criteria1 plus x*(criteria_range, criteria))
- if (args.length < 2 || args.length % 2 != 0) {
- return ErrorEval.VALUE_INVALID;
- }
-
- Double result = null;
- // for each (criteria_range, criteria) pair
- for (int i = 0; i < args.length; i += 2) {
- ValueEval firstArg = args[i];
- ValueEval secondArg = args[i + 1];
- NumberEval evaluate = (NumberEval) new Countif().evaluate(
- new ValueEval[] {firstArg, secondArg},
- ec.getRowIndex(),
- ec.getColumnIndex());
- if (result == null) {
- result = evaluate.getNumberValue();
- } else if (evaluate.getNumberValue() < result) {
- result = evaluate.getNumberValue();
- }
- }
- return new NumberEval(result == null ? 0 : result);
+ /**
+ * https://support.office.com/en-us/article/COUNTIFS-function-dda3dc6e-f74e-4aee-88bc-aa8c2a866842?ui=en-US&rs=en-US&ad=US
+ * COUNTIFS(criteria_range1, criteria1, [criteria_range2, criteria2]...)
+ * need at least 2 arguments and need to have an even number of arguments (criteria_range1, criteria1 plus x*(criteria_range, criteria))
+ * @see org.apache.poi.ss.formula.functions.Baseifs#hasInitialRange()
+ */
+ protected boolean hasInitialRange() {
+ return false;
}
}
diff --git a/src/java/org/apache/poi/ss/formula/functions/Sumifs.java b/src/java/org/apache/poi/ss/formula/functions/Sumifs.java
index 81d0003fc5..edc138a437 100644
--- a/src/java/org/apache/poi/ss/formula/functions/Sumifs.java
+++ b/src/java/org/apache/poi/ss/formula/functions/Sumifs.java
@@ -19,16 +19,6 @@
package org.apache.poi.ss.formula.functions;
-import org.apache.poi.ss.formula.OperationEvaluationContext;
-import org.apache.poi.ss.formula.eval.AreaEval;
-import org.apache.poi.ss.formula.eval.ErrorEval;
-import org.apache.poi.ss.formula.eval.EvaluationException;
-import org.apache.poi.ss.formula.eval.NumberEval;
-import org.apache.poi.ss.formula.eval.RefEval;
-import org.apache.poi.ss.formula.eval.ValueEval;
-import org.apache.poi.ss.formula.functions.CountUtils.I_MatchPredicate;
-import org.apache.poi.ss.formula.functions.Countif.ErrorMatcher;
-
/**
* Implementation for the Excel function SUMIFS<p>
*
@@ -48,127 +38,21 @@ import org.apache.poi.ss.formula.functions.Countif.ErrorMatcher;
* </ul>
* </p>
*
- * @author Yegor Kozlov
*/
-public final class Sumifs implements FreeRefFunction {
- public static final FreeRefFunction instance = new Sumifs();
-
- public ValueEval evaluate(ValueEval[] args, OperationEvaluationContext ec) {
- // https://support.office.com/en-us/article/SUMIFS-function-c9e748f5-7ea7-455d-9406-611cebce642b
- // COUNTIFS(sum_range, criteria_range1, criteria1, [criteria_range2, criteria2], ...
- // need at least 3 arguments and need to have an odd number of arguments (sum-range plus x*(criteria_range, criteria))
- if(args.length < 3 || args.length % 2 == 0) {
- return ErrorEval.VALUE_INVALID;
- }
-
- try {
- AreaEval sumRange = convertRangeArg(args[0]);
-
- // collect pairs of ranges and criteria
- AreaEval[] ae = new AreaEval[(args.length - 1)/2];
- I_MatchPredicate[] mp = new I_MatchPredicate[ae.length];
- for(int i = 1, k=0; i < args.length; i += 2, k++){
- ae[k] = convertRangeArg(args[i]);
-
- mp[k] = Countif.createCriteriaPredicate(args[i+1], ec.getRowIndex(), ec.getColumnIndex());
- }
-
- validateCriteriaRanges(ae, sumRange);
- validateCriteria(mp);
-
- double result = sumMatchingCells(ae, mp, sumRange);
- return new NumberEval(result);
- } catch (EvaluationException e) {
- return e.getErrorEval();
- }
- }
-
+public final class Sumifs extends Baseifs {
/**
- * Verify that each <code>criteriaRanges</code> argument contains the same number of rows and columns
- * as the <code>sumRange</code> argument
- *
- * @throws EvaluationException if the ranges do not match.
+ * Singleton
*/
- private void validateCriteriaRanges(AreaEval[] criteriaRanges, AreaEval sumRange) throws EvaluationException {
- for(AreaEval r : criteriaRanges){
- if(r.getHeight() != sumRange.getHeight() ||
- r.getWidth() != sumRange.getWidth() ) {
- throw EvaluationException.invalidValue();
- }
- }
- }
-
- /**
- * Verify that each <code>criteria</code> predicate is valid, i.e. not an error
- *
- * @throws EvaluationException if there are criteria which resulted in Errors.
- */
- private void validateCriteria(I_MatchPredicate[] criteria) throws EvaluationException {
- for(I_MatchPredicate predicate : criteria) {
-
- // check for errors in predicate and return immediately using this error code
- if(predicate instanceof ErrorMatcher) {
- throw new EvaluationException(ErrorEval.valueOf(((ErrorMatcher)predicate).getValue()));
- }
- }
- }
-
+ public static final FreeRefFunction instance = new Sumifs();
/**
- *
- * @param ranges criteria ranges, each range must be of the same dimensions as <code>aeSum</code>
- * @param predicates array of predicates, a predicate for each value in <code>ranges</code>
- * @param aeSum the range to sum
- *
- * @return the computed value
+ * https://support.office.com/en-us/article/SUMIFS-function-c9e748f5-7ea7-455d-9406-611cebce642b
+ * COUNTIFS(sum_range, criteria_range1, criteria1, [criteria_range2, criteria2], ...
+ * need at least 3 arguments and need to have an odd number of arguments (sum-range plus x*(criteria_range, criteria))
+ * @see org.apache.poi.ss.formula.functions.Baseifs#hasInitialRange()
*/
- private static double sumMatchingCells(AreaEval[] ranges, I_MatchPredicate[] predicates, AreaEval aeSum) {
- int height = aeSum.getHeight();
- int width = aeSum.getWidth();
-
- double result = 0.0;
- for (int r = 0; r < height; r++) {
- for (int c = 0; c < width; c++) {
-
- boolean matches = true;
- for(int i = 0; i < ranges.length; i++){
- AreaEval aeRange = ranges[i];
- I_MatchPredicate mp = predicates[i];
-
- if (!mp.matches(aeRange.getRelativeValue(r, c))) {
- matches = false;
- break;
- }
-
- }
-
- if(matches) { // sum only if all of the corresponding criteria specified are true for that cell.
- result += accumulate(aeSum, r, c);
- }
- }
- }
- return result;
+ @Override
+ protected boolean hasInitialRange() {
+ return true;
}
-
- private static double accumulate(AreaEval aeSum, int relRowIndex,
- int relColIndex) {
-
- ValueEval addend = aeSum.getRelativeValue(relRowIndex, relColIndex);
- if (addend instanceof NumberEval) {
- return ((NumberEval)addend).getNumberValue();
- }
- // everything else (including string and boolean values) counts as zero
- return 0.0;
- }
-
- private static AreaEval convertRangeArg(ValueEval eval) throws EvaluationException {
- if (eval instanceof AreaEval) {
- return (AreaEval) eval;
- }
- if (eval instanceof RefEval) {
- return ((RefEval)eval).offset(0, 0, 0, 0);
- }
- throw new EvaluationException(ErrorEval.VALUE_INVALID);
- }
-
}
diff --git a/src/testcases/org/apache/poi/ss/formula/functions/CountifsTests.java b/src/ooxml/testcases/org/apache/poi/ss/formula/functions/CountifsTests.java
index 5dfc15ad67..3b3d71465e 100644
--- a/src/testcases/org/apache/poi/ss/formula/functions/CountifsTests.java
+++ b/src/ooxml/testcases/org/apache/poi/ss/formula/functions/CountifsTests.java
@@ -18,6 +18,9 @@
package org.apache.poi.ss.formula.functions;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
import org.apache.poi.hssf.usermodel.HSSFWorkbook;
import org.apache.poi.ss.usermodel.Cell;
import org.apache.poi.ss.usermodel.CellType;
@@ -25,13 +28,44 @@ import org.apache.poi.ss.usermodel.CellValue;
import org.apache.poi.ss.usermodel.FormulaEvaluator;
import org.apache.poi.ss.usermodel.Row;
import org.apache.poi.ss.usermodel.Sheet;
+import org.apache.poi.ss.usermodel.Workbook;
+import org.apache.poi.ss.util.SheetUtil;
+import org.apache.poi.util.IOUtils;
+import org.apache.poi.xssf.XSSFTestDataSamples;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
-import junit.framework.TestCase;
-
-public class CountifsTests extends TestCase {
+/**
+ * Test the COUNTIFS() function
+ */
+public class CountifsTests {
+ private Workbook workbook;
+
+ /**
+ * initialize a workbook
+ */
+ @Before
+ public void before() {
+ // not sure why we allow this, COUNTIFS() is only available
+ // in OOXML, it was introduced with Office 2007
+ workbook = new HSSFWorkbook();
+ }
+
+ /**
+ * Close the workbook if needed
+ */
+ @After
+ public void after() {
+ IOUtils.closeQuietly(workbook);
+ }
+
+ /**
+ * Basic call
+ */
+ @Test
public void testCallFunction() {
- HSSFWorkbook workbook = new HSSFWorkbook();
Sheet sheet = workbook.createSheet("test");
Row row1 = sheet.createRow(0);
Cell cellA1 = row1.createCell(0, CellType.FORMULA);
@@ -47,11 +81,14 @@ public class CountifsTests extends TestCase {
cellA1.setCellFormula("COUNTIFS(B1:C1,1, D1:E1,2)");
FormulaEvaluator evaluator = workbook.getCreationHelper().createFormulaEvaluator();
CellValue evaluate = evaluator.evaluate(cellA1);
- assertEquals(1.0d, evaluate.getNumberValue());
+ assertEquals(1.0d, evaluate.getNumberValue(), 0.000000000000001);
}
+ /**
+ * Test argument count check
+ */
+ @Test
public void testCallFunction_invalidArgs() {
- HSSFWorkbook workbook = new HSSFWorkbook();
Sheet sheet = workbook.createSheet("test");
Row row1 = sheet.createRow(0);
Cell cellA1 = row1.createCell(0, CellType.FORMULA);
@@ -68,4 +105,18 @@ public class CountifsTests extends TestCase {
evaluate = evaluator.evaluate(cellA1);
assertEquals(15, evaluate.getErrorValue());
}
+
+ /**
+ * the bug returned the wrong count, this verifies the fix
+ * @throws Exception if the file can't be read
+ */
+ @Test
+ public void testBug56822() throws Exception {
+ workbook = XSSFTestDataSamples.openSampleWorkbook("56822-Countifs.xlsx");
+ FormulaEvaluator evaluator = workbook.getCreationHelper().createFormulaEvaluator();
+ Cell cell = SheetUtil.getCell(workbook.getSheetAt(0), 0, 3);
+ assertNotNull("Test workbook missing cell D1", cell);
+ CellValue evaluate = evaluator.evaluate(cell);
+ assertEquals(2.0d, evaluate.getNumberValue(), 0.00000000000001);
+ }
}
diff --git a/test-data/spreadsheet/56822-Countifs.xlsx b/test-data/spreadsheet/56822-Countifs.xlsx
new file mode 100644
index 0000000000..8444d198c0
--- /dev/null
+++ b/test-data/spreadsheet/56822-Countifs.xlsx
Binary files differ