Improvements to countif implementation in preparation for sumif

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@734243 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/hssf/record/formula/functions/Countif.java

@@ -24,6 +24,7 @@ import org.apache.poi.hssf.record.formula.eval.BlankEval;
 import org.apache.poi.hssf.record.formula.eval.BoolEval;
 import org.apache.poi.hssf.record.formula.eval.ErrorEval;
 import org.apache.poi.hssf.record.formula.eval.Eval;
+import org.apache.poi.hssf.record.formula.eval.EvaluationException;
 import org.apache.poi.hssf.record.formula.eval.NumberEval;
 import org.apache.poi.hssf.record.formula.eval.OperandResolver;
 import org.apache.poi.hssf.record.formula.eval.RefEval;
@@ -214,6 +215,24 @@ public final class Countif implements Function {
 			return _operator.evaluate(testValue - _value);
 		}
 	}
+	private static final class ErrorMatcher implements I_MatchPredicate {
+
+		private final int _value;
+		private final CmpOp _operator;
+
+		public ErrorMatcher(int errorCode, CmpOp operator) {
+			_value = errorCode;
+			_operator = operator;
+		}
+
+		public boolean matches(Eval x) {
+			if(x instanceof ErrorEval) {
+				int testValue = ((ErrorEval)x).getErrorCode();
+				return _operator.evaluate(testValue - _value);
+			}
+			return false;
+		}
+	}
 	private static final class StringMatcher implements I_MatchPredicate {
 
 		private final String _value;
@@ -322,7 +341,7 @@ public final class Countif implements Function {
 		}
 	}
 
-	public Eval evaluate(Eval[] args, int srcCellRow, short srcCellCol) {
+	public Eval evaluate(Eval[] args, int srcRowIndex, short srcColumnIndex) {
 		switch(args.length) {
 			case 2:
 				// expected
@@ -333,40 +352,35 @@ public final class Countif implements Function {
 				return ErrorEval.VALUE_INVALID;
 		}
 
-		Eval criteriaArg = args[1];
+		I_MatchPredicate mp = createCriteriaPredicate(args[1], srcRowIndex, srcColumnIndex);
-		if(criteriaArg instanceof RefEval) {
+		if(mp == null) {
-			// criteria is not a literal value, but a cell reference
-			// for example COUNTIF(B2:D4, E1)
-			RefEval re = (RefEval)criteriaArg;
-			criteriaArg = re.getInnerValueEval();
-		} else {
-			// other non literal tokens such as function calls, have been fully evaluated
-			// for example COUNTIF(B2:D4, COLUMN(E1))
-		}
-		if(criteriaArg instanceof BlankEval) {
 			// If the criteria arg is a reference to a blank cell, countif always returns zero.
 			return NumberEval.ZERO;
 		}
-		I_MatchPredicate mp = createCriteriaPredicate(criteriaArg);
+		double result = countMatchingCellsInArea(args[0], mp);
-		return countMatchingCellsInArea(args[0], mp);
+		return new NumberEval(result);
 	}
 	/**
 	 * @return the number of evaluated cells in the range that match the specified criteria
 	 */
-	private Eval countMatchingCellsInArea(Eval rangeArg, I_MatchPredicate criteriaPredicate) {
+	private double countMatchingCellsInArea(Eval rangeArg, I_MatchPredicate criteriaPredicate) {
 		
-		int result;
 		if (rangeArg instanceof RefEval) {
-			result = CountUtils.countMatchingCell((RefEval) rangeArg, criteriaPredicate);
+			return CountUtils.countMatchingCell((RefEval) rangeArg, criteriaPredicate);
 		} else if (rangeArg instanceof AreaEval) {
-			result = CountUtils.countMatchingCellsInArea((AreaEval) rangeArg, criteriaPredicate);
+			return CountUtils.countMatchingCellsInArea((AreaEval) rangeArg, criteriaPredicate);
 		} else {
 			throw new IllegalArgumentException("Bad range arg type (" + rangeArg.getClass().getName() + ")");
 		}
-		return new NumberEval(result);
 	}
 
-	/* package */ static I_MatchPredicate createCriteriaPredicate(Eval evaluatedCriteriaArg) {
+	/**
+	 * Creates a criteria predicate object for the supplied criteria arg
+	 * @return <code>null</code> if the arg evaluates to blank.
+	 */
+	/* package */ static I_MatchPredicate createCriteriaPredicate(Eval arg, int srcRowIndex, int srcColumnIndex) {
+
+		Eval evaluatedCriteriaArg = evaluateCriteriaArg(arg, srcRowIndex, srcColumnIndex);
 		
 		if(evaluatedCriteriaArg instanceof NumberEval) {
 			return new NumberMatcher(((NumberEval)evaluatedCriteriaArg).getNumberValue(), CmpOp.OP_NONE);
@@ -378,10 +392,27 @@ public final class Countif implements Function {
 		if(evaluatedCriteriaArg instanceof StringEval) {
 			return createGeneralMatchPredicate((StringEval)evaluatedCriteriaArg);
 		}
+		if(evaluatedCriteriaArg instanceof ErrorEval) {
+			return new ErrorMatcher(((ErrorEval)evaluatedCriteriaArg).getErrorCode(), CmpOp.OP_NONE);
+		}
+		if(evaluatedCriteriaArg == BlankEval.INSTANCE) {
+			return null;
+		}
 		throw new RuntimeException("Unexpected type for criteria (" 
 				+ evaluatedCriteriaArg.getClass().getName() + ")");
 	}
 
+	/**
+	 * 
+	 * @return the de-referenced criteria arg (possibly {@link ErrorEval})
+	 */
+	private static Eval evaluateCriteriaArg(Eval arg, int srcRowIndex, int srcColumnIndex) {
+		try {
+			return OperandResolver.getSingleValue(arg, srcRowIndex, (short)srcColumnIndex);
+		} catch (EvaluationException e) {
+			return e.getErrorEval();
+		}
+	}
 	/**
 	 * When the second argument is a string, many things are possible
 	 */
@@ -399,10 +430,28 @@ public final class Countif implements Function {
 		if(doubleVal != null) {
 			return new NumberMatcher(doubleVal.doubleValue(), operator);
 		}
+		ErrorEval ee = parseError(value);
+		if (ee != null) {
+			return new ErrorMatcher(ee.getErrorCode(), operator);
+		}
 
 		//else - just a plain string with no interpretation.
 		return new StringMatcher(value, operator);
 	}
+	private static ErrorEval parseError(String value) {
+		if (value.length() < 4 || value.charAt(0) != '#') {
+			return null;
+		}
+		if (value.equals("#NULL!"))  return ErrorEval.NULL_INTERSECTION;
+		if (value.equals("#DIV/0!")) return ErrorEval.DIV_ZERO;
+		if (value.equals("#VALUE!")) return ErrorEval.VALUE_INVALID;
+		if (value.equals("#REF!"))   return ErrorEval.REF_INVALID;
+		if (value.equals("#NAME?"))  return ErrorEval.NAME_INVALID;
+		if (value.equals("#NUM!"))   return ErrorEval.NUM_ERROR;
+		if (value.equals("#N/A"))    return ErrorEval.NA;
+
+		return null;
+	}
 	/**
 	 * Boolean literals ('TRUE', 'FALSE') treated similarly but NOT same as numbers. 
 	 */

src/testcases/org/apache/poi/hssf/record/formula/functions/TestCountFuncs.java

@@ -24,8 +24,10 @@ import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.record.formula.eval.AreaEval;
 import org.apache.poi.hssf.record.formula.eval.BlankEval;
 import org.apache.poi.hssf.record.formula.eval.BoolEval;
+import org.apache.poi.hssf.record.formula.eval.ErrorEval;
 import org.apache.poi.hssf.record.formula.eval.Eval;
 import org.apache.poi.hssf.record.formula.eval.NumberEval;
+import org.apache.poi.hssf.record.formula.eval.OperandResolver;
 import org.apache.poi.hssf.record.formula.eval.StringEval;
 import org.apache.poi.hssf.record.formula.eval.ValueEval;
 import org.apache.poi.hssf.record.formula.functions.CountUtils.I_MatchPredicate;
@@ -43,6 +45,8 @@ import org.apache.poi.ss.usermodel.CellValue;
  */
 public final class TestCountFuncs extends TestCase {
 	
+	private static final String NULL = null;
+
 	public void testCountA() {
 
 		Eval[] args;
@@ -142,89 +146,159 @@ public final class TestCountFuncs extends TestCase {
 		assertEquals(expected, result, 0);
 	}
 
-	public void testCountIfEmptyStringCriteria() {
+	private static I_MatchPredicate createCriteriaPredicate(Eval ev) {
+		return Countif.createCriteriaPredicate(ev, 0, 0);
+	}
+	
+	/**
+	 * the criteria arg is mostly handled by {@link OperandResolver#getSingleValue(Eval, int, short)}
+	 */
+	public void testCountifAreaCriteria() {
+		int srcColIx = 2; // anything but column A
+		
+		ValueEval v0 = new NumberEval(2.0);
+		ValueEval v1 = new StringEval("abc");
+		ValueEval v2 = ErrorEval.DIV_ZERO;
+		
+		AreaEval ev = EvalFactory.createAreaEval("A10:A12", new ValueEval[] { v0, v1, v2, });
+		
+		I_MatchPredicate mp;
+		mp = Countif.createCriteriaPredicate(ev, 9, srcColIx);
+		confirmPredicate(true, mp, srcColIx);
+		confirmPredicate(false, mp, "abc");
+		confirmPredicate(false, mp, ErrorEval.DIV_ZERO);
+		
+		mp = Countif.createCriteriaPredicate(ev, 10, srcColIx);
+		confirmPredicate(false, mp, srcColIx);
+		confirmPredicate(true, mp, "abc");
+		confirmPredicate(false, mp, ErrorEval.DIV_ZERO);
+		
+		mp = Countif.createCriteriaPredicate(ev, 11, srcColIx);
+		confirmPredicate(false, mp, srcColIx);
+		confirmPredicate(false, mp, "abc");
+		confirmPredicate(true, mp, ErrorEval.DIV_ZERO);
+		confirmPredicate(false, mp, ErrorEval.VALUE_INVALID);
+
+		// tricky: indexing outside of A10:A12 
+		// even this #VALUE! error gets used by COUNTIF as valid criteria
+		mp = Countif.createCriteriaPredicate(ev, 12, srcColIx);
+		confirmPredicate(false, mp, srcColIx);
+		confirmPredicate(false, mp, "abc");
+		confirmPredicate(false, mp, ErrorEval.DIV_ZERO);
+		confirmPredicate(true, mp, ErrorEval.VALUE_INVALID);
+	}
+	
+	public void testCountifEmptyStringCriteria() {
 		I_MatchPredicate mp;
 
 		// pred '=' matches blank cell but not empty string
-		mp = Countif.createCriteriaPredicate(new StringEval("="));
+		mp = createCriteriaPredicate(new StringEval("="));
 		confirmPredicate(false, mp, "");
-		confirmPredicate(true, mp, null);
+		confirmPredicate(true, mp, NULL);
 
 		// pred '' matches both blank cell but not empty string
-		mp = Countif.createCriteriaPredicate(new StringEval(""));
+		mp = createCriteriaPredicate(new StringEval(""));
 		confirmPredicate(true, mp, "");
-		confirmPredicate(true, mp, null);
+		confirmPredicate(true, mp, NULL);
 
 		// pred '<>' matches empty string but not blank cell
-		mp = Countif.createCriteriaPredicate(new StringEval("<>"));
+		mp = createCriteriaPredicate(new StringEval("<>"));
-		confirmPredicate(false, mp, null);
+		confirmPredicate(false, mp, NULL);
 		confirmPredicate(true, mp, "");
 	}
 
 	public void testCountifComparisons() {
 		I_MatchPredicate mp;
 
-		mp = Countif.createCriteriaPredicate(new StringEval(">5"));
+		mp = createCriteriaPredicate(new StringEval(">5"));
 		confirmPredicate(false, mp, 4);
 		confirmPredicate(false, mp, 5);
 		confirmPredicate(true, mp, 6);
 
-		mp = Countif.createCriteriaPredicate(new StringEval("<=5"));
+		mp = createCriteriaPredicate(new StringEval("<=5"));
 		confirmPredicate(true, mp, 4);
 		confirmPredicate(true, mp, 5);
 		confirmPredicate(false, mp, 6);
 		confirmPredicate(true, mp, "4.9");
 		confirmPredicate(false, mp, "4.9t");
 		confirmPredicate(false, mp, "5.1");
-		confirmPredicate(false, mp, null);
+		confirmPredicate(false, mp, NULL);
 
-		mp = Countif.createCriteriaPredicate(new StringEval("=abc"));
+		mp = createCriteriaPredicate(new StringEval("=abc"));
 		confirmPredicate(true, mp, "abc");
 
-		mp = Countif.createCriteriaPredicate(new StringEval("=42"));
+		mp = createCriteriaPredicate(new StringEval("=42"));
 		confirmPredicate(false, mp, 41);
 		confirmPredicate(true, mp, 42);
 		confirmPredicate(true, mp, "42");
 
-		mp = Countif.createCriteriaPredicate(new StringEval(">abc"));
+		mp = createCriteriaPredicate(new StringEval(">abc"));
 		confirmPredicate(false, mp, 4);
 		confirmPredicate(false, mp, "abc");
 		confirmPredicate(true, mp, "abd");
 
-		mp = Countif.createCriteriaPredicate(new StringEval(">4t3"));
+		mp = createCriteriaPredicate(new StringEval(">4t3"));
 		confirmPredicate(false, mp, 4);
 		confirmPredicate(false, mp, 500);
 		confirmPredicate(true, mp, "500");
 		confirmPredicate(true, mp, "4t4");
 	}
 	
+	/**
+	 * the criteria arg value can be an error code (the error does not
+	 * propagate to the COUNTIF result).
+	 */
+	public void testCountifErrorCriteria() {
+		I_MatchPredicate mp;
+
+		mp = createCriteriaPredicate(new StringEval("#REF!"));
+		confirmPredicate(false, mp, 4);
+		confirmPredicate(false, mp, "#REF!");
+		confirmPredicate(true, mp, ErrorEval.REF_INVALID);
+
+		mp = createCriteriaPredicate(new StringEval("<#VALUE!"));
+		confirmPredicate(false, mp, 4);
+		confirmPredicate(false, mp, "#DIV/0!");
+		confirmPredicate(false, mp, "#REF!");
+		confirmPredicate(true, mp, ErrorEval.DIV_ZERO);
+		confirmPredicate(false, mp, ErrorEval.REF_INVALID);
+		
+		// not quite an error literal, should be treated as plain text
+		mp = createCriteriaPredicate(new StringEval("<=#REF!a"));
+		confirmPredicate(false, mp, 4);
+		confirmPredicate(true, mp, "#DIV/0!");
+		confirmPredicate(true, mp, "#REF!");
+		confirmPredicate(false, mp, ErrorEval.DIV_ZERO);
+		confirmPredicate(false, mp, ErrorEval.REF_INVALID);
+	}
+
 	public void testWildCards() {
 		I_MatchPredicate mp;
 
-		mp = Countif.createCriteriaPredicate(new StringEval("a*b"));
+		mp = createCriteriaPredicate(new StringEval("a*b"));
 		confirmPredicate(false, mp, "abc");
 		confirmPredicate(true, mp, "ab");
 		confirmPredicate(true, mp, "axxb");
 		confirmPredicate(false, mp, "xab");
 
-		mp = Countif.createCriteriaPredicate(new StringEval("a?b"));
+		mp = createCriteriaPredicate(new StringEval("a?b"));
 		confirmPredicate(false, mp, "abc");
 		confirmPredicate(false, mp, "ab");
 		confirmPredicate(false, mp, "axxb");
 		confirmPredicate(false, mp, "xab");
 		confirmPredicate(true, mp, "axb");
 
-		mp = Countif.createCriteriaPredicate(new StringEval("a~?"));
+		mp = createCriteriaPredicate(new StringEval("a~?"));
 		confirmPredicate(false, mp, "a~a");
 		confirmPredicate(false, mp, "a~?");
 		confirmPredicate(true, mp, "a?");
 
-		mp = Countif.createCriteriaPredicate(new StringEval("~*a"));
+		mp = createCriteriaPredicate(new StringEval("~*a"));
 		confirmPredicate(false, mp, "~aa");
 		confirmPredicate(false, mp, "~*a");
 		confirmPredicate(true, mp, "*a");
 
-		mp = Countif.createCriteriaPredicate(new StringEval("12?12"));
+		mp = createCriteriaPredicate(new StringEval("12?12"));
 		confirmPredicate(false, mp, 12812);
 		confirmPredicate(true, mp, "12812");
 		confirmPredicate(false, mp, "128812");
@@ -233,18 +307,18 @@ public final class TestCountFuncs extends TestCase {
 		I_MatchPredicate mp;
 
 		// make sure special reg-ex chars are treated like normal chars
-		mp = Countif.createCriteriaPredicate(new StringEval("a.b"));
+		mp = createCriteriaPredicate(new StringEval("a.b"));
 		confirmPredicate(false, mp, "aab");
 		confirmPredicate(true, mp, "a.b");
 
 
-		mp = Countif.createCriteriaPredicate(new StringEval("a~b"));
+		mp = createCriteriaPredicate(new StringEval("a~b"));
 		confirmPredicate(false, mp, "ab");
 		confirmPredicate(false, mp, "axb");
 		confirmPredicate(false, mp, "a~~b");
 		confirmPredicate(true, mp, "a~b");
 
-		mp = Countif.createCriteriaPredicate(new StringEval(">a*b"));
+		mp = createCriteriaPredicate(new StringEval(">a*b"));
 		confirmPredicate(false, mp, "a(b");
 		confirmPredicate(true, mp, "aab");
 		confirmPredicate(false, mp, "a*a");
@@ -258,6 +332,9 @@ public final class TestCountFuncs extends TestCase {
 		Eval ev = value == null ? (Eval)BlankEval.INSTANCE : new StringEval(value);
 		assertEquals(expectedResult, matchPredicate.matches(ev));
 	}
+	private static void confirmPredicate(boolean expectedResult, I_MatchPredicate matchPredicate, ErrorEval value) {
+		assertEquals(expectedResult, matchPredicate.matches(value));
+	}
 
 	public void testCountifFromSpreadsheet() {
 		final String FILE_NAME = "countifExamples.xls";