Extended support for cached results of formula cells

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

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">Extended support for cached results of formula cells</action>
            <action dev="POI-DEVELOPERS" type="fix">45639 - Fixed AIOOBE due to bad index logic in ColumnInfoRecordsAggregate</action>
            <action dev="POI-DEVELOPERS" type="fix">Fixed special cases of INDEX function (single column/single row, errors)</action>
            <action dev="POI-DEVELOPERS" type="add">45761 - Support for Very Hidden excel sheets in HSSF</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">Extended support for cached results of formula cells</action>
            <action dev="POI-DEVELOPERS" type="fix">45639 - Fixed AIOOBE due to bad index logic in ColumnInfoRecordsAggregate</action>
            <action dev="POI-DEVELOPERS" type="fix">Fixed special cases of INDEX function (single column/single row, errors)</action>
            <action dev="POI-DEVELOPERS" type="add">45761 - Support for Very Hidden excel sheets in HSSF</action>

src/java/org/apache/poi/hssf/extractor/EventBasedExcelExtractor.java

@@ -14,6 +14,7 @@
    See the License for the specific language governing permissions and
    limitations under the License.
 ==================================================================== */
+
 package org.apache.poi.hssf.extractor;
 
 import java.io.IOException;
@@ -64,7 +65,7 @@ public class EventBasedExcelExtractor extends POIOLE2TextExtractor {
 	private boolean includeSheetNames = true;
 	private boolean formulasNotResults = false;
 
-	public EventBasedExcelExtractor(POIFSFileSystem fs) throws IOException {
+	public EventBasedExcelExtractor(POIFSFileSystem fs) {
 		super(null);
 		this.fs = fs;
 	}
@@ -178,7 +179,7 @@ public class EventBasedExcelExtractor extends POIOLE2TextExtractor {
 				if(formulasNotResults) {
 					thisText = FormulaParser.toFormulaString(null, frec.getParsedExpression());
 				} else {
-	        		if(Double.isNaN( frec.getValue() )) {
+					if(frec.hasCachedResultString()) {
 						// Formula result is a string
 						// This is stored in the next record
 						outputNextStringValue = true;

src/java/org/apache/poi/hssf/extractor/ExcelExtractor.java

@@ -14,16 +14,16 @@
    See the License for the specific language governing permissions and
    limitations under the License.
 ==================================================================== */
+
 package org.apache.poi.hssf.extractor;
 
 import java.io.IOException;
 
 import org.apache.poi.POIOLE2TextExtractor;
+import org.apache.poi.hssf.record.formula.eval.ErrorEval;
 import org.apache.poi.hssf.usermodel.HeaderFooter;
 import org.apache.poi.hssf.usermodel.HSSFCell;
 import org.apache.poi.hssf.usermodel.HSSFComment;
-import org.apache.poi.hssf.usermodel.HSSFFooter;
-import org.apache.poi.hssf.usermodel.HSSFHeader;
 import org.apache.poi.hssf.usermodel.HSSFRichTextString;
 import org.apache.poi.hssf.usermodel.HSSFRow;
 import org.apache.poi.hssf.usermodel.HSSFSheet;
@@ -110,40 +110,52 @@ public class ExcelExtractor extends POIOLE2TextExtractor {
 				int lastCell = row.getLastCellNum();
 				for(int k=firstCell;k<lastCell;k++) {
 					HSSFCell cell = row.getCell(k);
-					boolean outputContents = false;
 					if(cell == null) { continue; }
+					boolean outputContents = true;
 					
 					switch(cell.getCellType()) {
+						case HSSFCell.CELL_TYPE_BLANK:
+							outputContents = false;
+							break;
 						case HSSFCell.CELL_TYPE_STRING:
 							text.append(cell.getRichStringCellValue().getString());
-							outputContents = true;
 							break;
 						case HSSFCell.CELL_TYPE_NUMERIC:
 							// Note - we don't apply any formatting!
 							text.append(cell.getNumericCellValue());
-							outputContents = true;
 							break;
 						case HSSFCell.CELL_TYPE_BOOLEAN:
 							text.append(cell.getBooleanCellValue());
-							outputContents = true;
+							break;
+						case HSSFCell.CELL_TYPE_ERROR:
+							text.append(ErrorEval.getText(cell.getErrorCellValue()));
 							break;
 						case HSSFCell.CELL_TYPE_FORMULA:
 							if(formulasNotResults) {
 								text.append(cell.getCellFormula());
 							} else {
-								// Try it as a string, if not as a number
+								switch(cell.getCachedFormulaResultType()) {
-								HSSFRichTextString str = 
+									case HSSFCell.CELL_TYPE_STRING:
-									cell.getRichStringCellValue();
+										HSSFRichTextString str = cell.getRichStringCellValue();
 										if(str != null && str.length() > 0) {
 											text.append(str.toString());
-								} else {
-									// Try and treat it as a number
-									double val = cell.getNumericCellValue();
-									text.append(val);
 										}
-							}
-							outputContents = true;
 										break;
+									case HSSFCell.CELL_TYPE_NUMERIC:
+										text.append(cell.getNumericCellValue());
+										break;
+									case HSSFCell.CELL_TYPE_BOOLEAN:
+										text.append(cell.getBooleanCellValue());
+										break;
+									case HSSFCell.CELL_TYPE_ERROR:
+										text.append(ErrorEval.getText(cell.getErrorCellValue()));
+										break;
+										
+								}
+							}
+							break;
+						default:
+							throw new RuntimeException("Unexpected cell type (" + cell.getCellType() + ")");
 					}
 					
 					// Output the comment, if requested and exists

src/java/org/apache/poi/hssf/record/FormulaRecord.java

@@ -18,6 +18,8 @@
 package org.apache.poi.hssf.record;
 
 import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.eval.ErrorEval;
+import org.apache.poi.hssf.usermodel.HSSFCell;
 import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
 import org.apache.poi.util.HexDump;
@@ -39,6 +41,132 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 	private static final BitField calcOnLoad = BitFieldFactory.getInstance(0x0002);
 	private static final BitField sharedFormula = BitFieldFactory.getInstance(0x0008);
 
+	/**
+	 * Manages the cached formula result values of other types besides numeric.
+	 * Excel encodes the same 8 bytes that would be field_4_value with various NaN
+	 * values that are decoded/encoded by this class. 
+	 */
+	private static final class SpecialCachedValue {
+		/** deliberately chosen by Excel in order to encode other values within Double NaNs */
+		private static final long BIT_MARKER = 0xFFFF000000000000L;
+		private static final int VARIABLE_DATA_LENGTH = 6;
+		private static final int DATA_INDEX = 2;
+
+		public static final int STRING = 0;
+		public static final int BOOLEAN = 1;
+		public static final int ERROR_CODE = 2;
+		public static final int EMPTY = 3;
+
+		private final byte[] _variableData;
+
+		private SpecialCachedValue(byte[] data) {
+			_variableData = data;
+		}
+		public int getTypeCode() {
+			return _variableData[0];
+		}
+
+		/**
+		 * @return <code>null</code> if the double value encoded by <tt>valueLongBits</tt> 
+		 * is a normal (non NaN) double value.
+		 */
+		public static SpecialCachedValue create(long valueLongBits) {
+			if ((BIT_MARKER & valueLongBits) != BIT_MARKER) {
+				return null;
+			}
+
+			byte[] result = new byte[VARIABLE_DATA_LENGTH];
+			long x = valueLongBits;
+			for (int i=0; i<VARIABLE_DATA_LENGTH; i++) {
+				result[i] = (byte) x;
+				x >>= 8;
+			}
+			switch (result[0]) {
+				case STRING:
+				case BOOLEAN:
+				case ERROR_CODE:
+				case EMPTY:
+					break;
+				default:
+					throw new RecordFormatException("Bad special value code (" + result[0] + ")");
+			}
+			return new SpecialCachedValue(result);
+		}
+		public void serialize(byte[] data, int offset) {
+			System.arraycopy(_variableData, 0, data, offset, VARIABLE_DATA_LENGTH);
+			LittleEndian.putUShort(data, offset+VARIABLE_DATA_LENGTH, 0xFFFF);
+		}
+		public String formatDebugString() {
+			return formatValue() + ' ' + HexDump.toHex(_variableData);
+		}
+		private String formatValue() {
+			int typeCode = getTypeCode();
+			switch (typeCode) {
+				case STRING:	 return "<string>";
+				case BOOLEAN:	return getDataValue() == 0 ? "FALSE" : "TRUE";
+				case ERROR_CODE: return ErrorEval.getText(getDataValue());
+				case EMPTY:	  return "<empty>";
+			}
+			return "#error(type=" + typeCode + ")#";
+		}
+		private int getDataValue() {
+			return _variableData[DATA_INDEX];
+		}
+		public static SpecialCachedValue createCachedEmptyValue() {
+			return create(EMPTY, 0);
+		}
+		public static SpecialCachedValue createForString() {
+			return create(STRING, 0);
+		}
+		public static SpecialCachedValue createCachedBoolean(boolean b) {
+			return create(BOOLEAN, b ? 0 : 1);
+		}
+		public static SpecialCachedValue createCachedErrorCode(int errorCode) {
+			return create(ERROR_CODE, errorCode);
+		}
+		private static SpecialCachedValue create(int code, int data) {
+			byte[] vd = {
+					(byte) code,
+					0,
+					(byte) data,
+					0,
+					0,
+					0,
+			};
+			return new SpecialCachedValue(vd);
+		}
+		public String toString() {
+			StringBuffer sb = new StringBuffer(64);
+			sb.append(getClass().getName());
+			sb.append('[').append(formatValue()).append(']');
+			return sb.toString();
+		}
+		public int getValueType() {
+			int typeCode = getTypeCode();
+			switch (typeCode) {
+				case STRING:	 return HSSFCell.CELL_TYPE_STRING;
+				case BOOLEAN:	return HSSFCell.CELL_TYPE_BOOLEAN;
+				case ERROR_CODE: return HSSFCell.CELL_TYPE_ERROR;
+				case EMPTY:	  return HSSFCell.CELL_TYPE_STRING; // is this correct?
+			}
+			throw new IllegalStateException("Unexpected type id (" + typeCode + ")");
+		}
+		public boolean getBooleanValue() {
+			if (getTypeCode() != BOOLEAN) {
+				throw new IllegalStateException("Not a boolean cached value - " + formatValue());
+			}
+			return getDataValue() != 0;
+		}
+		public int getErrorValue() {
+			if (getTypeCode() != ERROR_CODE) {
+				throw new IllegalStateException("Not an error cached value - " + formatValue());
+			}
+			return getDataValue();
+		}
+	}
+
+
+
 	private int    field_1_row;
 	private short  field_2_column;
 	private short  field_3_xf;
@@ -50,7 +178,7 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 	/**
 	 * Since the NaN support seems sketchy (different constants) we'll store and spit it out directly
 	 */
-    private byte[]            value_data;
+	private SpecialCachedValue specialCachedValue;
 
 	/** Creates new FormulaRecord */
 
@@ -75,11 +203,11 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 		field_1_row	 = in.readUShort();
 		field_2_column  = in.readShort();
 		field_3_xf	  = in.readShort();
-          field_4_value          = in.readDouble();
+		long valueLongBits  = in.readLong();
 		field_5_options = in.readShort();
-
+		specialCachedValue = SpecialCachedValue.create(valueLongBits);
-        if (Double.isNaN(field_4_value)) {
+		if (specialCachedValue == null) {
-            value_data = in.getNANData();
+			field_4_value = Double.longBitsToDouble(valueLongBits);
 		}
 
 		field_6_zero		   = in.readInt();
@@ -92,6 +220,7 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 		}
 	}
 
+
 	public void setRow(int row) {
 		field_1_row = row;
 	}
@@ -111,8 +240,48 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 	 */
 	public void setValue(double value) {
 		field_4_value = value;
+		specialCachedValue = null;
 	}
 
+	public void setCachedResultTypeEmptyString() {
+		specialCachedValue = SpecialCachedValue.createCachedEmptyValue();
+	}
+	public void setCachedResultTypeString() {
+		specialCachedValue = SpecialCachedValue.createForString();
+	}
+	public void setCachedResultErrorCode(int errorCode) {
+		specialCachedValue = SpecialCachedValue.createCachedErrorCode(errorCode);
+	}
+	public void setCachedResultBoolean(boolean value) {
+		specialCachedValue = SpecialCachedValue.createCachedBoolean(value);
+	}
+	/**
+	 * @return <code>true</code> if this {@link FormulaRecord} is followed by a
+	 *  {@link StringRecord} representing the cached text result of the formula
+	 *  evaluation.
+	 */
+	public boolean hasCachedResultString() {
+		if (specialCachedValue == null) {
+			return false;
+		}
+		return specialCachedValue.getTypeCode() == SpecialCachedValue.STRING;
+	}
+
+	public int getCachedResultType() {
+		if (specialCachedValue == null) {
+			return HSSFCell.CELL_TYPE_NUMERIC;
+		}
+		return specialCachedValue.getValueType();
+	}
+
+	public boolean getCachedBooleanValue() {
+		return specialCachedValue.getBooleanValue();
+	}
+	public int getCachedErrorValue() {
+		return specialCachedValue.getErrorValue();
+	}
+
+
 	/**
 	 * set the option flags
 	 *
@@ -216,11 +385,10 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 		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 (specialCachedValue == null) {
-        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);
+		} else {
+			specialCachedValue.serialize(data, 10+offset);
 		}
 
 		LittleEndian.putShort(data, 18 + offset, getOptions());
@@ -254,10 +422,10 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 		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) {
+		if (specialCachedValue == null) {
-            sb.append("(NaN)").append(HexDump.dump(value_data,0,0)).append("\n");
+			sb.append(field_4_value).append("\n");
 		} else {
-            sb.append(getValue()).append("\n");
+			sb.append(specialCachedValue.formatDebugString()).append("\n");
 		}
 		sb.append("	.options   = ").append(HexDump.shortToHex(getOptions())).append("\n");
 		sb.append("	.alwaysCalc= ").append(alwaysCalc.isSet(getOptions())).append("\n");
@@ -288,7 +456,7 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 			ptgs[i] = field_8_parsed_expr[i].copy();
 		}
 		rec.field_8_parsed_expr = ptgs;
-      rec.value_data = value_data;
+		rec.specialCachedValue = specialCachedValue;
 		return rec;
 	}
 }

src/java/org/apache/poi/hssf/record/RecordInputStream.java

@@ -209,30 +209,18 @@ public class RecordInputStream extends InputStream {
     return result;
   }
 
-  byte[] NAN_data = null;
   public double readDouble() {
     checkRecordPosition();
-    //Reset NAN data
+    long valueLongBits = LittleEndian.getLong(data, recordOffset);
-    NAN_data = null;
+    double result = Double.longBitsToDouble(valueLongBits);
-    double result = LittleEndian.getDouble(data, recordOffset);
-    //Excel represents NAN in several ways, at this point in time we do not often
-    //know the sequence of bytes, so as a hack we store the NAN byte sequence
-    //so that it is not corrupted.
     if (Double.isNaN(result)) {
-      NAN_data = new byte[8];
+      throw new RuntimeException("Did not expect to read NaN");
-      System.arraycopy(data, recordOffset, NAN_data, 0, 8);
     }
-    
     recordOffset += LittleEndian.DOUBLE_SIZE;
     pos += LittleEndian.DOUBLE_SIZE;
     return result;
   }
 
-  public byte[] getNANData() {
-    if (NAN_data == null)
-      throw new RecordFormatException("Do NOT call getNANData without calling readDouble that returns NaN");
-    return NAN_data;
-  }
   
   public short[] readShortArray() {
     checkRecordPosition();
@@ -276,9 +264,6 @@ public class RecordInputStream extends InputStream {
   }
     
   public String readCompressedUnicode(int length) {
-    if(length == 0) {
-        return "";
-    }
     if ((length < 0) || ((remaining() < length) && !isContinueNext())) {
             throw new IllegalArgumentException("Illegal length " + length);
     }
@@ -291,9 +276,7 @@ public class RecordInputStream extends InputStream {
           if(compressByte != 0) throw new IllegalArgumentException("compressByte in continue records must be 0 while reading compressed unicode");
       }
       byte b = readByte();
-      //Typecast direct to char from byte with high bit set causes all ones
+      char ch = (char)(0x00FF & b); // avoid sex
-      //in the high byte of the char (which is of course incorrect)
-      char ch = (char)( (short)0xff & (short)b );
       buf.append(ch); 
     }
     return buf.toString();    

src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java

@@ -20,6 +20,7 @@ package org.apache.poi.hssf.record.aggregates;
 import org.apache.poi.hssf.record.CellValueRecordInterface;
 import org.apache.poi.hssf.record.FormulaRecord;
 import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.RecordFormatException;
 import org.apache.poi.hssf.record.StringRecord;
 
 /**
@@ -44,6 +45,14 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
         if (svm == null) {
             throw new IllegalArgumentException("sfm must not be null");
         }
+        boolean hasStringRec = stringRec != null;
+        boolean hasCachedStringFlag = formulaRec.hasCachedResultString();
+		if (hasStringRec != hasCachedStringFlag) {
+			throw new RecordFormatException("String record was "
+					+ (hasStringRec ? "": "not ") + " supplied but formula record flag is "
+					+ (hasCachedStringFlag ? "" : "not ") + " set");
+		}
+        	
         if (formulaRec.isSharedFormula()) {
             svm.convertSharedFormulaRecord(formulaRec);
         }
@@ -52,14 +61,14 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
         _stringRecord = stringRec;
     }
 
-    public void setStringRecord(StringRecord stringRecord) {
-        _stringRecord = stringRecord;
-    }
-    
     public FormulaRecord getFormulaRecord() {
         return _formulaRecord;
     }
 
+    /**
+     * debug only
+     * TODO - encapsulate
+     */
     public StringRecord getStringRecord() {
         return _stringRecord;
     }
@@ -109,4 +118,26 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
         }
         return _stringRecord.getString();
     }
+
+    public void setCachedStringResult(String value) {
+
+        // Save the string into a String Record, creating one if required
+        if(_stringRecord == null) {
+            _stringRecord = new StringRecord();
+        }
+        _stringRecord.setString(value);
+        if (value.length() < 1) {
+            _formulaRecord.setCachedResultTypeEmptyString();
+        } else {
+            _formulaRecord.setCachedResultTypeString();
+        }
+    }
+    public void setCachedBooleanResult(boolean value) {
+        _stringRecord = null;
+        _formulaRecord.setCachedResultBoolean(value);
+    }
+    public void setCachedErrorResult(int errorCode) {
+        _stringRecord = null;
+        _formulaRecord.setCachedResultErrorCode(errorCode);
+    }
 }

src/java/org/apache/poi/hssf/usermodel/HSSFCell.java

@@ -23,6 +23,7 @@ import java.util.Calendar;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.List;
 
 import org.apache.poi.hssf.model.FormulaParser;
 import org.apache.poi.hssf.model.Sheet;
@@ -86,10 +87,11 @@ public final class HSSFCell {
     public final static short        ENCODING_UNCHANGED          = -1;
     public final static short        ENCODING_COMPRESSED_UNICODE = 0;
     public final static short        ENCODING_UTF_16             = 1;
+
+    private final HSSFWorkbook       book;
+    private final Sheet              sheet;
     private int                      cellType;
     private HSSFRichTextString       stringValue;
-    private HSSFWorkbook             book;
-    private Sheet                    sheet;
     private CellValueRecordInterface record;
     private HSSFComment              comment;
 
@@ -122,6 +124,9 @@ public final class HSSFCell {
         short xfindex = sheet.getXFIndexForColAt(col);
         setCellType(CELL_TYPE_BLANK, false, row, col,xfindex);
     }
+    /* package */ Sheet getSheet() {
+        return sheet;
+    }
 
     /**
      * Creates new Cell - Should only be called by HSSFRow.  This creates a cell
@@ -417,7 +422,7 @@ public final class HSSFCell {
                 errRec.setColumn(col);
                 if (setValue)
                 {
-                    errRec.setValue(getErrorCellValue());
+                    errRec.setValue((byte)HSSFErrorConstants.ERROR_VALUE);
                 }
                 errRec.setXFIndex(styleIndex);
                 errRec.setRow(row);
@@ -453,21 +458,20 @@ public final class HSSFCell {
      *        precalculated value, for numerics we'll set its value. For other types we
      *        will change the cell to a numeric cell and set its value.
      */
-    public void setCellValue(double value)
+    public void setCellValue(double value) {
-    {
         int row=record.getRow();
         short col=record.getColumn();
         short styleIndex=record.getXFIndex();
-        if ((cellType != CELL_TYPE_NUMERIC) && (cellType != CELL_TYPE_FORMULA))
-        {
-            setCellType(CELL_TYPE_NUMERIC, false, row, col, styleIndex);
-        }
 
-        // Save into the appropriate record
+        switch (cellType) {
-        if(record instanceof FormulaRecordAggregate) {
+            default:
-        	(( FormulaRecordAggregate ) record).getFormulaRecord().setValue(value);
+                setCellType(CELL_TYPE_NUMERIC, false, row, col, styleIndex);
-        } else {
+            case CELL_TYPE_ERROR:
                 (( NumberRecord ) record).setValue(value);
+                break;
+            case CELL_TYPE_FORMULA:
+                ((FormulaRecordAggregate)record).getFormulaRecord().setValue(value);
+                break;
         }
     }
 
@@ -542,20 +546,9 @@ public final class HSSFCell {
             // Set the 'pre-evaluated result' for the formula
             // note - formulas do not preserve text formatting.
             FormulaRecordAggregate fr = (FormulaRecordAggregate) record;
-            
+            fr.setCachedStringResult(value.getString());
-            // Save the string into a String Record, creating
-            //  one if required
-            StringRecord sr = fr.getStringRecord();
-            if(sr == null) {
-            	// Wasn't a string before, need a new one
-            	sr = new StringRecord();
-                fr.setStringRecord(sr);
-            }
-            
-            // Save, loosing the formatting
-            sr.setString(value.getString());
             // Update our local cache to the un-formatted version
-            stringValue = new HSSFRichTextString(sr.getString());
+            stringValue = new HSSFRichTextString(value.getString());
 
             // All done
             return;
@@ -599,11 +592,44 @@ public final class HSSFCell {
         Ptg[] ptgs = FormulaParser.parse(formula, book);
         frec.setParsedExpression(ptgs);
     }
+    /* package */ void setFormulaOnly(Ptg[] ptgs) {
+        if (ptgs == null) {
+            throw new IllegalArgumentException("ptgs must not be null");
+        }
+        ((FormulaRecordAggregate)record).getFormulaRecord().setParsedExpression(ptgs);
+    }
 
     public String getCellFormula() {
         return FormulaParser.toFormulaString(book, ((FormulaRecordAggregate)record).getFormulaRecord().getParsedExpression());
     }
 
+    /**
+     * Used to help format error messages
+     */
+    private static String getCellTypeName(int cellTypeCode) {
+        switch (cellTypeCode) {
+            case CELL_TYPE_BLANK:   return "blank";
+            case CELL_TYPE_STRING:  return "text";
+            case CELL_TYPE_BOOLEAN: return "boolean";
+            case CELL_TYPE_ERROR:   return "error";
+            case CELL_TYPE_NUMERIC: return "numeric";
+            case CELL_TYPE_FORMULA: return "formula";
+        }
+        return "#unknown cell type (" + cellTypeCode + ")#";
+    }
+
+    private static RuntimeException typeMismatch(int expectedTypeCode, int actualTypeCode, boolean isFormulaCell) {
+        String msg = "Cannot get a "
+            + getCellTypeName(expectedTypeCode) + " value from a "
+            + getCellTypeName(actualTypeCode) + " " + (isFormulaCell ? "formula " : "") + "cell";
+        return new IllegalStateException(msg);
+    }
+    private static void checkFormulaCachedValueType(int expectedTypeCode, FormulaRecord fr) {
+        int cachedValueType = fr.getCachedResultType();
+        if (cachedValueType != expectedTypeCode) {
+            throw typeMismatch(expectedTypeCode, cachedValueType, true);
+        }
+    }
 
     /**
      * Get the value of the cell as a number.
@@ -613,36 +639,21 @@ public final class HSSFCell {
      *  number into a string similar to that which
      *  Excel would render this number as.
      */
-    public double getNumericCellValue()
+    public double getNumericCellValue() {
-    {
+
-        if (cellType == CELL_TYPE_BLANK)
+        switch(cellType) {
-        {
+            case CELL_TYPE_BLANK:
-            return 0;
+                return 0.0;
-        }
+            case CELL_TYPE_NUMERIC:
-        if (cellType == CELL_TYPE_STRING)
-        {
-            throw new NumberFormatException(
-                "You cannot get a numeric value from a String based cell");
-        }
-        if (cellType == CELL_TYPE_BOOLEAN)
-        {
-            throw new NumberFormatException(
-                "You cannot get a numeric value from a boolean cell");
-        }
-        if (cellType == CELL_TYPE_ERROR)
-        {
-            throw new NumberFormatException(
-                "You cannot get a numeric value from an error cell");
-        }
-        if(cellType == CELL_TYPE_NUMERIC)
-        {
                 return ((NumberRecord)record).getValue();
+            default:
+                throw typeMismatch(CELL_TYPE_NUMERIC, cellType, false);
+            case CELL_TYPE_FORMULA:
+                break;
         }
-        if(cellType == CELL_TYPE_FORMULA)
+        FormulaRecord fr = ((FormulaRecordAggregate)record).getFormulaRecord();
-        {
+        checkFormulaCachedValueType(CELL_TYPE_NUMERIC, fr);
-          return ((FormulaRecordAggregate)record).getFormulaRecord().getValue();
+        return fr.getValue();
-        }
-        throw new NumberFormatException("Unknown Record Type in Cell:"+cellType);
     }
 
     /**
@@ -652,35 +663,17 @@ public final class HSSFCell {
      * See {@link HSSFDataFormatter} for formatting
      *  this date into a string similar to how excel does.
      */
-    public Date getDateCellValue()
+    public Date getDateCellValue() {
-    {
+
-        if (cellType == CELL_TYPE_BLANK)
+        if (cellType == CELL_TYPE_BLANK) {
-        {
             return null;
         }
-        if (cellType == CELL_TYPE_STRING)
+        double value = getNumericCellValue();
-        {
-            throw new NumberFormatException(
-                "You cannot get a date value from a String based cell");
-        }
-        if (cellType == CELL_TYPE_BOOLEAN)
-        {
-            throw new NumberFormatException(
-                "You cannot get a date value from a boolean cell");
-        }
-        if (cellType == CELL_TYPE_ERROR)
-        {
-            throw new NumberFormatException(
-                "You cannot get a date value from an error cell");
-        }
-        double value=this.getNumericCellValue();
         if (book.getWorkbook().isUsing1904DateWindowing()) {
             return HSSFDateUtil.getJavaDate(value, true);
         }
-        else {
         return HSSFDateUtil.getJavaDate(value, false);
     }
-    }
 
     /**
      * get the value of the cell as a string - for numeric cells we throw an exception.
@@ -700,33 +693,22 @@ public final class HSSFCell {
      * For blank cells we return an empty string.
      * For formulaCells that are not string Formulas, we return empty String
      */
+    public HSSFRichTextString getRichStringCellValue() {
 
-    public HSSFRichTextString getRichStringCellValue()
+        switch(cellType) {
-    {
+            case CELL_TYPE_BLANK:
-        if (cellType == CELL_TYPE_BLANK)
-        {
                 return new HSSFRichTextString("");
-        }
+            case CELL_TYPE_STRING:
-        if (cellType == CELL_TYPE_NUMERIC)
-        {
-            throw new NumberFormatException(
-                "You cannot get a string value from a numeric cell");
-        }
-        if (cellType == CELL_TYPE_BOOLEAN)
-        {
-            throw new NumberFormatException(
-                "You cannot get a string value from a boolean cell");
-        }
-        if (cellType == CELL_TYPE_ERROR)
-        {
-            throw new NumberFormatException(
-                "You cannot get a string value from an error cell");
-        }
-        if (cellType == CELL_TYPE_FORMULA) 
-        {
-            if (stringValue==null) return new HSSFRichTextString("");
-        }
                 return stringValue;
+            default:
+                throw typeMismatch(CELL_TYPE_STRING, cellType, false);
+            case CELL_TYPE_FORMULA:
+                break;
+        }
+        FormulaRecordAggregate fra = ((FormulaRecordAggregate)record);
+        checkFormulaCachedValueType(CELL_TYPE_STRING, fra.getFormulaRecord());
+        String strVal = fra.getStringValue();
+        return new HSSFRichTextString(strVal == null ? "" : strVal);
     }
 
     /**
@@ -737,36 +719,45 @@ public final class HSSFCell {
      *        will change the cell to a boolean cell and set its value.
      */
 
-    public void setCellValue(boolean value)
+    public void setCellValue(boolean value) {
-    {
         int row=record.getRow();
         short col=record.getColumn();
         short styleIndex=record.getXFIndex();
-        if ((cellType != CELL_TYPE_BOOLEAN ) && ( cellType != CELL_TYPE_FORMULA))
+
-        {
+        switch (cellType) {
+            default:
                 setCellType(CELL_TYPE_BOOLEAN, false, row, col, styleIndex);
-        }
+            case CELL_TYPE_ERROR:
                 (( BoolErrRecord ) record).setValue(value);
+                break;
+            case CELL_TYPE_FORMULA:
+                ((FormulaRecordAggregate)record).getFormulaRecord().setCachedResultBoolean(value);
+                break;
+        }
     }
 
     /**
      * set a error value for the cell
      *
-     * @param value the error value to set this cell to.  For formulas we'll set the
+     * @param errorCode the error value to set this cell to.  For formulas we'll set the
-     *        precalculated value ??? IS THIS RIGHT??? , for errors we'll set
+     *        precalculated value , for errors we'll set
      *        its value. For other types we will change the cell to an error
      *        cell and set its value.
      */
-
+    public void setCellErrorValue(byte errorCode) {
-    public void setCellErrorValue(byte value)
-    {
         int row=record.getRow();
         short col=record.getColumn();
         short styleIndex=record.getXFIndex();
-        if (cellType != CELL_TYPE_ERROR) {
+        switch (cellType) {
+            default:
                 setCellType(CELL_TYPE_ERROR, false, row, col, styleIndex);
+            case CELL_TYPE_ERROR:
+                (( BoolErrRecord ) record).setValue(errorCode);
+                break;
+            case CELL_TYPE_FORMULA:
+                ((FormulaRecordAggregate)record).getFormulaRecord().setCachedResultErrorCode(errorCode);
+                break;
         }
-        (( BoolErrRecord ) record).setValue(value);
     }
     /**
      * Chooses a new boolean value for the cell when its type is changing.<p/>
@@ -801,38 +792,39 @@ public final class HSSFCell {
      * get the value of the cell as a boolean.  For strings, numbers, and errors, we throw an exception.
      * For blank cells we return a false.
      */
+    public boolean getBooleanCellValue() {
 
-    public boolean getBooleanCellValue()
+        switch(cellType) {
-    {
+            case CELL_TYPE_BLANK:
-        if (cellType == CELL_TYPE_BOOLEAN)
-        {
-            return (( BoolErrRecord ) record).getBooleanValue();
-        }
-        if (cellType == CELL_TYPE_BLANK)
-        {
                 return false;
+            case CELL_TYPE_BOOLEAN:
+                return (( BoolErrRecord ) record).getBooleanValue();
+            default:
+                throw typeMismatch(CELL_TYPE_BOOLEAN, cellType, false);
+            case CELL_TYPE_FORMULA:
+                break;
         }
-        throw new NumberFormatException(
+        FormulaRecord fr = ((FormulaRecordAggregate)record).getFormulaRecord();
-            "You cannot get a boolean value from a non-boolean cell");
+        checkFormulaCachedValueType(CELL_TYPE_BOOLEAN, fr);
+        return fr.getCachedBooleanValue();
     }
 
     /**
      * get the value of the cell as an error code.  For strings, numbers, and booleans, we throw an exception.
      * For blank cells we return a 0.
      */
-
+    public byte getErrorCellValue() {
-    public byte getErrorCellValue()
+        switch(cellType) {
-    {
+            case CELL_TYPE_ERROR:
-        if (cellType == CELL_TYPE_ERROR)
-        {
                 return (( BoolErrRecord ) record).getErrorValue();
+            default:
+                throw typeMismatch(CELL_TYPE_ERROR, cellType, false);
+            case CELL_TYPE_FORMULA:
+                break;
         }
-        if (cellType == CELL_TYPE_BLANK)
+        FormulaRecord fr = ((FormulaRecordAggregate)record).getFormulaRecord();
-        {
+        checkFormulaCachedValueType(CELL_TYPE_ERROR, fr);
-            return ( byte ) 0;
+        return (byte) fr.getCachedErrorValue();
-        }
-        throw new NumberFormatException(
-            "You cannot get an error value from a non-error cell");
     }
 
     /**
@@ -983,7 +975,8 @@ public final class HSSFCell {
         }
 
         // Zap the underlying NoteRecord
-    	sheet.getRecords().remove(comment.getNoteRecord());
+        List sheetRecords = sheet.getRecords();
+        sheetRecords.remove(comment.getNoteRecord());
 
         // If we have a TextObjectRecord, is should
         //  be proceeed by:
@@ -992,21 +985,21 @@ public final class HSSFCell {
         // MSODRAWING with EscherTextboxRecord
         if(comment.getTextObjectRecord() != null) {
             TextObjectRecord txo = comment.getTextObjectRecord();
-    		int txoAt = sheet.getRecords().indexOf(txo);
+            int txoAt = sheetRecords.indexOf(txo);
 
-    		if(sheet.getRecords().get(txoAt-3) instanceof DrawingRecord &&
+            if(sheetRecords.get(txoAt-3) instanceof DrawingRecord &&
-    			sheet.getRecords().get(txoAt-2) instanceof ObjRecord &&
+                sheetRecords.get(txoAt-2) instanceof ObjRecord &&
-    			sheet.getRecords().get(txoAt-1) instanceof DrawingRecord) {
+                sheetRecords.get(txoAt-1) instanceof DrawingRecord) {
                 // Zap these, in reverse order
-    			sheet.getRecords().remove(txoAt-1);
+                sheetRecords.remove(txoAt-1);
-    			sheet.getRecords().remove(txoAt-2);
+                sheetRecords.remove(txoAt-2);
-    			sheet.getRecords().remove(txoAt-3);
+                sheetRecords.remove(txoAt-3);
             } else {
                 throw new IllegalStateException("Found the wrong records before the TextObjectRecord, can't remove comment");
             }
 
             // Now remove the text record
-    		sheet.getRecords().remove(txo);
+            sheetRecords.remove(txo);
         }
     }
 
@@ -1100,4 +1093,16 @@ public final class HSSFCell {
         int eofLoc = sheet.findFirstRecordLocBySid( EOFRecord.sid );
         sheet.getRecords().add( eofLoc, link.record );
     }
+    /**
+     * Only valid for formula cells
+     * @return one of ({@link #CELL_TYPE_NUMERIC}, {@link #CELL_TYPE_STRING},
+     *     {@link #CELL_TYPE_BOOLEAN}, {@link #CELL_TYPE_ERROR}) depending
+     * on the cached value of the formula
+     */
+    public int getCachedFormulaResultType() {
+        if (this.cellType != CELL_TYPE_FORMULA) {
+            throw new IllegalStateException("Only formula cells have cached results");
+        }
+        return ((FormulaRecordAggregate)record).getFormulaRecord().getCachedResultType();
+    }
 }

src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java

@@ -1295,9 +1295,7 @@ public final class HSSFSheet {
                     // If any references were changed, then
                     //  re-create the formula string
                     if(changed) {
-                        c.setCellFormula(
+                        c.setFormulaOnly(ptgs);
-                             FormulaParser.toFormulaString(workbook, ptgs)
-                        );
                     }
                 }
             }

src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java

@@ -660,6 +660,16 @@ public class HSSFWorkbook extends POIDocument
         return -1;
     }
 
+    /* package */ int findSheetIndex(Sheet sheet) {
+        for(int i=0; i<_sheets.size(); i++) {
+            HSSFSheet hSheet = (HSSFSheet) _sheets.get(i);
+            if(hSheet.getSheet() == sheet) {
+                return i;
+            }
+        }
+        throw new IllegalArgumentException("Specified sheet not found in this workbook");
+    }
+
     /**
      * Returns the external sheet index of the sheet
      *  with the given internal index, creating one

src/testcases/org/apache/poi/hssf/model/TestSheet.java

@@ -214,7 +214,9 @@ public final class TestSheet extends TestCase {
         records.add(new DimensionsRecord());
         records.add(new RowRecord(0));
         records.add(new RowRecord(1));
-        records.add(new FormulaRecord());
+        FormulaRecord formulaRecord = new FormulaRecord();
+        formulaRecord.setCachedResultTypeString();
+		records.add(formulaRecord);
         records.add(new StringRecord());
         records.add(new RowRecord(2));
         records.add(createWindow2Record());

src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java

@@ -26,6 +26,8 @@ import org.apache.poi.hssf.record.formula.FuncVarPtg;
 import org.apache.poi.hssf.record.formula.IntPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.record.formula.RefPtg;
+import org.apache.poi.hssf.usermodel.HSSFCell;
+import org.apache.poi.hssf.usermodel.HSSFErrorConstants;
 
 /**
  * Tests the serialization and deserialization of the FormulaRecord
@@ -51,25 +53,39 @@ public final class TestFormulaRecord extends TestCase {
 	 * This formula record is a representation of =1/0 at row 0, column 0
 	 */
 	public void testCheckNanPreserve() {
-		byte[] formulaByte = new byte[29];
+		byte[] formulaByte = {
+			0, 0, 0, 0,
+			0x0F, 0x00,
 
-		formulaByte[4] = (byte)0x0F;
+			// 8 bytes cached number is a 'special value' in this case
-		formulaByte[6] = (byte)0x02;
+			0x02, // special cached value type 'error'
-		formulaByte[8] = (byte)0x07;
+			0x00,
-		formulaByte[12] = (byte)0xFF;
+			HSSFErrorConstants.ERROR_DIV_0,
-		formulaByte[13] = (byte)0xFF;
+			0x00,
-		formulaByte[18] = (byte)0xE0;
+			0x00,
-		formulaByte[19] = (byte)0xFC;
+			0x00,
-		formulaByte[20] = (byte)0x07;
+			(byte)0xFF,
-		formulaByte[22] = (byte)0x1E;
+			(byte)0xFF,
-		formulaByte[23] = (byte)0x01;
+
-		formulaByte[25] = (byte)0x1E;
+			0x00,
-		formulaByte[28] = (byte)0x06;
+			0x00,
+			0x00,
+			0x00,
+
+			(byte)0xE0, //18
+			(byte)0xFC,
+			// Ptgs
+			0x07, 0x00, // encoded length
+			0x1E, 0x01, 0x00, // IntPtg(1)
+			0x1E, 0x00,	0x00, // IntPtg(0)
+			0x06, // DividePtg
+
+		};
 
 		FormulaRecord record = new FormulaRecord(new TestcaseRecordInputStream(FormulaRecord.sid, (short)29, formulaByte));
 		assertEquals("Row", 0, record.getRow());
 		assertEquals("Column", 0, record.getColumn());
-		assertTrue("Value is not NaN", Double.isNaN(record.getValue()));
+		assertEquals(HSSFCell.CELL_TYPE_ERROR, record.getCachedResultType());
 
 		byte[] output = record.serialize();
 		assertEquals("Output size", 33, output.length); //includes sid+recordlength

src/testcases/org/apache/poi/hssf/record/aggregates/TestFormulaRecordAggregate.java

@@ -30,6 +30,7 @@ public final class TestFormulaRecordAggregate extends TestCase {
     
     public void testBasic() throws Exception {
         FormulaRecord f = new FormulaRecord();
+        f.setCachedResultTypeString();
         StringRecord s = new StringRecord();
         s.setString("abc");
         FormulaRecordAggregate fagg = new FormulaRecordAggregate(f, s, SharedValueManager.EMPTY);

src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java

@@ -1166,18 +1166,20 @@ public final class TestBugs extends TestCase {
         assertEquals("\"70164\"", c2.getCellFormula());
 
         // And check the values - blank
-        assertEquals(0.0, c1.getNumericCellValue(), 0.00001);
+        confirmCachedValue(0.0, c1);
-        assertEquals("", c1.getRichStringCellValue().getString());
+        confirmCachedValue(0.0, c2);
-        assertEquals(0.0, c2.getNumericCellValue(), 0.00001);
+        confirmCachedValue(0.0, c3);
-        assertEquals("", c2.getRichStringCellValue().getString());
-        assertEquals(0.0, c3.getNumericCellValue(), 0.00001);
-        assertEquals("", c3.getRichStringCellValue().getString());
 
         // Try changing the cached value on one of the string
         //  formula cells, so we can see it updates properly
         c3.setCellValue(new HSSFRichTextString("test"));
-        assertEquals(0.0, c3.getNumericCellValue(), 0.00001);
+        confirmCachedValue("test", c3);
-        assertEquals("test", c3.getRichStringCellValue().getString());
+        try {
+            c3.getNumericCellValue();
+            throw new AssertionFailedError("exception should have been thrown");
+        } catch (IllegalStateException e) {
+            assertEquals("Cannot get a numeric value from a text formula cell", e.getMessage());
+        }
 
 
         // Now evaluate, they should all be changed
@@ -1188,12 +1190,9 @@ public final class TestBugs extends TestCase {
 
         // Check that the cells now contain
         //  the correct values
-        assertEquals(70164.0, c1.getNumericCellValue(), 0.00001);
+        confirmCachedValue(70164.0, c1);
-        assertEquals("", c1.getRichStringCellValue().getString());
+        confirmCachedValue("70164", c2);
-        assertEquals(0.0, c2.getNumericCellValue(), 0.00001);
+        confirmCachedValue("90210", c3);
-        assertEquals("70164", c2.getRichStringCellValue().getString());
-        assertEquals(0.0, c3.getNumericCellValue(), 0.00001);
-        assertEquals("90210", c3.getRichStringCellValue().getString());
 
 
         // Write and read
@@ -1204,12 +1203,9 @@ public final class TestBugs extends TestCase {
         HSSFCell nc3 = ns.getRow(0).getCell(2);
 
         // Re-check
-        assertEquals(70164.0, nc1.getNumericCellValue(), 0.00001);
+        confirmCachedValue(70164.0, nc1);
-        assertEquals("", nc1.getRichStringCellValue().getString());
+        confirmCachedValue("70164", nc2);
-        assertEquals(0.0, nc2.getNumericCellValue(), 0.00001);
+        confirmCachedValue("90210", nc3);
-        assertEquals("70164", nc2.getRichStringCellValue().getString());
-        assertEquals(0.0, nc3.getNumericCellValue(), 0.00001);
-        assertEquals("90210", nc3.getRichStringCellValue().getString());
 
         CellValueRecordInterface[] cvrs = ns.getSheet().getValueRecords();
         for (int i = 0; i < cvrs.length; i++) {
@@ -1234,6 +1230,17 @@ public final class TestBugs extends TestCase {
         assertEquals(3, cvrs.length);
     }
 
+    private static void confirmCachedValue(double expectedValue, HSSFCell cell) {
+        assertEquals(HSSFCell.CELL_TYPE_FORMULA, cell.getCellType());
+        assertEquals(HSSFCell.CELL_TYPE_NUMERIC, cell.getCachedFormulaResultType());
+        assertEquals(expectedValue, cell.getNumericCellValue(), 0.0);
+    }
+    private static void confirmCachedValue(String expectedValue, HSSFCell cell) {
+        assertEquals(HSSFCell.CELL_TYPE_FORMULA, cell.getCellType());
+        assertEquals(HSSFCell.CELL_TYPE_STRING, cell.getCachedFormulaResultType());
+        assertEquals(expectedValue, cell.getRichStringCellValue().getString());
+    }
+
     /**
      * Problem with "Vector Rows", eg a whole
      *  column which is set to the result of