diff --git a/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java b/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java index 251009b28..49c66e072 100644 --- a/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java +++ b/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java @@ -47,7 +47,7 @@ public class ArrayPtg extends Ptg { protected Object[] token_3_arrayValues; protected ArrayPtg() { - //Required for clone methods + //Required for clone methods } public ArrayPtg(RecordInputStream in) @@ -89,12 +89,16 @@ public class ArrayPtg extends Ptg { for (int x=0;x= token_1_columns) { throw new IllegalArgumentException("Specified colIx (" + colIx @@ -104,7 +108,7 @@ public class ArrayPtg extends Ptg { throw new IllegalArgumentException("Specified rowIx (" + rowIx + ") is outside the allowed range (0.." + (token_2_rows-1) + ")"); } - return rowIx * token_1_columns + colIx; + return rowIx + token_2_rows * colIx; } public void writeBytes(byte[] data, int offset) { @@ -137,22 +141,21 @@ public class ArrayPtg extends Ptg { return size; } - public String toFormulaString(HSSFWorkbook book) - { + public String toFormulaString(HSSFWorkbook book) { StringBuffer b = new StringBuffer(); b.append("{"); - for (int x=0;x 0) { + for (int x = 0; x < getColumnCount(); x++) { + if (x > 0) { b.append(";"); } - for (int y=0;y 0) { b.append(","); } - Object o = token_3_arrayValues[getValueIndex(x, y)]; - b.append(getConstantText(o)); - } - } + Object o = token_3_arrayValues[getValueIndex(x, y)]; + b.append(getConstantText(o)); + } + } b.append("}"); return b.toString(); } @@ -166,6 +169,7 @@ public class ArrayPtg extends Ptg { return "\"" + ((UnicodeString)o).getString() + "\""; } if (o instanceof Double) { + // TODO - numeric array elements need default Excel number formatting return ((Double)o).toString(); } if (o instanceof Boolean) { @@ -182,13 +186,13 @@ public class ArrayPtg extends Ptg { } public Object clone() { - ArrayPtg ptg = new ArrayPtg(); - ptg.field_1_reserved = (byte[]) field_1_reserved.clone(); - - ptg.token_1_columns = token_1_columns; - ptg.token_2_rows = token_2_rows; - ptg.token_3_arrayValues = (Object[]) token_3_arrayValues.clone(); - ptg.setClass(ptgClass); - return ptg; + ArrayPtg ptg = new ArrayPtg(); + ptg.field_1_reserved = (byte[]) field_1_reserved.clone(); + + ptg.token_1_columns = token_1_columns; + ptg.token_2_rows = token_2_rows; + ptg.token_3_arrayValues = (Object[]) token_3_arrayValues.clone(); + ptg.setClass(ptgClass); + return ptg; } } diff --git a/src/testcases/org/apache/poi/hssf/data/ex42564-elementOrder.xls b/src/testcases/org/apache/poi/hssf/data/ex42564-elementOrder.xls new file mode 100644 index 000000000..3c49fc257 Binary files /dev/null and b/src/testcases/org/apache/poi/hssf/data/ex42564-elementOrder.xls differ diff --git a/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java b/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java index 16f80bb79..39464b5e0 100644 --- a/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java +++ b/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java @@ -19,8 +19,10 @@ package org.apache.poi.hssf.record.formula; import java.util.Arrays; +import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.record.TestcaseRecordInputStream; import org.apache.poi.hssf.record.UnicodeString; +import org.apache.poi.hssf.usermodel.HSSFWorkbook; import junit.framework.AssertionFailedError; import junit.framework.TestCase; @@ -77,7 +79,7 @@ public final class TestArrayPtg extends TestCase { } /** - * make sure constant elements are stored row by row + * Excel stores array elements column by column. This test makes sure POI does the same. */ public void testElementOrdering() { ArrayPtg ptg = new ArrayPtgV(new TestcaseRecordInputStream(ArrayPtgV.sid, ENCODED_PTG_DATA)); @@ -86,10 +88,27 @@ public final class TestArrayPtg extends TestCase { assertEquals(2, ptg.getRowCount()); assertEquals(0, ptg.getValueIndex(0, 0)); - assertEquals(1, ptg.getValueIndex(1, 0)); - assertEquals(2, ptg.getValueIndex(2, 0)); - assertEquals(3, ptg.getValueIndex(0, 1)); - assertEquals(4, ptg.getValueIndex(1, 1)); + assertEquals(2, ptg.getValueIndex(1, 0)); + assertEquals(4, ptg.getValueIndex(2, 0)); + assertEquals(1, ptg.getValueIndex(0, 1)); + assertEquals(3, ptg.getValueIndex(1, 1)); assertEquals(5, ptg.getValueIndex(2, 1)); } + + /** + * Test for a bug which was temporarily introduced by the fix for bug 42564. + * A spreadsheet was added to make the ordering clearer. + */ + public void testElementOrderingInSpreadsheet() { + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex42564-elementOrder.xls"); + + // The formula has an array with 3 rows and 5 column + String formula = wb.getSheetAt(0).getRow(0).getCell((short)0).getCellFormula(); + // TODO - These number literals should not have '.0'. Excel has different number rendering rules + + if (formula.equals("SUM({1.0,6.0,11.0;2.0,7.0,12.0;3.0,8.0,13.0;4.0,9.0,14.0;5.0,10.0,15.0})")) { + throw new AssertionFailedError("Identified bug 42564 b"); + } + assertEquals("SUM({1.0,2.0,3.0;4.0,5.0,6.0;7.0,8.0,9.0;10.0,11.0,12.0;13.0,14.0,15.0})", formula); + } }