You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by jo...@apache.org on 2009/05/15 23:36:26 UTC
svn commit: r775360 - in /poi/trunk/src:
java/org/apache/poi/hssf/record/formula/functions/Index.java
testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls
testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java
Author: josh
Date: Fri May 15 21:36:25 2009
New Revision: 775360
URL: http://svn.apache.org/viewvc?rev=775360&view=rev
Log:
Fix for bug noticed while investigating bugzilla 47048. Made INDEX() support missing args.
Modified:
poi/trunk/src/java/org/apache/poi/hssf/record/formula/functions/Index.java
poi/trunk/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls
poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java
Modified: poi/trunk/src/java/org/apache/poi/hssf/record/formula/functions/Index.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/formula/functions/Index.java?rev=775360&r1=775359&r2=775360&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/formula/functions/Index.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/formula/functions/Index.java Fri May 15 21:36:25 2009
@@ -18,9 +18,11 @@
package org.apache.poi.hssf.record.formula.functions;
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.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.MissingArgEval;
import org.apache.poi.hssf.record.formula.eval.OperandResolver;
import org.apache.poi.hssf.record.formula.eval.RefEval;
import org.apache.poi.hssf.record.formula.eval.ValueEval;
@@ -28,7 +30,7 @@
/**
* Implementation for the Excel function INDEX
* <p>
- *
+ *
* Syntax : <br/>
* INDEX ( reference, row_num[, column_num [, area_num]])</br>
* INDEX ( array, row_num[, column_num])
@@ -40,12 +42,11 @@
* <tr><th>area_num</th><td>used when reference is a union of areas</td></tr>
* </table>
* </p>
- *
+ *
* @author Josh Micich
*/
public final class Index implements Function {
- // TODO - javadoc for interface method
public Eval evaluate(Eval[] args, int srcCellRow, short srcCellCol) {
int nArgs = args.length;
if(nArgs < 2) {
@@ -58,112 +59,141 @@
firstArg = ((RefEval)firstArg).offset(0, 0, 0, 0);
}
if(!(firstArg instanceof AreaEval)) {
-
+
// else the other variation of this function takes an array as the first argument
// it seems like interface 'ArrayEval' does not even exist yet
-
+
throw new RuntimeException("Incomplete code - cannot handle first arg of type ("
+ firstArg.getClass().getName() + ")");
}
AreaEval reference = (AreaEval) firstArg;
-
+
int rowIx = 0;
int columnIx = 0;
- int areaIx = 0;
- try {
+ boolean colArgWasPassed = false;
+ try {
switch(nArgs) {
case 4:
- areaIx = convertIndexArgToZeroBase(args[3], srcCellRow, srcCellCol);
throw new RuntimeException("Incomplete code" +
" - don't know how to support the 'area_num' parameter yet)");
// Excel expression might look like this "INDEX( (A1:B4, C3:D6, D2:E5 ), 1, 2, 3)
// In this example, the 3rd area would be used i.e. D2:E5, and the overall result would be E2
// Token array might be encoded like this: MemAreaPtg, AreaPtg, AreaPtg, UnionPtg, UnionPtg, ParenthesesPtg
// The formula parser doesn't seem to support this yet. Not sure if the evaluator does either
-
+
case 3:
- columnIx = convertIndexArgToZeroBase(args[2], srcCellRow, srcCellCol);
+ columnIx = resolveIndexArg(args[2], srcCellRow, srcCellCol);
+ colArgWasPassed = true;
case 2:
- rowIx = convertIndexArgToZeroBase(args[1], srcCellRow, srcCellCol);
+ rowIx = resolveIndexArg(args[1], srcCellRow, srcCellCol);
break;
default:
// too many arguments
return ErrorEval.VALUE_INVALID;
}
- return getValueFromArea(reference, rowIx, columnIx, nArgs);
+ return getValueFromArea(reference, rowIx, columnIx, colArgWasPassed, srcCellRow, srcCellCol);
} catch (EvaluationException e) {
return e.getErrorEval();
}
}
-
+
/**
- * @param nArgs - needed because error codes are slightly different when only 2 args are passed
+ * @param colArgWasPassed <code>false</code> if the INDEX argument list had just 2 items
+ * (exactly 1 comma). If anything is passed for the <tt>column_num</tt> argument
+ * (including {@link BlankEval} or {@link MissingArgEval}) this parameter will be
+ * <code>true</code>. This parameter is needed because error codes are slightly
+ * different when only 2 args are passed.
*/
- private static ValueEval getValueFromArea(AreaEval ae, int pRowIx, int pColumnIx, int nArgs) throws EvaluationException {
+ private static ValueEval getValueFromArea(AreaEval ae, int pRowIx, int pColumnIx,
+ boolean colArgWasPassed, int srcRowIx, int srcColIx) throws EvaluationException {
+ boolean rowArgWasEmpty = pRowIx == 0;
+ boolean colArgWasEmpty = pColumnIx == 0;
int rowIx;
int columnIx;
-
+
// when the area ref is a single row or a single column,
// there are special rules for conversion of rowIx and columnIx
if (ae.isRow()) {
if (ae.isColumn()) {
- rowIx = pRowIx == -1 ? 0 : pRowIx;
- columnIx = pColumnIx == -1 ? 0 : pColumnIx;
+ // single cell ref
+ rowIx = rowArgWasEmpty ? 0 : pRowIx-1;
+ columnIx = colArgWasEmpty ? 0 : pColumnIx-1;
} else {
- if (nArgs == 2) {
- rowIx = 0;
- columnIx = pRowIx;
+ if (colArgWasPassed) {
+ rowIx = rowArgWasEmpty ? 0 : pRowIx-1;
+ columnIx = pColumnIx-1;
} else {
- rowIx = pRowIx == -1 ? 0 : pRowIx;
- columnIx = pColumnIx;
+ // special case - row arg seems to get used as the column index
+ rowIx = 0;
+ // transfer both the index value and the empty flag from 'row' to 'column':
+ columnIx = pRowIx-1;
+ colArgWasEmpty = rowArgWasEmpty;
}
}
- if (rowIx < -1 || columnIx < -1) {
- throw new EvaluationException(ErrorEval.VALUE_INVALID);
- }
} else if (ae.isColumn()) {
- if (nArgs == 2) {
- rowIx = pRowIx;
- columnIx = 0;
+ if (rowArgWasEmpty) {
+ rowIx = srcRowIx - ae.getFirstRow();
} else {
- rowIx = pRowIx;
- columnIx = pColumnIx == -1 ? 0 : pColumnIx;
+ rowIx = pRowIx-1;
}
- if (rowIx < -1 || columnIx < -1) {
- throw new EvaluationException(ErrorEval.VALUE_INVALID);
+ if (colArgWasEmpty) {
+ columnIx = 0;
+ } else {
+ columnIx = colArgWasEmpty ? 0 : pColumnIx-1;
}
} else {
- if (nArgs == 2) {
+ // ae is an area (not single row or column)
+ if (!colArgWasPassed) {
// always an error with 2-D area refs
- if (pRowIx < -1) {
- throw new EvaluationException(ErrorEval.VALUE_INVALID);
- }
- throw new EvaluationException(ErrorEval.REF_INVALID);
+ // Note - the type of error changes if the pRowArg is negative
+ throw new EvaluationException(pRowIx < 0 ? ErrorEval.VALUE_INVALID : ErrorEval.REF_INVALID);
}
// Normal case - area ref is 2-D, and both index args were provided
- rowIx = pRowIx;
- columnIx = pColumnIx;
+ // if either arg is missing (or blank) the logic is similar to OperandResolver.getSingleValue()
+ if (rowArgWasEmpty) {
+ rowIx = srcRowIx - ae.getFirstRow();
+ } else {
+ rowIx = pRowIx-1;
+ }
+ if (colArgWasEmpty) {
+ columnIx = srcColIx - ae.getFirstColumn();
+ } else {
+ columnIx = pColumnIx-1;
+ }
}
-
+
int width = ae.getWidth();
int height = ae.getHeight();
// Slightly irregular logic for bounds checking errors
- if (rowIx >= height || columnIx >= width) {
+ if (!rowArgWasEmpty && rowIx >= height || !colArgWasEmpty && columnIx >= width) {
+ // high bounds check fail gives #REF! if arg was explicitly passed
throw new EvaluationException(ErrorEval.REF_INVALID);
}
- if (rowIx < 0 || columnIx < 0) {
+ if (rowIx < 0 || columnIx < 0 || rowIx >= height || columnIx >= width) {
throw new EvaluationException(ErrorEval.VALUE_INVALID);
}
return ae.getRelativeValue(rowIx, columnIx);
}
+
/**
- * takes a NumberEval representing a 1-based index and returns the zero-based int value
+ * @param arg a 1-based index.
+ * @return the resolved 1-based index. Zero if the arg was missing or blank
+ * @throws EvaluationException if the arg is an error value evaluates to a negative numeric value
*/
- private static int convertIndexArgToZeroBase(Eval arg, int srcCellRow, short srcCellCol) throws EvaluationException {
-
+ private static int resolveIndexArg(Eval arg, int srcCellRow, short srcCellCol) throws EvaluationException {
+
ValueEval ev = OperandResolver.getSingleValue(arg, srcCellRow, srcCellCol);
- int oneBasedVal = OperandResolver.coerceValueToInt(ev);
- return oneBasedVal - 1;
+ if (ev == MissingArgEval.instance) {
+ return 0;
+ }
+ if (ev == BlankEval.INSTANCE) {
+ return 0;
+ }
+ int result = OperandResolver.coerceValueToInt(ev);
+ if (result < 0) {
+ throw new EvaluationException(ErrorEval.VALUE_INVALID);
+ }
+ return result;
}
}
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls?rev=775360&r1=775359&r2=775360&view=diff
==============================================================================
Binary files - no diff available.
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java?rev=775360&r1=775359&r2=775360&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java Fri May 15 21:36:25 2009
@@ -17,20 +17,28 @@
package org.apache.poi.hssf.record.formula.functions;
+import junit.framework.AssertionFailedError;
import junit.framework.TestCase;
import org.apache.poi.hssf.record.formula.eval.AreaEval;
import org.apache.poi.hssf.record.formula.eval.Eval;
+import org.apache.poi.hssf.record.formula.eval.MissingArgEval;
import org.apache.poi.hssf.record.formula.eval.NumberEval;
import org.apache.poi.hssf.record.formula.eval.ValueEval;
/**
- * Tests for the INDEX() function
- *
+ * Tests for the INDEX() function.</p>
+ *
+ * This class contains just a few specific cases that directly invoke {@link Index},
+ * with minimum overhead.<br/>
+ * Another test: {@link TestIndexFunctionFromSpreadsheet} operates from a higher level
+ * and has far greater coverage of input permutations.<br/>
+ *
* @author Josh Micich
*/
public final class TestIndex extends TestCase {
+ private static final Index FUNC_INST = new Index();
private static final double[] TEST_VALUES0 = {
1, 2,
3, 4,
@@ -39,44 +47,69 @@
9, 10,
11, 12,
};
-
+
/**
* For the case when the first argument to INDEX() is an area reference
*/
public void testEvaluateAreaReference() {
-
+
double[] values = TEST_VALUES0;
confirmAreaEval("C1:D6", values, 4, 1, 7);
confirmAreaEval("C1:D6", values, 6, 2, 12);
confirmAreaEval("C1:D6", values, 3, 1, 5);
-
+
// now treat same data as 3 columns, 4 rows
- confirmAreaEval("C10:E13", values, 2, 2, 5);
+ confirmAreaEval("C10:E13", values, 2, 2, 5);
confirmAreaEval("C10:E13", values, 4, 1, 10);
}
-
+
/**
* @param areaRefString in Excel notation e.g. 'D2:E97'
* @param dValues array of evaluated values for the area reference
* @param rowNum 1-based
* @param colNum 1-based, pass -1 to signify argument not present
*/
- private static void confirmAreaEval(String areaRefString, double[] dValues,
+ private static void confirmAreaEval(String areaRefString, double[] dValues,
int rowNum, int colNum, double expectedResult) {
ValueEval[] values = new ValueEval[dValues.length];
for (int i = 0; i < values.length; i++) {
values[i] = new NumberEval(dValues[i]);
}
AreaEval arg0 = EvalFactory.createAreaEval(areaRefString, values);
-
+
Eval[] args;
if (colNum > 0) {
args = new Eval[] { arg0, new NumberEval(rowNum), new NumberEval(colNum), };
} else {
args = new Eval[] { arg0, new NumberEval(rowNum), };
}
-
- double actual = NumericFunctionInvoker.invoke(new Index(), args);
+
+ double actual = NumericFunctionInvoker.invoke(FUNC_INST, args);
assertEquals(expectedResult, actual, 0D);
}
+
+ /**
+ * Tests expressions like "INDEX(A1:C1,,2)".<br/>
+ * This problem was found while fixing bug 47048 and is observable up to svn r773441.
+ */
+ public void testMissingArg() {
+ ValueEval[] values = {
+ new NumberEval(25.0),
+ new NumberEval(26.0),
+ new NumberEval(28.0),
+ };
+ AreaEval arg0 = EvalFactory.createAreaEval("A10:C10", values);
+ Eval[] args = new Eval[] { arg0, MissingArgEval.instance, new NumberEval(2), };
+ Eval actualResult;
+ try {
+ actualResult = FUNC_INST.evaluate(args, 1, (short)1);
+ } catch (RuntimeException e) {
+ if (e.getMessage().equals("Unexpected arg eval type (org.apache.poi.hssf.record.formula.eval.MissingArgEval")) {
+ throw new AssertionFailedError("Identified bug 47048b - INDEX() should support missing-arg");
+ }
+ throw e;
+ }
+ assertEquals(NumberEval.class, actualResult.getClass());
+ assertEquals(26.0, ((NumberEval)actualResult).getNumberValue(), 0.0);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org