Follow-on fix for bug 42564 (r653668). Array elements are stored internally column...

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

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 251009b2870af8d6d1b69bcdf7d47457daa1d769..49c66e07200a564177f5ff9b2103254e1f81b460 100644 (file)
--- 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<getColumnCount();x++) {
                        for (int y=0;y<getRowCount();y++) {
                                Object o = token_3_arrayValues[getValueIndex(x, y)];
-                               buffer.append("[").append(x).append("][").append(y).append("] = ").append(o).append("\n"); 
+                               buffer.append("[").append(x).append("][").append(y).append("] = ").append(o).append("\n"); 
                        }
                }
                return buffer.toString();
        }
 
+       /**
+        * Note - (2D) array elements are stored column by column 
+        * @return the index into the internal 1D array for the specified column and row
+        */
        /* package */ int getValueIndex(int colIx, int rowIx) {
                if(colIx < 0 || colIx >= 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<getColumnCount();x++) {
-                       if (x > 0) {
+               for (int x = 0; x < getColumnCount(); x++) {
+                       if (x > 0) {
                                b.append(";");
                        }
-                       for (int y=0;y<getRowCount();y++) {
+                       for (int y = 0; y < getRowCount(); y++) {
                                if (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 (file)
index 0000000..3c49fc2
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 16f80bb791a0c7f242be57a80455a123e5687368..39464b5e03ab468e0b4ebff6871fc1a9be96b0b8 100644 (file)
--- 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);
+       }
 }