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 2008/05/26 20:02:26 UTC

svn commit: r660256 - in /poi/trunk/src: java/org/apache/poi/hssf/record/formula/ArrayPtg.java testcases/org/apache/poi/hssf/data/ex42564-elementOrder.xls testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java

Author: josh
Date: Mon May 26 11:02:23 2008
New Revision: 660256

URL: http://svn.apache.org/viewvc?rev=660256&view=rev
Log:
Follow-on fix for bug 42564 (r653668). Array elements are stored internally column by column.

Added:
    poi/trunk/src/testcases/org/apache/poi/hssf/data/ex42564-elementOrder.xls   (with props)
Modified:
    poi/trunk/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java
    poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java

Modified: poi/trunk/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java?rev=660256&r1=660255&r2=660256&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java Mon May 26 11:02:23 2008
@@ -47,7 +47,7 @@
 	protected Object[] token_3_arrayValues;
 
 	protected ArrayPtg() {
-	  //Required for clone methods
+		//Required for clone methods
 	}
 
 	public ArrayPtg(RecordInputStream in)
@@ -89,12 +89,16 @@
 		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 @@
 			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 @@
 		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 @@
 			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 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;
 	}
 }

Added: poi/trunk/src/testcases/org/apache/poi/hssf/data/ex42564-elementOrder.xls
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/data/ex42564-elementOrder.xls?rev=660256&view=auto
==============================================================================
Binary file - no diff available.

Propchange: poi/trunk/src/testcases/org/apache/poi/hssf/data/ex42564-elementOrder.xls
------------------------------------------------------------------------------
    svn:mime-type = application/octet-stream

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java?rev=660256&r1=660255&r2=660256&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java Mon May 26 11:02:23 2008
@@ -19,8 +19,10 @@
 
 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 @@
 	}
 
 	/**
-	 * 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 @@
 		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);
+	}
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org