Consolidated TableRecord inside FormulaRecordAggregate. Simplifications to FormulaRecord

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

13 files changed, 370 insertions, 508 deletions

diff --git a/src/java/org/apache/poi/hssf/model/RecordStream.java b/src/java/org/apache/poi/hssf/model/RecordStream.java
index 1a06873954..8869a9cf03 100755
--- a/src/java/org/apache/poi/hssf/model/RecordStream.java
+++ b/src/java/org/apache/poi/hssf/model/RecordStream.java
@@ -30,19 +30,28 @@ public final class RecordStream {
 	private final List _list;
 	private int _nextIndex;
 	private int _countRead;
+	private final int _endIx;
 
-	public RecordStream(List inputList, int startIndex) {
+	/**
+	 * Creates a RecordStream bounded by startIndex and endIndex
+	 */
+	public RecordStream(List inputList, int startIndex, int endIx) {
 		_list = inputList;
 		_nextIndex = startIndex;
+		_endIx = endIx;
 		_countRead = 0;
 	}
 
+	public RecordStream(List records, int startIx) {
+		this(records, startIx, records.size());
+	}
+
 	public boolean hasNext() {
-		return _nextIndex < _list.size();
+		return _nextIndex < _endIx;
 	}
 
 	public Record getNext() {
-		if(_nextIndex >= _list.size()) {
+		if(!hasNext()) {
 			throw new RuntimeException("Attempt to read past end of record stream");
 		}
 		_countRead ++;
@@ -53,14 +62,17 @@ public final class RecordStream {
 	 * @return the {@link Class} of the next Record. <code>null</code> if this stream is exhausted.
 	 */
 	public Class peekNextClass() {
-		if(_nextIndex >= _list.size()) {
+		if(!hasNext()) {
 			return null;
 		}
 		return _list.get(_nextIndex).getClass();
 	}
 
+	/**
+	 * @return -1 if at end of records
+	 */
 	public int peekNextSid() {
-		if(_nextIndex >= _list.size()) {
+		if(!hasNext()) {
 			return -1;
 		}
 		return ((Record)_list.get(_nextIndex)).getSid();
diff --git a/src/java/org/apache/poi/hssf/record/FormulaRecord.java b/src/java/org/apache/poi/hssf/record/FormulaRecord.java
index 9f6bb45866..1074c3c21e 100644
--- a/src/java/org/apache/poi/hssf/record/FormulaRecord.java
+++ b/src/java/org/apache/poi/hssf/record/FormulaRecord.java
@@ -17,12 +17,13 @@
 
 package org.apache.poi.hssf.record;
 
+import java.util.Arrays;
 import java.util.List;
-import java.util.Stack;
 
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
+import org.apache.poi.util.HexDump;
 import org.apache.poi.util.LittleEndian;
 
 /**
@@ -33,69 +34,60 @@ import org.apache.poi.util.LittleEndian;
  * @version 2.0-pre
  */
 public final class FormulaRecord extends Record implements CellValueRecordInterface {
-    
+
     public static final short sid = 0x0006;   // docs say 406...because of a bug Microsoft support site article #Q184647)
+    private static int FIXED_SIZE = 22;
 
     private static final BitField alwaysCalc = BitFieldFactory.getInstance(0x0001);
     private static final BitField calcOnLoad = BitFieldFactory.getInstance(0x0002);
-    private static final BitField sharedFormula = BitFieldFactory.getInstance(0x0008);    
+    private static final BitField sharedFormula = BitFieldFactory.getInstance(0x0008);
 
-    private int             field_1_row;
+    private int               field_1_row;
     private short             field_2_column;
     private short             field_3_xf;
     private double            field_4_value;
     private short             field_5_options;
     private int               field_6_zero;
-    private short             field_7_expression_len;
-    private Stack             field_8_parsed_expr;
-    
+    private Ptg[]             field_8_parsed_expr;
+
     /**
      * Since the NaN support seems sketchy (different constants) we'll store and spit it out directly
      */
-    private byte[]			value_data;
-    private byte[]            all_data; //if formula support is not enabled then
-                                        //we'll just store/reserialize
+    private byte[]            value_data;
 
     /** Creates new FormulaRecord */
 
-    public FormulaRecord()
-    {
-        field_8_parsed_expr = new Stack();
+    public FormulaRecord() {
+        field_8_parsed_expr = Ptg.EMPTY_PTG_ARRAY;
     }
 
     /**
      * Constructs a Formula record and sets its fields appropriately.
-     * Note - id must be 0x06 (NOT 0x406 see MSKB #Q184647 for an 
+     * Note - id must be 0x06 (NOT 0x406 see MSKB #Q184647 for an
      * "explanation of this bug in the documentation) or an exception
      *  will be throw upon validation
      *
      * @param in the RecordInputstream to read the record from
      */
 
-    public FormulaRecord(RecordInputStream in)
-    {
+    public FormulaRecord(RecordInputStream in) {
         super(in);
     }
 
-    protected void fillFields(RecordInputStream in)
-    {
-        try {
+    protected void fillFields(RecordInputStream in) {
           field_1_row            = in.readUShort();
           field_2_column         = in.readShort();
           field_3_xf             = in.readShort();
           field_4_value          = in.readDouble();
           field_5_options        = in.readShort();
-		        
+
         if (Double.isNaN(field_4_value)) {
             value_data = in.getNANData();
         }
-        
+
           field_6_zero           = in.readInt();
-          field_7_expression_len = in.readShort();
-          field_8_parsed_expr    = Ptg.createParsedExpressionTokens(field_7_expression_len, in);
-        } catch (java.lang.UnsupportedOperationException uoe)  {
-          throw new RecordFormatException(uoe);
-        }
+          int field_7_expression_len = in.readShort(); // this length does not include any extra array data
+          field_8_parsed_expr = Ptg.readTokens(field_7_expression_len, in);
         if (in.remaining() == 10) {
             // TODO - this seems to occur when IntersectionPtg is present
             // 10 extra bytes are just 0x01 and 0x00
@@ -103,19 +95,15 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
         }
     }
 
-    //public void setRow(short row)
-    public void setRow(int row)
-    {
+    public void setRow(int row) {
         field_1_row = row;
     }
 
-    public void setColumn(short column)
-    {
+    public void setColumn(short column) {
         field_2_column = column;
     }
 
-    public void setXFIndex(short xf)
-    {
+    public void setXFIndex(short xf) {
         field_3_xf = xf;
     }
 
@@ -124,9 +112,7 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
      *
      * @param value  calculated value
      */
-
-    public void setValue(double value)
-    {
+    public void setValue(double value) {
         field_4_value = value;
     }
 
@@ -135,35 +121,19 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
      *
      * @param options  bitmask
      */
-
-    public void setOptions(short options)
-    {
+    public void setOptions(short options) {
         field_5_options = options;
     }
 
-    /**
-     * set the length (in number of tokens) of the expression
-     * @param len  length
-     */
-
-    public void setExpressionLength(short len)
-    {
-        field_7_expression_len = len;
-    }
-
-    //public short getRow()
-    public int getRow()
-    {
+    public int getRow() {
         return field_1_row;
     }
 
-    public short getColumn()
-    {
+    public short getColumn() {
         return field_2_column;
     }
 
-    public short getXFIndex()
-    {
+    public short getXFIndex() {
         return field_3_xf;
     }
 
@@ -172,8 +142,7 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
      *
      * @return calculated value
      */
-    public double getValue()
-    {
+    public double getValue() {
         return field_4_value;
     }
 
@@ -182,108 +151,51 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
      *
      * @return bitmask
      */
-    public short getOptions()
-    {
+    public short getOptions() {
         return field_5_options;
-    }    
-    
+    }
+
     public boolean isSharedFormula() {
         return sharedFormula.isSet(field_5_options);
     }
     public void setSharedFormula(boolean flag) {
-    	field_5_options =
-    		sharedFormula.setShortBoolean(field_5_options, flag);
+        field_5_options =
+            sharedFormula.setShortBoolean(field_5_options, flag);
     }
-    
+
     public boolean isAlwaysCalc() {
-    	return alwaysCalc.isSet(field_5_options);
+        return alwaysCalc.isSet(field_5_options);
     }
     public void setAlwaysCalc(boolean flag) {
-    	field_5_options =
-    		alwaysCalc.setShortBoolean(field_5_options, flag);
+        field_5_options =
+            alwaysCalc.setShortBoolean(field_5_options, flag);
     }
-    
+
     public boolean isCalcOnLoad() {
-    	return calcOnLoad.isSet(field_5_options);
+        return calcOnLoad.isSet(field_5_options);
     }
     public void setCalcOnLoad(boolean flag) {
-    	field_5_options =
-    		calcOnLoad.setShortBoolean(field_5_options, flag);
-    }
-    
-    /**
-     * get the length (in number of tokens) of the expression
-     * @return  expression length
-     */
-
-    public short getExpressionLength()
-    {
-        return field_7_expression_len;
-    }
-
-    /**
-     * push a token onto the stack
-     *
-     * @param ptg  the token
-     */
-
-    public void pushExpressionToken(Ptg ptg)
-    {
-        field_8_parsed_expr.push(ptg);
-    }
-
-    /**
-     * pop a token off of the stack
-     *
-     * @return Ptg - the token
-     */
-
-    public Ptg popExpressionToken()
-    {
-        return ( Ptg ) field_8_parsed_expr.pop();
-    }
-
-    /**
-     * peek at the token on the top of stack
-     *
-     * @return Ptg - the token
-     */
-
-    public Ptg peekExpressionToken()
-    {
-        return ( Ptg ) field_8_parsed_expr.peek();
+        field_5_options =
+            calcOnLoad.setShortBoolean(field_5_options, flag);
     }
 
     /**
      * get the size of the stack
      * @return size of the stack
      */
-
-    public int getNumberOfExpressionTokens()
-    {
-        if (this.field_8_parsed_expr == null) {
-            return 0;
-        } else {
-            return field_8_parsed_expr.size();
-        }
+    public int getNumberOfExpressionTokens() {
+        return field_8_parsed_expr.length;
     }
 
     /**
-     * get the stack as a list
-     *
-     * @return list of tokens (casts stack to a list and returns it!)
-     * this method can return null is we are unable to create Ptgs from 
-     *     existing excel file
-     * callers should check for null!
+     * @return list of formula tokens. never <code>null</code>
      */
-
-    public List getParsedExpression()
-    {
-        return field_8_parsed_expr;
+    public List getParsedExpression() {
+        return Arrays.asList(field_8_parsed_expr); // TODO - return array
     }
-    
-    public void setParsedExpression(Stack ptgs) {
-      field_8_parsed_expr = ptgs;
+
+    public void setParsedExpression(Ptg[] ptgs) {
+        field_8_parsed_expr = ptgs;
     }
 
     /**
@@ -292,156 +204,86 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
      *
      * @param id alleged id for this record
      */
-
-    protected void validateSid(short id)
-    {
-        if (id != sid)
-        {
+    protected void validateSid(short id) {
+        if (id != sid) {
             throw new RecordFormatException("NOT A FORMULA RECORD");
         }
     }
 
-    public short getSid()
-    {
+    public short getSid() {
         return sid;
     }
 
-    /**
-     * called by the class that is responsible for writing this sucker.
-     * Subclasses should implement this so that their data is passed back in a
-     * byte array.
-     *
-     * @return byte array containing instance data
-     */
+    private int getDataSize() {
+        return FIXED_SIZE + Ptg.getEncodedSize(field_8_parsed_expr);
+    }
+    public int serialize(int offset, byte [] data) {
 
-    public int serialize(int offset, byte [] data)
-    {
-        if (this.field_8_parsed_expr != null) {
-        int ptgSize = getTotalPtgSize();
+        int dataSize = getDataSize();
 
         LittleEndian.putShort(data, 0 + offset, sid);
-        LittleEndian.putShort(data, 2 + offset, ( short ) (22 + ptgSize));
-        //LittleEndian.putShort(data, 4 + offset, getRow());
-        LittleEndian.putShort(data, 4 + offset, ( short ) getRow());
+        LittleEndian.putUShort(data, 2 + offset, dataSize);
+        LittleEndian.putUShort(data, 4 + offset, getRow());
         LittleEndian.putShort(data, 6 + offset, getColumn());
         LittleEndian.putShort(data, 8 + offset, getXFIndex());
-        
+
         //only reserialize if the value is still NaN and we have old nan data
-        if (Double.isNaN(this.getValue()) && value_data != null) {        	
-			System.arraycopy(value_data,0,data,10 + offset,value_data.length);
+        if (Double.isNaN(getValue()) && value_data != null) {
+            System.arraycopy(value_data,0,data,10 + offset,value_data.length);
         } else {
-			LittleEndian.putDouble(data, 10 + offset, field_4_value);
+            LittleEndian.putDouble(data, 10 + offset, field_4_value);
         }
-        	
+
         LittleEndian.putShort(data, 18 + offset, getOptions());
-        
+
         //when writing the chn field (offset 20), it's supposed to be 0 but ignored on read
         //Microsoft Excel Developer's Kit Page 318
         LittleEndian.putInt(data, 20 + offset, 0);
-        LittleEndian.putShort(data, 24 + offset, getExpressionLength());
-        Ptg.serializePtgStack(field_8_parsed_expr, data, 26+offset);
-        } else {
-            System.arraycopy(all_data,0,data,offset,all_data.length);
-        }
-        return getRecordSize();
+        int formulaTokensSize = Ptg.getEncodedSizeWithoutArrayData(field_8_parsed_expr);
+        LittleEndian.putUShort(data, 24 + offset, formulaTokensSize);
+        Ptg.serializePtgs(field_8_parsed_expr, data, 26+offset);
+        return 4 + dataSize;
     }
-    
-    
-    
-
-    public int getRecordSize()
-    {
-        int retval =0;
-        
-        if (this.field_8_parsed_expr != null) {
-            retval = getTotalPtgSize() + 26;
-        } else {
-            retval =all_data.length;
-        }
-        return retval;
 
-        // return getTotalPtgSize() + 28;
-    }
-
-    private int getTotalPtgSize()
-    {
-        List list   = getParsedExpression();
-        int  retval = 0;
-
-        for (int k = 0; k < list.size(); k++)
-        {
-            Ptg ptg = ( Ptg ) list.get(k);
-
-            retval += ptg.getSize();
-        }
-        return retval;
+    public int getRecordSize() {
+        return 4 + getDataSize();
     }
 
-    public boolean isInValueSection()
-    {
+    public boolean isInValueSection() {
         return true;
     }
 
-    public boolean isValue()
-    {
+    public boolean isValue() {
         return true;
     }
-    
-    public String toString()
-    {
-        StringBuffer buffer = new StringBuffer();
-            buffer.append("[FORMULA]\n");
-            buffer.append("    .row       = ")
-                .append(Integer.toHexString(getRow())).append("\n");
-            buffer.append("    .column    = ")
-                .append(Integer.toHexString(getColumn()))
-                .append("\n");
-            buffer.append("    .xf              = ")
-                .append(Integer.toHexString(getXFIndex())).append("\n");
-            if (Double.isNaN(this.getValue()) && value_data != null)
-              buffer.append("    .value (NaN)     = ")
-                  .append(org.apache.poi.util.HexDump.dump(value_data,0,0))
-                  .append("\n");
-            else
-              buffer.append("    .value           = ").append(getValue())
-                  .append("\n");
-            buffer.append("    .options         = ").append(getOptions())
-                .append("\n");
-            buffer.append("      .alwaysCalc         = ").append(alwaysCalc.isSet(getOptions()))
-                .append("\n");
-            buffer.append("      .calcOnLoad         = ").append(calcOnLoad.isSet(getOptions()))
-                .append("\n");
-            buffer.append("      .sharedFormula         = ").append(sharedFormula.isSet(getOptions()))
-                .append("\n");
-            buffer.append("    .zero            = ").append(field_6_zero)
-                .append("\n");
-            buffer.append("    .expressionlength= ").append(getExpressionLength())
-                .append("\n");
-
-            if (field_8_parsed_expr != null) {
-                buffer.append("    .numptgsinarray  = ").append(field_8_parsed_expr.size())
-                    .append("\n");
-            
-
-                for (int k = 0; k < field_8_parsed_expr.size(); k++ ) {
-                   buffer.append("     Ptg(")
-                        .append(k)
-                        .append(")=")
-                        .append(field_8_parsed_expr.get(k).toString())
-                        .append("\n")
-                        .append(((Ptg)field_8_parsed_expr.get(k)).toDebugString())
-                        .append("\n");
-                }
-            }else {
-                buffer.append("Formula full data \n")
-                    .append(org.apache.poi.util.HexDump.dump(this.all_data,0,0));
-            }
-            
-            
-            buffer.append("[/FORMULA]\n");
-        return buffer.toString();
+
+    public String toString() {
+
+        StringBuffer sb = new StringBuffer();
+        sb.append("[FORMULA]\n");
+        sb.append("    .row       = ").append(HexDump.shortToHex(getRow())).append("\n");
+        sb.append("    .column    = ").append(HexDump.shortToHex(getColumn())).append("\n");
+        sb.append("    .xf        = ").append(HexDump.shortToHex(getXFIndex())).append("\n");
+        sb.append("    .value     = ");
+        if (Double.isNaN(this.getValue()) && value_data != null) {
+            sb.append("(NaN)").append(HexDump.dump(value_data,0,0)).append("\n");
+        } else {
+            sb.append(getValue()).append("\n");
+        }
+        sb.append("    .options   = ").append(HexDump.shortToHex(getOptions())).append("\n");
+        sb.append("    .alwaysCalc= ").append(alwaysCalc.isSet(getOptions())).append("\n");
+        sb.append("    .calcOnLoad= ").append(calcOnLoad.isSet(getOptions())).append("\n");
+        sb.append("    .shared    = ").append(sharedFormula.isSet(getOptions())).append("\n");
+        sb.append("    .zero      = ").append(HexDump.intToHex(field_6_zero)).append("\n");
+
+        for (int k = 0; k < field_8_parsed_expr.length; k++ ) {
+            sb.append("     Ptg[").append(k).append("]=");
+            sb.append(field_8_parsed_expr[k].toString()).append("\n");
+        }
+        sb.append("[/FORMULA]\n");
+        return sb.toString();
     }
-    
+
     public Object clone() {
       FormulaRecord rec = new FormulaRecord();
       rec.field_1_row = field_1_row;
@@ -450,18 +292,14 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
       rec.field_4_value = field_4_value;
       rec.field_5_options = field_5_options;
       rec.field_6_zero = field_6_zero;
-      rec.field_7_expression_len = field_7_expression_len;
-      rec.field_8_parsed_expr = new Stack();
-      int size = 0;
-      if (field_8_parsed_expr != null)
-        size = field_8_parsed_expr.size();
-      for (int i=0; i< size; i++) {
-        Ptg ptg = ((Ptg)field_8_parsed_expr.get(i)).copy();        
-        rec.field_8_parsed_expr.add(i, ptg);
+      int nTokens = field_8_parsed_expr.length;
+      Ptg[] ptgs = new Ptg[nTokens];
+      for (int i=0; i< nTokens; i++) {
+        ptgs[i] = field_8_parsed_expr[i].copy();
       }
+      rec.field_8_parsed_expr = ptgs;
       rec.value_data = value_data;
-      rec.all_data = all_data;
       return rec;
     }
-
 }
+
diff --git a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java
index a8aeed0dae..3f3a047e64 100755
--- a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java
+++ b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java
@@ -14,17 +14,22 @@
    See the License for the specific language governing permissions and
    limitations under the License.
 ==================================================================== */
- 
+
 package org.apache.poi.hssf.record;
 
+import java.util.List;
 import java.util.Stack;
 
-import org.apache.poi.hssf.record.formula.*;
+import org.apache.poi.hssf.record.formula.AreaNPtg;
+import org.apache.poi.hssf.record.formula.AreaPtg;
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.RefNPtg;
+import org.apache.poi.hssf.record.formula.RefPtg;
 
 /**
  * Title:        SharedFormulaRecord
  * Description:  Primarily used as an excel optimization so that multiple similar formulas
- * 				  are not written out too many times.  We should recognize this record and
+ *               are not written out too many times.  We should recognize this record and
  *               serialize as is since this is used when reading templates.
  * <p>
  * Note: the documentation says that the SID is BC where biffviewer reports 4BC.  The hex dump shows
@@ -33,15 +38,15 @@ import org.apache.poi.hssf.record.formula.*;
  * @author Danny Mui at apache dot org
  */
 public final class SharedFormulaRecord extends Record {
-    public final static short   sid = 0x4BC;
-    
+    public final static short   sid = 0x04BC;
+
     private int               field_1_first_row;
     private int               field_2_last_row;
     private short             field_3_first_column;
     private short             field_4_last_column;
     private int               field_5_reserved;
     private short             field_6_expression_len;
-    private Stack             field_7_parsed_expr;    
+    private Stack             field_7_parsed_expr;
 
     public SharedFormulaRecord()
     {
@@ -55,15 +60,15 @@ public final class SharedFormulaRecord extends Record {
     {
           super(in);
     }
-    
+
     protected void validateSid(short id)
     {
         if (id != this.sid)
         {
             throw new RecordFormatException("Not a valid SharedFormula");
-        }        
-    }    
-    
+        }
+    }
+
     public int getFirstRow() {
       return field_1_first_row;
     }
@@ -139,7 +144,7 @@ public final class SharedFormulaRecord extends Record {
                 .append(field_7_parsed_expr.get(k).toString())
                 .append("\n");
         }
-        
+
         buffer.append("[/SHARED FORMULA RECORD]\n");
         return buffer.toString();
     }
@@ -163,7 +168,7 @@ public final class SharedFormulaRecord extends Record {
     private Stack getParsedExpressionTokens(RecordInputStream in)
     {
         Stack stack = new Stack();
-        
+
         while (in.remaining() != 0) {
             Ptg ptg = Ptg.createPtg(in);
             stack.push(ptg);
@@ -180,15 +185,15 @@ public final class SharedFormulaRecord extends Record {
       return ((getFirstRow() <= formulaRow) && (getLastRow() >= formulaRow) &&
           (getFirstColumn() <= formulaColumn) && (getLastColumn() >= formulaColumn));
     }
-    
+
     /**
-     * Creates a non shared formula from the shared formula 
+     * Creates a non shared formula from the shared formula
      * counter part
      */
     protected static Stack convertSharedFormulas(Stack ptgs, int formulaRow, int formulaColumn) {
         if(false) {
             /*
-             * TODO - (May-2008) Stop converting relative ref Ptgs in shared formula records. 
+             * TODO - (May-2008) Stop converting relative ref Ptgs in shared formula records.
              * If/when POI writes out the workbook, this conversion makes an unnecessary diff in the BIFF records.
              * Disabling this code breaks one existing junit.
              * Some fix-up will be required to make Ptg.toFormulaString(HSSFWorkbook) work properly.
@@ -225,31 +230,30 @@ public final class SharedFormulaRecord extends Record {
             if (!ptg.isBaseToken()) {
                 ptg.setClass(originalOperandClass);
             }
-            
+
             newPtgStack.add(ptg);
         }
         return newPtgStack;
     }
 
-    /** 
-     * Creates a non shared formula from the shared formula 
+    /**
+     * Creates a non shared formula from the shared formula
      * counter part
      */
     public void convertSharedFormulaRecord(FormulaRecord formula) {
       //Sanity checks
-      final int formulaRow = formula.getRow();
-      final int formulaColumn = formula.getColumn();
-      if (isFormulaInShared(formula)) {
-        formula.setExpressionLength(getExpressionLength());
-        
-        Stack newPtgStack = 
-        	convertSharedFormulas(field_7_parsed_expr, formulaRow, formulaColumn);
-        formula.setParsedExpression(newPtgStack);
+        if (!isFormulaInShared(formula)) {
+            throw new RuntimeException("Shared Formula Conversion: Coding Error");
+        }
+        final int formulaRow = formula.getRow();
+        final int formulaColumn = formula.getColumn();
+
+        List ptgList =  convertSharedFormulas(field_7_parsed_expr, formulaRow, formulaColumn);
+        Ptg[] ptgs = new Ptg[ptgList.size()];
+        ptgList.toArray(ptgs);
+        formula.setParsedExpression(ptgs);
         //Now its not shared!
         formula.setSharedFormula(false);
-      } else {
-        throw new RuntimeException("Shared Formula Conversion: Coding Error");
-      }
     }
 
     private static int fixupRelativeColumn(int currentcolumn, int column, boolean relative) {
diff --git a/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java b/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java
index 3359ca55a4..393f1c0c05 100644
--- a/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java
+++ b/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java
@@ -17,9 +17,12 @@
 
 package org.apache.poi.hssf.record.aggregates;
 
+import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.CellValueRecordInterface;
 import org.apache.poi.hssf.record.FormulaRecord;
+import org.apache.poi.hssf.record.SharedFormulaRecord;
 import org.apache.poi.hssf.record.StringRecord;
+import org.apache.poi.hssf.record.TableRecord;
 
 /**
  * The formula record aggregate is used to join together the formula record and it's
@@ -29,61 +32,67 @@ import org.apache.poi.hssf.record.StringRecord;
  */
 public final class FormulaRecordAggregate extends RecordAggregate implements CellValueRecordInterface {
 
-    private FormulaRecord _formulaRecord;
+    private final FormulaRecord _formulaRecord;
+    /** caches the calculated result of the formula */
     private StringRecord _stringRecord;
+    private TableRecord _tableRecord;
     
-    public FormulaRecordAggregate( FormulaRecord formulaRecord, StringRecord stringRecord )
-    {
+    public FormulaRecordAggregate(FormulaRecord formulaRecord) {
         _formulaRecord = formulaRecord;
-        _stringRecord = stringRecord;
+        _stringRecord = null;
     }
-
-    public void setStringRecord( StringRecord stringRecord ) {
-        _stringRecord = stringRecord;
+    public FormulaRecordAggregate(FormulaRecord formulaRecord, RecordStream rs) {
+        _formulaRecord = formulaRecord;
+        Class nextClass = rs.peekNextClass();
+        if (nextClass == SharedFormulaRecord.class) {
+            // For (text) shared formulas, the SharedFormulaRecord comes before the StringRecord.
+            // In any case it is OK to skip SharedFormulaRecords because they were collected 
+            // before constructing the ValueRecordsAggregate.
+            rs.getNext(); // skip the shared formula record
+            nextClass = rs.peekNextClass();
+        }
+        if (nextClass == StringRecord.class) {
+            _stringRecord = (StringRecord) rs.getNext();
+        } else if (nextClass == TableRecord.class) {
+            _tableRecord = (TableRecord) rs.getNext();
+        }
     }
 
-    public void setFormulaRecord( FormulaRecord formulaRecord )
-    {
-        _formulaRecord = formulaRecord;
+    public void setStringRecord(StringRecord stringRecord) {
+        _stringRecord = stringRecord;
+        _tableRecord = null; // probably can't have both present at the same time
+        // TODO - establish rules governing when each of these sub records may exist
     }
     
-    public FormulaRecord getFormulaRecord()
-    {
+    public FormulaRecord getFormulaRecord() {
         return _formulaRecord;
     }
 
-    public StringRecord getStringRecord()
-    {
+    public StringRecord getStringRecord() {
         return _stringRecord;
     }
     
-    public short getXFIndex()
-    {
+    public short getXFIndex() {
         return _formulaRecord.getXFIndex();
     }
 
-    public void setXFIndex(short xf)
-    {
-        _formulaRecord.setXFIndex( xf );
+    public void setXFIndex(short xf) {
+        _formulaRecord.setXFIndex(xf);
     }
 
-    public void setColumn(short col)
-    {
-        _formulaRecord.setColumn( col );
+    public void setColumn(short col) {
+        _formulaRecord.setColumn(col);
     }
 
-    public void setRow(int row)
-    {
-        _formulaRecord.setRow( row );
+    public void setRow(int row) {
+        _formulaRecord.setRow(row);
     }
 
-    public short getColumn()
-    {
+    public short getColumn() {
         return _formulaRecord.getColumn();
     }
 
-    public int getRow()
-    {
+    public int getRow() {
         return _formulaRecord.getRow();
     }
 
@@ -94,8 +103,11 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
     public void visitContainedRecords(RecordVisitor rv) {
          rv.visitRecord(_formulaRecord);
          if (_stringRecord != null) {
-              rv.visitRecord(_stringRecord);
+             rv.visitRecord(_stringRecord);
          }
+         if (_tableRecord != null) {
+             rv.visitRecord(_tableRecord);
+        }
     }
    
     public String getStringValue() {
diff --git a/src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java b/src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java
index d839ecfab6..fcbc89f63b 100644
--- a/src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java
+++ b/src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java
@@ -82,7 +82,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
             if (!rec.isValue()) {
                 throw new RuntimeException("Unexpected record type (" + rec.getClass().getName() + ")");
             }
-            i += _valuesAgg.construct(recs, i, endIx, sfh);
+            i += _valuesAgg.construct(recs, i, endIx, sfh)-1;
         }
         "".length();
     }
diff --git a/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java b/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java
index 0db1201432..886bb617d5 100644
--- a/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java
+++ b/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java
@@ -21,6 +21,7 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
+import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.CellValueRecordInterface;
 import org.apache.poi.hssf.record.DBCellRecord;
 import org.apache.poi.hssf.record.FormulaRecord;
@@ -111,12 +112,12 @@ public final class ValueRecordsAggregate {
 
     public void removeAllCellsValuesForRow(int rowIndex) {
         if (rowIndex >= records.length) {
-            throw new IllegalArgumentException("Specified rowIndex " + rowIndex 
+            throw new IllegalArgumentException("Specified rowIndex " + rowIndex
                     + " is outside the allowable range (0.." +records.length + ")");
         }
         records[rowIndex] = null;
     }
-    
+
 
     public int getPhysicalNumberOfCells()
     {
@@ -142,62 +143,48 @@ public final class ValueRecordsAggregate {
     }
 
     /**
-     * Processes a sequential group of cell value records.  Stops at endIx or the first 
+     * Processes a sequential group of cell value records.  Stops at endIx or the first
      * non-value record encountered.
      * @param sfh used to resolve any shared formulas for the current sheet
      * @return the number of records consumed
      */
     public int construct(List records, int offset, int endIx, SharedFormulaHolder sfh) {
-        int k = 0;
+        RecordStream rs = new RecordStream(records, offset, endIx);
 
-        FormulaRecordAggregate lastFormulaAggregate = null;
-        
         // Now do the main processing sweep
-        for (k = offset; k < endIx; k++) {
-            Record rec = ( Record ) records.get(k);
-
-            if (rec instanceof StringRecord) {
-                if (lastFormulaAggregate == null) {
-                    throw new RuntimeException("StringRecord found without preceding FormulaRecord");
-                }
-                if (lastFormulaAggregate.getStringRecord() != null) {
-                    throw new RuntimeException("Multiple StringRecords found after FormulaRecord");
-                }
-                lastFormulaAggregate.setStringRecord((StringRecord)rec);
-                lastFormulaAggregate = null;
-                continue;
-            }
-            
-            if (rec instanceof TableRecord) {
-                // TODO - don't loose this record
-                // DATATABLE probably belongs in formula record aggregate
-                if (lastFormulaAggregate == null) {
-                    throw new RuntimeException("No preceding formula record found");
-                }
-                lastFormulaAggregate = null;
-                continue; 
+        while (rs.hasNext()) {
+            Class recClass = rs.peekNextClass();
+            if (recClass == StringRecord.class) {
+                throw new RuntimeException("Loose StringRecord found without preceding FormulaRecord");
             }
-            
-            if (rec instanceof SharedFormulaRecord) {
-                // Already handled, not to worry
-                continue;
+
+            if (recClass == TableRecord.class) {
+                throw new RuntimeException("Loose TableRecord found without preceding FormulaRecord");
             }
 
-            if (rec instanceof UnknownRecord) {
+            if (recClass == UnknownRecord.class) {
                 break;
             }
-            if (rec instanceof RowRecord) {
-                break; 
+            if (recClass == RowRecord.class) {
+                break;
             }
-            if (rec instanceof DBCellRecord) {
+            if (recClass == DBCellRecord.class) {
                 // end of 'Row Block'.  This record is ignored by POI
                 break;
             }
-            if (rec instanceof MergeCellsRecord) {
+
+            Record rec = rs.getNext();
+
+            if (recClass == SharedFormulaRecord.class) {
+                // Already handled, not to worry
+                continue;
+            }
+            if (recClass == MergeCellsRecord.class) {
                 // doesn't really belong here
                 // can safely be ignored, because it has been processed in a higher method
                 continue;
             }
+
             if (!rec.isValue()) {
                 throw new RuntimeException("bad record type");
             }
@@ -206,14 +193,13 @@ public final class ValueRecordsAggregate {
                 if (formula.isSharedFormula()) {
                     sfh.convertSharedFormulaRecord(formula);
                 }
-                
-                lastFormulaAggregate = new FormulaRecordAggregate((FormulaRecord)rec, null);
-                insertCell( lastFormulaAggregate );
+
+                insertCell(new FormulaRecordAggregate((FormulaRecord)rec, rs));
                 continue;
             }
             insertCell(( CellValueRecordInterface ) rec);
         }
-        return k - offset - 1;
+        return rs.getCountRead();
     }
 
     /** Tallies a count of the size of the cell records
@@ -235,7 +221,7 @@ public final class ValueRecordsAggregate {
 
     /** Returns true if the row has cells attached to it */
     public boolean rowHasCells(int row) {
-        if (row > records.length-1) //previously this said row > records.length which means if 
+        if (row > records.length-1) //previously this said row > records.length which means if
             return false;  // if records.length == 60 and I pass "60" here I get array out of bounds
       CellValueRecordInterface[] rowCells=records[row]; //because a 60 length array has the last index = 59
       if(rowCells==null) return false;
@@ -260,7 +246,7 @@ public final class ValueRecordsAggregate {
         }
         return pos - offset;
     }
-    
+
     public int visitCellsForRow(int rowIndex, RecordVisitor rv) {
         int result = 0;
         CellValueRecordInterface[] cellRecs = records[rowIndex];
@@ -292,7 +278,7 @@ public final class ValueRecordsAggregate {
 
     public CellValueRecordInterface[] getValueRecords() {
         List temp = new ArrayList();
-        
+
         for (int i = 0; i < records.length; i++) {
             CellValueRecordInterface[] rowCells = records[i];
             if (rowCells == null) {
@@ -305,7 +291,7 @@ public final class ValueRecordsAggregate {
                 }
             }
         }
-        
+
         CellValueRecordInterface[] result = new CellValueRecordInterface[temp.size()];
         temp.toArray(result);
         return result;
@@ -314,7 +300,7 @@ public final class ValueRecordsAggregate {
     {
     return new MyIterator();
     }
-  
+
   private final class MyIterator implements Iterator {
     short nextColumn=-1;
     int nextRow,lastRow;
@@ -325,7 +311,7 @@ public final class ValueRecordsAggregate {
       this.lastRow=records.length-1;
       findNext();
     }
-    
+
     public MyIterator(int firstRow,int lastRow)
     {
       this.nextRow=firstRow;
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 97fad47b9e..f2e836ffcc 100644
--- a/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java
+++ b/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java
@@ -39,6 +39,11 @@ public final class ArrayPtg extends Ptg {
 	public static final byte sid  = 0x20;
 
 	private static final int RESERVED_FIELD_LEN = 7;
+	/** 
+	 * The size of the plain tArray token written within the standard formula tokens
+	 * (not including the data which comes after all formula tokens)
+	 */
+	public static final int PLAIN_TOKEN_SIZE = 1+RESERVED_FIELD_LEN;
 	// TODO - fix up field visibility and subclasses
 	private byte[] field_1_reserved;
 	
@@ -123,7 +128,7 @@ public final class ArrayPtg extends Ptg {
 	public int writeTokenValueBytes(byte[] data, int offset) {
 
 		LittleEndian.putByte(data,  offset + 0, token_1_columns-1);
-		LittleEndian.putShort(data, offset + 1, (short)(token_2_rows-1));
+		LittleEndian.putUShort(data, offset + 1, token_2_rows-1);
 		ConstantValueParser.encode(data, offset + 3, token_3_arrayValues);
 		return 3 + ConstantValueParser.getEncodedSize(token_3_arrayValues);
 	}
@@ -137,11 +142,11 @@ public final class ArrayPtg extends Ptg {
 	}
 
 	/** This size includes the size of the array Ptg plus the Array Ptg Token value size*/
-	public int getSize()
-	{
-		int size = 1+7+1+2;
-		size += ConstantValueParser.getEncodedSize(token_3_arrayValues);
-		return size;
+	public int getSize() {
+		return PLAIN_TOKEN_SIZE 
+			// data written after the all tokens:
+			+ 1 + 2 // column, row
+			+ ConstantValueParser.getEncodedSize(token_3_arrayValues);
 	}
 
 	public String toFormulaString(HSSFWorkbook book)
diff --git a/src/java/org/apache/poi/hssf/record/formula/Ptg.java b/src/java/org/apache/poi/hssf/record/formula/Ptg.java
index d1ea411cb9..3af4991d4e 100644
--- a/src/java/org/apache/poi/hssf/record/formula/Ptg.java
+++ b/src/java/org/apache/poi/hssf/record/formula/Ptg.java
@@ -222,13 +222,6 @@ public abstract class Ptg implements Cloneable {
 		throw new RuntimeException("Unexpected base token id (" + id + ")");
 	}
 	/**
-	 * 
-	 * 
-	 */
-	public static int getEncodedSize(Stack ptgs) {
-		return getEncodedSize(toPtgArray(ptgs));
-	}
-	/**
 	 * @return a distinct copy of this <tt>Ptg</tt> if the class is mutable, or the same instance
 	 * if the class is immutable.
 	 */
@@ -265,6 +258,11 @@ public abstract class Ptg implements Cloneable {
 		} 
 		return result;
 	}
+	/**
+	 * This method will return the same result as {@link #getEncodedSizeWithoutArrayData(Ptg[])} 
+	 * if there are no array tokens present.
+	 * @return the full size taken to encode the specified <tt>Ptg</tt>s 
+	 */
 	// TODO - several duplicates of this code should be refactored here
 	public static int getEncodedSize(Ptg[] ptgs) {
 		int result = 0;
@@ -274,6 +272,22 @@ public abstract class Ptg implements Cloneable {
 		return result;
 	}
 	/**
+	 * Used to calculate value that should be encoded at the start of the encoded Ptg token array;
+	 * @return the size of the encoded Ptg tokens not including any trailing array data.
+	 */
+	public static int getEncodedSizeWithoutArrayData(Ptg[] ptgs) {
+		int result = 0;
+		for (int i = 0; i < ptgs.length; i++) {
+			Ptg ptg = ptgs[i];
+			if (ptg instanceof ArrayPtg) {
+				result += ArrayPtg.PLAIN_TOKEN_SIZE;
+			} else {
+				result += ptg.getSize();
+			}
+		}
+		return result;
+	}
+	/**
 	 * Writes the ptgs to the data buffer, starting at the specified offset.  
 	 *
 	 * <br/>
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java
index 61665fdb62..17e1778c86 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java
@@ -292,7 +292,7 @@ public final class HSSFCell {
 
                 if (cellType != this.cellType)
                 {
-                    frec = new FormulaRecordAggregate(new FormulaRecord(),null);
+                    frec = new FormulaRecordAggregate(new FormulaRecord());
                 }
                 else
                 {
@@ -584,41 +584,27 @@ public final class HSSFCell {
         int row=record.getRow();
         short col=record.getColumn();
         short styleIndex=record.getXFIndex();
-        //Workbook.currentBook=book;
-        if (formula==null) {
-            setCellType(CELL_TYPE_BLANK,false,row,col,styleIndex);
-        } else {
-            setCellType(CELL_TYPE_FORMULA,false,row,col,styleIndex);
-            FormulaRecordAggregate rec = (FormulaRecordAggregate) record;
-            FormulaRecord frec = rec.getFormulaRecord();
-            frec.setOptions(( short ) 2);
-            frec.setValue(0);
-            
-            //only set to default if there is no extended format index already set
-            if (rec.getXFIndex() == (short)0) rec.setXFIndex(( short ) 0x0f);
-            Ptg[] ptgs = FormulaParser.parse(formula, book);
-            int   size = 0;
-
-            // clear the Ptg Stack
-            for (int i=0, iSize=frec.getNumberOfExpressionTokens(); i<iSize; i++) {
-                frec.popExpressionToken();
-            }
 
-            // fill the Ptg Stack with Ptgs of new formula
-            for (int k = 0; k < ptgs.length; k++) {
-                size += ptgs[ k ].getSize();
-                frec.pushExpressionToken(ptgs[ k ]);
-            }
-            rec.getFormulaRecord().setExpressionLength(( short ) size);
-            //Workbook.currentBook = null;
+        if (formula==null) {
+            setCellType(CELL_TYPE_BLANK, false, row, col, styleIndex);
+            return;
         }
+        setCellType(CELL_TYPE_FORMULA, false, row, col, styleIndex);
+        FormulaRecordAggregate rec = (FormulaRecordAggregate) record;
+        FormulaRecord frec = rec.getFormulaRecord();
+        frec.setOptions((short) 2);
+        frec.setValue(0);
+        
+        //only set to default if there is no extended format index already set
+        if (rec.getXFIndex() == (short)0) {
+			rec.setXFIndex((short) 0x0f);
+		}
+        Ptg[] ptgs = FormulaParser.parse(formula, book);
+        frec.setParsedExpression(ptgs);
     }
 
     public String getCellFormula() {
-        //Workbook.currentBook=book;
-        String retval = FormulaParser.toFormulaString(book, ((FormulaRecordAggregate)record).getFormulaRecord().getParsedExpression());
-        //Workbook.currentBook=null;
-        return retval;
+        return FormulaParser.toFormulaString(book, ((FormulaRecordAggregate)record).getFormulaRecord().getParsedExpression());
     }
 
 
diff --git a/src/java/org/apache/poi/util/HexDump.java b/src/java/org/apache/poi/util/HexDump.java
index aaea9d57a2..ed6f128822 100644
--- a/src/java/org/apache/poi/util/HexDump.java
+++ b/src/java/org/apache/poi/util/HexDump.java
@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -15,11 +14,17 @@
    See the License for the specific language governing permissions and
    limitations under the License.
 ==================================================================== */
-        
 
 package org.apache.poi.util;
 
-import java.io.*;
+import java.io.BufferedInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PrintStream;
 import java.text.DecimalFormat;
 
 /**
@@ -29,27 +34,16 @@ import java.text.DecimalFormat;
  * @author Marc Johnson
  * @author Glen Stampoultzis  (glens at apache.org)
  */
-
-public class HexDump
-{
-    public static final String        EOL         =
-        System.getProperty("line.separator");
-//    private static final StringBuffer _lbuffer    = new StringBuffer(8);
-//    private static final StringBuffer _cbuffer    = new StringBuffer(2);
-    private static final char         _hexcodes[] =
-    {
-        '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D',
-        'E', 'F'
-    };
-    private static final int          _shifts[]   =
+public class HexDump {
+    public static final String EOL = System.getProperty("line.separator");
+    private static final char _hexcodes[] = "0123456789ABCDEF".toCharArray();
+    private static final int _shifts[]   =
     {
         60, 56, 52, 48, 44, 40, 36, 32, 28, 24, 20, 16, 12, 8, 4, 0
     };
 
-
-    // all static methods, so no need for a public constructor
-    private HexDump()
-    {
+    private HexDump() {
+        // all static methods, so no need for a public constructor
     }
 
     /**
@@ -69,7 +63,7 @@ public class HexDump
      * @exception IllegalArgumentException if the output stream is
      *            null
      */
-    public synchronized static void dump(final byte [] data, final long offset,
+    public static void dump(final byte [] data, final long offset,
                             final OutputStream stream, final int index, final int length)
             throws IOException, ArrayIndexOutOfBoundsException,
                     IllegalArgumentException
@@ -413,6 +407,50 @@ public class HexDump
         byte[] data = buf.toByteArray();
         dump(data, 0, out, start, data.length);
     }
+    /**
+     * @return char array of uppercase hex chars, zero padded and prefixed with '0x'
+     */
+    private static char[] toHexChars(long pValue, int nBytes) {
+        int charPos = 2 + nBytes*2;
+        // The return type is char array because most callers will probably append the value to a
+        // StringBuffer, or write it to a Stream / Writer so there is no need to create a String;
+        char[] result = new char[charPos];
+        
+        long value = pValue;
+        do {
+            result[--charPos] = _hexcodes[(int) (value & 0x0F)];
+            value >>>= 4;
+        } while (charPos > 1);
+    
+        // Prefix added to avoid ambiguity
+        result[0] = '0';
+        result[1] = 'x';
+        return result;
+    }
+    /**
+     * @return char array of 4 (zero padded) uppercase hex chars and prefixed with '0x'
+     */
+    public static char[] longToHex(long value) {
+        return toHexChars(value, 8);
+    }
+    /**
+     * @return char array of 4 (zero padded) uppercase hex chars and prefixed with '0x'
+     */
+    public static char[] intToHex(int value) {
+        return toHexChars(value, 4);
+    }
+    /**
+     * @return char array of 2 (zero padded) uppercase hex chars and prefixed with '0x'
+     */
+    public static char[] shortToHex(int value) {
+        return toHexChars(value, 2);
+    }
+    /**
+     * @return char array of 1 (zero padded) uppercase hex chars and prefixed with '0x'
+     */
+    public static char[] byteToHex(int value) {
+        return toHexChars(value, 1);
+    }
 
     public static void main(String[] args) throws Exception {
         File file = new File(args[0]);
diff --git a/src/testcases/org/apache/poi/hssf/record/aggregates/TestFormulaRecordAggregate.java b/src/testcases/org/apache/poi/hssf/record/aggregates/TestFormulaRecordAggregate.java
index 88b5477783..b7e43ec4d4 100644
--- a/src/testcases/org/apache/poi/hssf/record/aggregates/TestFormulaRecordAggregate.java
+++ b/src/testcases/org/apache/poi/hssf/record/aggregates/TestFormulaRecordAggregate.java
@@ -35,7 +35,8 @@ public final class TestFormulaRecordAggregate extends junit.framework.TestCase {
         FormulaRecord f = new FormulaRecord();
         StringRecord s = new StringRecord();
         s.setString("abc");
-        FormulaRecordAggregate fagg = new FormulaRecordAggregate(f,s);
+        FormulaRecordAggregate fagg = new FormulaRecordAggregate(f);
+        fagg.setStringRecord(s);
         assertEquals("abc", fagg.getStringValue());
     }
 }
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBug42464.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBug42464.java
index 530ccc0559..1f1eca4fc2 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestBug42464.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBug42464.java
@@ -71,7 +71,7 @@ public final class TestBug42464 extends TestCase {
 			if(false && cellRef.equals("BP24")) { // TODO - replace System.out.println()s with asserts
 				System.out.print(cellRef);
 				System.out.println(" - has " + r.getNumberOfExpressionTokens() 
-				        + " ptgs over " + r.getExpressionLength()  + " tokens:");
+				        + " ptgs:");
 				for(int i=0; i<ptgs.size(); i++) {
 					String c = ptgs.get(i).getClass().toString();
 					System.out.println("\t" + c.substring(c.lastIndexOf('.')+1) );
diff --git a/src/testcases/org/apache/poi/util/TestHexDump.java b/src/testcases/org/apache/poi/util/TestHexDump.java
index 47d006068a..a95ebbc238 100644
--- a/src/testcases/org/apache/poi/util/TestHexDump.java
+++ b/src/testcases/org/apache/poi/util/TestHexDump.java
@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -15,54 +14,26 @@
    See the License for the specific language governing permissions and
    limitations under the License.
 ==================================================================== */
-        
 
 package org.apache.poi.util;
 
-import junit.framework.*;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 
-import java.io.*;
+import junit.framework.TestCase;
 
 /**
  * @author Glen Stampoultzis (glens at apache.org)
  * @author Marc Johnson (mjohnson at apache dot org)
  */
+public final class TestHexDump extends TestCase {
 
-public class TestHexDump
-    extends TestCase
-{
-
-    /**
-     * Creates new TestHexDump
-     *
-     * @param name
-     */
-
-    public TestHexDump(String name)
-    {
-        super(name);
-    }
-
-    private char toHex(final int n)
-    {
-        char[] hexChars =
-        {
-            '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C',
-            'D', 'E', 'F'
-        };
 
-        return hexChars[ n % 16 ];
+    private static char toHex(int n) {
+        return Character.toUpperCase(Character.forDigit(n & 0x0F, 16));
     }
 
-    /**
-     * test dump method
-     *
-     * @exception IOException
-     */
-
-    public void testDump()
-        throws IOException
-    {
+    public void testDump() throws IOException {
         byte[] testArray = new byte[ 256 ];
 
         for (int j = 0; j < 256; j++)
@@ -245,8 +216,7 @@ public class TestHexDump
         // verify proper behavior with negative index
         try
         {
-            HexDump.dump(testArray, 0x10000000, new ByteArrayOutputStream(),
-                         -1);
+            HexDump.dump(testArray, 0x10000000, new ByteArrayOutputStream(), -1);
             fail("should have caught ArrayIndexOutOfBoundsException on negative index");
         }
         catch (ArrayIndexOutOfBoundsException ignored_exception)
@@ -287,37 +257,33 @@ public class TestHexDump
 
     }
 
-    public void testToHex()
-            throws Exception
-    {
-        assertEquals( "000A", HexDump.toHex((short)0xA));
-        assertEquals( "0A", HexDump.toHex((byte)0xA));
-        assertEquals( "0000000A", HexDump.toHex(0xA));
+    public void testToHex() {
+        assertEquals("000A", HexDump.toHex((short)0xA));
+        assertEquals("0A", HexDump.toHex((byte)0xA));
+        assertEquals("0000000A", HexDump.toHex(0xA));
 
-        assertEquals( "FFFF", HexDump.toHex((short)0xFFFF));
+        assertEquals("FFFF", HexDump.toHex((short)0xFFFF));
+        
+        confirmStr("0xFE", HexDump.byteToHex(-2));
+        confirmStr("0x25", HexDump.byteToHex(37));
+        confirmStr("0xFFFE", HexDump.shortToHex(-2));
+        confirmStr("0x0005", HexDump.shortToHex(5));
+        confirmStr("0xFFFFFF9C", HexDump.intToHex(-100));
+        confirmStr("0x00001001", HexDump.intToHex(4097));
+        confirmStr("0xFFFFFFFFFFFF0006", HexDump.longToHex(-65530));
+        confirmStr("0x0000000000003FCD", HexDump.longToHex(16333));
+    }
 
+    private static void confirmStr(String expected, char[] actualChars) {
+        assertEquals(expected, new String(actualChars));
     }
 
-    private char toAscii(final int c)
-    {
+    private static char toAscii(int c) {
         char rval = '.';
 
-        if ((c >= 32) && (c <= 126))
-        {
+        if (c >= 32 && c <= 126) {
             rval = ( char ) c;
         }
         return rval;
     }
-
-    /**
-     * main method to run the unit tests
-     *
-     * @param ignored_args
-     */
-
-    public static void main(String [] ignored_args)
-    {
-        System.out.println("Testing util.HexDump functionality");
-        junit.textui.TestRunner.run(TestHexDump.class);
-    }
 }