criteriaRanges argument contains the same number of rows and columns
+ * including the sumRange 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 criteria 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 ranges
+ * @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
* @@ -30,33 +25,20 @@ import org.apache.poi.ss.formula.eval.ValueEval; *
*/ -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* @@ -48,127 +38,21 @@ import org.apache.poi.ss.formula.functions.Countif.ErrorMatcher; * *
* - * @author Yegor Kozlov */ -public final class Sumifs implements FreeRefFunction { +public final class Sumifs extends Baseifs { + /** + * Singleton + */ 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(); - } - } - /** - * Verify that eachcriteriaRanges argument contains the same number of rows and columns
- * as the sumRange argument
- *
- * @throws EvaluationException if the ranges do not match.
+ * 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 void validateCriteriaRanges(AreaEval[] criteriaRanges, AreaEval sumRange) throws EvaluationException {
- for(AreaEval r : criteriaRanges){
- if(r.getHeight() != sumRange.getHeight() ||
- r.getWidth() != sumRange.getWidth() ) {
- throw EvaluationException.invalidValue();
- }
- }
+ @Override
+ protected boolean hasInitialRange() {
+ return true;
}
-
- /**
- * Verify that each criteria 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()));
- }
- }
- }
-
-
- /**
- *
- * @param ranges criteria ranges, each range must be of the same dimensions as aeSum
- * @param predicates array of predicates, a predicate for each value in ranges
- * @param aeSum the range to sum
- *
- * @return the computed value
- */
- 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;
- }
-
- 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
similarity index 64%
rename from src/testcases/org/apache/poi/ss/formula/functions/CountifsTests.java
rename to 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
Binary files /dev/null and b/test-data/spreadsheet/56822-Countifs.xlsx differ