made ArrayPtg immutable

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

src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java

@@ -43,22 +43,23 @@ public final class ArrayPtg extends Ptg {
 	 */
 	public static final int PLAIN_TOKEN_SIZE = 1+RESERVED_FIELD_LEN;
 
-	private static final byte[] DEFAULT_RESERVED_DATA = new byte[RESERVED_FIELD_LEN];
+	// 7 bytes of data (stored as an int, short and byte here)
-
+	private final int _reserved0Int;
-	// TODO - fix up field visibility and subclasses
+	private final int _reserved1Short;
-	private final byte[] field_1_reserved;
+	private final int _reserved2Byte;
 
 	// data from these fields comes after the Ptg data of all tokens in current formula
-	private int  token_1_columns;
+	private final int  _nColumns;
-	private short token_2_rows;
+	private final int _nRows;
-	private Object[] token_3_arrayValues;
+	private final Object[] _arrayValues;
 
-	public ArrayPtg(LittleEndianInput in) {
+	ArrayPtg(int reserved0, int reserved1, int reserved2, int nColumns, int nRows, Object[] arrayValues) {
-		field_1_reserved = new byte[RESERVED_FIELD_LEN];
+		_reserved0Int = reserved0;
-		// TODO - add readFully method to RecordInputStream
+		_reserved1Short = reserved1;
-		for(int i=0; i< RESERVED_FIELD_LEN; i++) {
+		_reserved2Byte = reserved2;
-			field_1_reserved[i] = in.readByte();
+		_nColumns = nColumns;
-		}
+		_nRows = nRows;
+		_arrayValues = arrayValues;
 	}
 	/**
 	 * @param values2d array values arranged in rows
@@ -67,10 +68,10 @@ public final class ArrayPtg extends Ptg {
 		int nColumns = values2d[0].length;
 		int nRows = values2d.length;
 		// convert 2-d to 1-d array (row by row according to getValueIndex())
-		token_1_columns = (short) nColumns;
+		_nColumns = (short) nColumns;
-		token_2_rows = (short) nRows;
+		_nRows = (short) nRows;
 
-		Object[] vv = new Object[token_1_columns * token_2_rows];
+		Object[] vv = new Object[_nColumns * _nRows];
 		for (int r=0; r<nRows; r++) {
 			Object[] rowData = values2d[r];
 			for (int c=0; c<nColumns; c++) {
@@ -78,21 +79,23 @@ public final class ArrayPtg extends Ptg {
 			}
 		}
 
-		token_3_arrayValues = vv;
+		_arrayValues = vv;
-		field_1_reserved = DEFAULT_RESERVED_DATA;
+		_reserved0Int = 0;
+		_reserved1Short = 0;
+		_reserved2Byte = 0;
 	}
 	/**
 	 * @return 2-d array (inner index is rowIx, outer index is colIx)
 	 */
 	public Object[][] getTokenArrayValues() {
-		if (token_3_arrayValues == null) {
+		if (_arrayValues == null) {
 			throw new IllegalStateException("array values not read yet");
 		}
-		Object[][] result = new Object[token_2_rows][token_1_columns];
+		Object[][] result = new Object[_nRows][_nColumns];
-		for (int r = 0; r < token_2_rows; r++) {
+		for (int r = 0; r < _nRows; r++) {
 			Object[] rowData = result[r];
-			for (int c = 0; c < token_1_columns; c++) {
+			for (int c = 0; c < _nColumns; c++) {
-				rowData[c] = token_3_arrayValues[getValueIndex(c, r)];
+				rowData[c] = _arrayValues[getValueIndex(c, r)];
 			}
 		}
 		return result;
@@ -102,33 +105,12 @@ public final class ArrayPtg extends Ptg {
 		return false;
 	}
 
-	/**
-	 * Read in the actual token (array) values. This occurs
-	 * AFTER the last Ptg in the expression.
-	 * See page 304-305 of Excel97-2007BinaryFileFormat(xls)Specification.pdf
-	 */
-	public void readTokenValues(LittleEndianInput in) {
-		int nColumns = in.readUByte();
-		short nRows = in.readShort();
-		//The token_1_columns and token_2_rows do not follow the documentation.
-		//The number of physical rows and columns is actually +1 of these values.
-		//Which is not explicitly documented.
-		nColumns++;
-		nRows++;
-
-		token_1_columns = nColumns;
-		token_2_rows = nRows;
-
-		int totalCount = nRows * nColumns;
-		token_3_arrayValues = ConstantValueParser.parse(in, totalCount);
-	}
-
 	public String toString() {
 		StringBuffer sb = new StringBuffer("[ArrayPtg]\n");
 
 		sb.append("nRows = ").append(getRowCount()).append("\n");
 		sb.append("nCols = ").append(getColumnCount()).append("\n");
-		if (token_3_arrayValues == null) {
+		if (_arrayValues == null) {
 			sb.append("  #values#uninitialised#\n");
 		} else {
 			sb.append("  ").append(toFormulaString());
@@ -141,36 +123,38 @@ public final class ArrayPtg extends Ptg {
 	 * @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) {
+		if(colIx < 0 || colIx >= _nColumns) {
 			throw new IllegalArgumentException("Specified colIx (" + colIx
-					+ ") is outside the allowed range (0.." + (token_1_columns-1) + ")");
+					+ ") is outside the allowed range (0.." + (_nColumns-1) + ")");
 		}
-		if(rowIx < 0 || rowIx >= token_2_rows) {
+		if(rowIx < 0 || rowIx >= _nRows) {
 			throw new IllegalArgumentException("Specified rowIx (" + rowIx
-					+ ") is outside the allowed range (0.." + (token_2_rows-1) + ")");
+					+ ") is outside the allowed range (0.." + (_nRows-1) + ")");
 		}
-		return rowIx * token_1_columns + colIx;
+		return rowIx * _nColumns + colIx;
 	}
 
 	public void write(LittleEndianOutput out) {
 		out.writeByte(sid + getPtgClass());
-		out.write(field_1_reserved);
+		out.writeInt(_reserved0Int);
+		out.writeShort(_reserved1Short);
+		out.writeByte(_reserved2Byte);
 	}
 
 	public int writeTokenValueBytes(LittleEndianOutput out) {
 
-		out.writeByte(token_1_columns-1);
+		out.writeByte(_nColumns-1);
-		out.writeShort(token_2_rows-1);
+		out.writeShort(_nRows-1);
-		ConstantValueParser.encode(out, token_3_arrayValues);
+		ConstantValueParser.encode(out, _arrayValues);
-		return 3 + ConstantValueParser.getEncodedSize(token_3_arrayValues);
+		return 3 + ConstantValueParser.getEncodedSize(_arrayValues);
 	}
 
-	public short getRowCount() {
+	public int getRowCount() {
-		return token_2_rows;
+		return _nRows;
 	}
 
-	public short getColumnCount() {
+	public int getColumnCount() {
-		return (short)token_1_columns;
+		return _nColumns;
 	}
 
 	/** This size includes the size of the array Ptg plus the Array Ptg Token value size*/
@@ -178,7 +162,7 @@ public final class ArrayPtg extends Ptg {
 		return PLAIN_TOKEN_SIZE
 			// data written after the all tokens:
 			+ 1 + 2 // column, row
-			+ ConstantValueParser.getEncodedSize(token_3_arrayValues);
+			+ ConstantValueParser.getEncodedSize(_arrayValues);
 	}
 
 	public String toFormulaString() {
@@ -192,7 +176,7 @@ public final class ArrayPtg extends Ptg {
 			  	if (x > 0) {
 					b.append(",");
 				}
-		  		Object o = token_3_arrayValues[getValueIndex(x, y)];
+		  		Object o = _arrayValues[getValueIndex(x, y)];
 		  		b.append(getConstantText(o));
 		  	}
 		  }
@@ -223,4 +207,61 @@ public final class ArrayPtg extends Ptg {
 	public byte getDefaultOperandClass() {
 		return Ptg.CLASS_ARRAY;
 	}
+
+	/**
+	 * Represents the initial plain tArray token (without the constant data that trails the whole
+	 * formula).  Objects of this class are only temporary and cannot be used as {@link Ptg}s.
+	 * These temporary objects get converted to {@link ArrayPtg} by the
+	 * {@link #finishReading(LittleEndianInput)} method.
+	 */
+	static final class Initial extends Ptg {
+		private final int _reserved0;
+		private final int _reserved1;
+		private final int _reserved2;
+
+		public Initial(LittleEndianInput in) {
+			_reserved0 = in.readInt();
+			_reserved1 = in.readUShort();
+			_reserved2 = in.readUByte();
+		}
+		private static RuntimeException invalid() {
+			throw new IllegalStateException("This object is a partially initialised tArray, and cannot be used as a Ptg");
+		}
+		public byte getDefaultOperandClass() {
+			throw invalid();
+		}
+		public int getSize() {
+			return PLAIN_TOKEN_SIZE;
+		}
+		public boolean isBaseToken() {
+			return false;
+		}
+		public String toFormulaString() {
+			throw invalid();
+		}
+		public void write(LittleEndianOutput out) {
+			throw invalid();
+		}
+		/**
+		 * Read in the actual token (array) values. This occurs
+		 * AFTER the last Ptg in the expression.
+		 * See page 304-305 of Excel97-2007BinaryFileFormat(xls)Specification.pdf
+		 */
+		public ArrayPtg finishReading(LittleEndianInput in) {
+			int nColumns = in.readUByte();
+			short nRows = in.readShort();
+			//The token_1_columns and token_2_rows do not follow the documentation.
+			//The number of physical rows and columns is actually +1 of these values.
+			//Which is not explicitly documented.
+			nColumns++;
+			nRows++;
+
+			int totalCount = nRows * nColumns;
+			Object[] arrayValues = ConstantValueParser.parse(in, totalCount);
+
+			ArrayPtg result = new ArrayPtg(_reserved0, _reserved1, _reserved2, nColumns, nRows, arrayValues);
+			result.setClass(getPtgClass());
+			return result;
+		}
+	}
 }

src/java/org/apache/poi/hssf/record/formula/Ptg.java

@@ -51,29 +51,27 @@ public abstract class Ptg implements Cloneable {
 	public static Ptg[] readTokens(int size, LittleEndianInput in) {
 		List<Ptg> temp = new ArrayList<Ptg>(4 + size / 2);
 		int pos = 0;
-		List<Ptg> arrayPtgs = null;
+		boolean hasArrayPtgs = false;
 		while (pos < size) {
 			Ptg ptg = Ptg.createPtg(in);
-			if (ptg instanceof ArrayPtg) {
+			if (ptg instanceof ArrayPtg.Initial) {
-				if (arrayPtgs == null) {
+				hasArrayPtgs = true;
-					arrayPtgs = new ArrayList<Ptg>(5);
 			}
-				arrayPtgs.add(ptg);
-				pos += ArrayPtg.PLAIN_TOKEN_SIZE;
-			} else {
 			pos += ptg.getSize();
-			}
 			temp.add(ptg);
 		}
 		if(pos != size) {
 			throw new RuntimeException("Ptg array size mismatch");
 		}
-		if (arrayPtgs != null) {
+		if (hasArrayPtgs) {
-			for (int i=0;i<arrayPtgs.size();i++) {
+			Ptg[] result = toPtgArray(temp);
-				ArrayPtg p = (ArrayPtg)arrayPtgs.get(i);
+			for (int i=0;i<result.length;i++) {
-				p.readTokenValues(in);
+				if (result[i] instanceof ArrayPtg.Initial) {
+					result[i] = ((ArrayPtg.Initial) result[i]).finishReading(in);
 				}
 			}
+			return result;
+		}
 		return toPtgArray(temp);
 	}
 
@@ -101,7 +99,7 @@ public abstract class Ptg implements Cloneable {
 		int baseId = id & 0x1F | 0x20;
 
 		switch (baseId) {
-			case ArrayPtg.sid:    return new ArrayPtg(in);    // 0x20, 0x40, 0x60
+			case ArrayPtg.sid:    return new ArrayPtg.Initial(in);//0x20, 0x40, 0x60
 			case FuncPtg.sid:     return FuncPtg.create(in);  // 0x21, 0x41, 0x61
 			case FuncVarPtg.sid:  return FuncVarPtg.create(in);//0x22, 0x42, 0x62
 			case NamePtg.sid:     return new NamePtg(in);     // 0x23, 0x43, 0x63

src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java

@@ -49,14 +49,16 @@ public final class TestArrayPtg extends TestCase {
 		2, 2, 0, 0, 70, 71, // "FG"
 	};
 
+	private static ArrayPtg create(byte[] initialData, byte[] constantData) {
+		ArrayPtg.Initial ptgInit = new ArrayPtg.Initial(TestcaseRecordInputStream.createLittleEndian(initialData));
+		return ptgInit.finishReading(TestcaseRecordInputStream.createLittleEndian(constantData));
+	}
+
 	/**
-	 * Lots of problems with ArrayPtg's encoding of
+	 * Lots of problems with ArrayPtg's decoding and encoding of the element value data
 	 */
 	public void testReadWriteTokenValueBytes() {
-
+		ArrayPtg ptg = create(ENCODED_PTG_DATA, ENCODED_CONSTANT_DATA);
-		ArrayPtg ptg = new ArrayPtg(TestcaseRecordInputStream.createLittleEndian(ENCODED_PTG_DATA));
-
-		ptg.readTokenValues(TestcaseRecordInputStream.createLittleEndian(ENCODED_CONSTANT_DATA));
 		assertEquals(3, ptg.getColumnCount());
 		assertEquals(2, ptg.getRowCount());
 		Object[][] values = ptg.getTokenArrayValues();
@@ -78,12 +80,12 @@ public final class TestArrayPtg extends TestCase {
 		assertTrue(Arrays.equals(ENCODED_CONSTANT_DATA, outBuf));
 	}
 
+
 	/**
 	 * Excel stores array elements column by column.  This test makes sure POI does the same.
 	 */
 	public void testElementOrdering() {
-		ArrayPtg ptg = new ArrayPtg(TestcaseRecordInputStream.createLittleEndian(ENCODED_PTG_DATA));
+		ArrayPtg ptg = create(ENCODED_PTG_DATA, ENCODED_CONSTANT_DATA);
-		ptg.readTokenValues(TestcaseRecordInputStream.createLittleEndian(ENCODED_CONSTANT_DATA));
 		assertEquals(3, ptg.getColumnCount());
 		assertEquals(2, ptg.getRowCount());
 
@@ -113,10 +115,7 @@ public final class TestArrayPtg extends TestCase {
 	}
 
 	public void testToFormulaString() {
-		ArrayPtg ptg = new ArrayPtg(TestcaseRecordInputStream.createLittleEndian(ENCODED_PTG_DATA));
+		ArrayPtg ptg = create(ENCODED_PTG_DATA, ENCODED_CONSTANT_DATA);
-
-		ptg.readTokenValues(TestcaseRecordInputStream.createLittleEndian(ENCODED_CONSTANT_DATA));
-
 		String actualFormula;
 		try {
 			actualFormula = ptg.toFormulaString();
@@ -139,9 +138,7 @@ public final class TestArrayPtg extends TestCase {
 	}
 
 	private static void confirmOperandClassDecoding(byte operandClass) {
-		byte[] fullData = new byte[ENCODED_PTG_DATA.length + ENCODED_CONSTANT_DATA.length];
+		byte[] fullData = concat(ENCODED_PTG_DATA, ENCODED_CONSTANT_DATA);
-		System.arraycopy(ENCODED_PTG_DATA, 0, fullData, 0, ENCODED_PTG_DATA.length);
-		System.arraycopy(ENCODED_CONSTANT_DATA, 0, fullData, ENCODED_PTG_DATA.length, ENCODED_CONSTANT_DATA.length);
 
 		// Force encoded operand class for tArray
 		fullData[0] = (byte) (ArrayPtg.sid + operandClass);
@@ -153,4 +150,11 @@ public final class TestArrayPtg extends TestCase {
 		ArrayPtg aPtg = (ArrayPtg) ptgs[0];
 		assertEquals(operandClass, aPtg.getPtgClass());
 	}
+
+	private static byte[] concat(byte[] a, byte[] b) {
+		byte[] result = new byte[a.length + b.length];
+		System.arraycopy(a, 0, result, 0, a.length);
+		System.arraycopy(b, 0, result, a.length, b.length);
+		return result;
+	}
 }