Merged revisions 638786-638802,638805-638811,638813-638814,638816-639230,639233-639241,639243-639253,639255-639486,639488-639601,639603-639835,639837-639917,639919-640056,640058-640710,640712-641156,641158-641184,641186-641795,641797-641798,641800-641933,641935-641963,641965-641966,641968-641995,641997-642230,642232-642562,642564-642565,642568-642570,642572-642573,642576-642736,642739-642877,642879,642881-642890,642892-642903,642905-642945,642947-643624,643626-643653,643655-643669,643671,643673-643830,643832-643833,643835-644342,644344-644472,644474-644508,644510-645347,645349-645351,645353-645559,645561-645565,645568-645951,645953-646193,646195-646311,646313-646404,646406-646665,646667-646853,646855-646869,646871-647151,647153-647185,647187-647277,647279-647566,647568-647573,647575,647578-647711,647714-647737,647739-647823,647825-648155,648157-648202,648204-648273,648275,648277-648302,648304-648333,648335-648588,648590-648622,648625-648673,648675-649141,649144,649146-649556,649558-649795,649799,649801-649910,649912-649913,649915-650128,650131-650132,650134-650137,650140-650914,650916-651991,651993-652284,652286-652287,652289,652291,652293-652297,652299-652328,652330-652425,652427-652445,652447-652560,652562-652933,652935,652937-652993,652995-653116,653118-653124,653126-653483,653487-653519,653522-653550,653552-653607,653609-653667,653669-653674,653676-653814,653817-653830,653832-653891,653893-653944,653946-654055,654057-654355,654357-654365,654367-654648,654651-655215,655217-655277,655279-655281,655283-655911,655913-656212,656214,656216-656251,656253-656698,656700-656756,656758-656892,656894-657135,657137-657165,657168-657179,657181-657354,657356-657357,657359-657701,657703-657874,657876-658032,658034-658284,658286,658288-658301,658303-658307,658309-658321,658323-658335,658337-658348,658351,658353-658832,658834-658983,658985,658987-659066,659068-659402,659404-659428,659430-659451,659453-659454,659456-659461,659463-659477,659479-659524,659526-659571,659574-660890 via svnmerge from

https://svn.apache.org/repos/asf/poi/trunk

........
  r659575 | nick | 2008-05-23 16:55:08 +0100 (Fri, 23 May 2008) | 1 line
  
  Help for bug #44840 - Improved handling of HSSFObjectData, especially for entries with data held not in POIFS
........
  r660256 | josh | 2008-05-26 19:02:23 +0100 (Mon, 26 May 2008) | 1 line
  
  Follow-on fix for bug 42564 (r653668). Array elements are stored internally column by column.
........
  r660263 | josh | 2008-05-26 19:25:02 +0100 (Mon, 26 May 2008) | 1 line
  
  Small fix for FormulaParser. Need case-insentive match for IF function name
........
  r660280 | josh | 2008-05-26 20:36:56 +0100 (Mon, 26 May 2008) | 1 line
  
  Added test cases for parsing IF expressions.  Segregated IF test cases into a new class
........
  r660344 | josh | 2008-05-27 01:57:23 +0100 (Tue, 27 May 2008) | 1 line
  
  Changed class hierarchy of Ptg to improve 'operand class' transformation.
........
  r660474 | nick | 2008-05-27 12:44:49 +0100 (Tue, 27 May 2008) | 1 line
  
  X, Y, Width and Height getters/setters on HSSFChart
........
  r660828 | josh | 2008-05-28 07:19:31 +0100 (Wed, 28 May 2008) | 1 line
  
  Fix for 45060 (and 45041) - Improved token class transformation during formula parsing
........
  r660834 | yegor | 2008-05-28 07:50:35 +0100 (Wed, 28 May 2008) | 1 line
  
  bump 3.1-beta2 announcement
........
  r660889 | nick | 2008-05-28 11:03:00 +0100 (Wed, 28 May 2008) | 1 line
  
  Fix bug #45087 - Correctly detect date formats like [Black]YYYY as being date based
........


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

src/documentation/content/xdocs/changes.xml

@@ -46,6 +46,9 @@
            <action dev="POI-DEVELOPERS" type="add">Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx</action>
         </release>
         <release version="3.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats like [Black]YYYY as being date based</action>
+           <action dev="POI-DEVELOPERS" type="add">45060 - Improved token class transformation during formula parsing</action>
+           <action dev="POI-DEVELOPERS" type="add">44840 - Improved handling of HSSFObjectData, especially for entries with data held not in POIFS</action>
            <action dev="POI-DEVELOPERS" type="add">45043 - Support for getting excel cell comments when extracting text</action>
            <action dev="POI-DEVELOPERS" type="add">Extend the support for specifying a policy to HSSF on missing / blank cells when fetching, to be able to specify the policy at the HSSFWorkbook level</action>
            <action dev="POI-DEVELOPERS" type="fix">45025 - improved FormulaParser parse error messages</action>

src/documentation/content/xdocs/index.xml

@@ -40,9 +40,9 @@
        People interested should follow the
        <link href="mailinglists.html">dev list</link> to track progress.</p>
     </section>
-    <section><title>POI 3.1-BETA1 Released (2008-04028)</title>
+    <section><title>POI 3.1-BETA2 Released (2008-05-28)</title>
       <p>
-        The POI team is pleased to announce the release of 3.1 BETA1 which is one of the final steps before 3.1 FINAL.
+        The POI team is pleased to announce the release of 3.1 BETA2 which is one of the final steps before 3.1 FINAL.
         The status of this release is a beta, meaning that we encourage users to try it out.
         If you find any bugs, please report them to the POI <link href="https://issues.apache.org/bugzilla/buglist.cgi?product=POI">bug database</link> or to
         the <link href="./mailinglists.html">POI Developer List</link>.
@@ -54,7 +54,7 @@
       </p>
       <p>
         The release is also available from the central Maven repository 
-        under Group ID "org.apache.poi" and Version "3.1-beta1".
+        under Group ID "org.apache.poi" and Version "3.1-beta2".
       </p>  
     </section>
     <section><title>POI 3.0.2 Released</title>

src/documentation/content/xdocs/status.xml

@@ -43,6 +43,9 @@
            <action dev="POI-DEVELOPERS" type="add">Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx</action>
         </release>
         <release version="3.1-final" date="2008-06-??">
+           <action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats like [Black]YYYY as being date based</action>
+           <action dev="POI-DEVELOPERS" type="add">45060 - Improved token class transformation during formula parsing</action>
+           <action dev="POI-DEVELOPERS" type="add">44840 - Improved handling of HSSFObjectData, especially for entries with data held not in POIFS</action>
            <action dev="POI-DEVELOPERS" type="add">45043 - Support for getting excel cell comments when extracting text</action>
            <action dev="POI-DEVELOPERS" type="add">Extend the support for specifying a policy to HSSF on missing / blank cells when fetching, to be able to specify the policy at the HSSFWorkbook level</action>
            <action dev="POI-DEVELOPERS" type="fix">45025 - improved FormulaParser parse error messages</action>

src/java/org/apache/poi/hssf/model/FormulaParser.java

@@ -18,7 +18,6 @@
 package org.apache.poi.hssf.model;
 
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Stack;
 import java.util.regex.Pattern;
@@ -61,17 +60,17 @@ public final class FormulaParser {
         }
     }
 
-    public static int FORMULA_TYPE_CELL = 0;
-    public static int FORMULA_TYPE_SHARED = 1;
-    public static int FORMULA_TYPE_ARRAY =2;
-    public static int FORMULA_TYPE_CONDFOMRAT = 3;
-    public static int FORMULA_TYPE_NAMEDRANGE = 4;
+    public static final int FORMULA_TYPE_CELL = 0;
+    public static final int FORMULA_TYPE_SHARED = 1;
+    public static final int FORMULA_TYPE_ARRAY =2;
+    public static final int FORMULA_TYPE_CONDFOMRAT = 3;
+    public static final int FORMULA_TYPE_NAMEDRANGE = 4;
 
     private final String formulaString;
     private final int formulaLength;
     private int pointer;
 
-    private final List tokens = new Stack();
+    private ParseNode _rootNode;
 
     /**
      * Used for spotting if we have a cell reference,
@@ -221,14 +220,15 @@ public final class FormulaParser {
         return value.length() == 0 ? null : value.toString();
     }
 
-    /** Parse and Translate a String Identifier */
-    private Ptg parseIdent() {
-        String name;
-        name = GetName();
+    private ParseNode parseFunctionOrIdentifier() {
+        String name = GetName();
         if (look == '('){
             //This is a function
             return function(name);
         }
+        return new ParseNode(parseIdentifier(name));
+    }
+    private Ptg parseIdentifier(String name) {
 
         if (look == ':' || look == '.') { // this is a AreaReference
             GetChar();
@@ -287,14 +287,6 @@ public final class FormulaParser {
                     + name + "\", but that named range wasn't defined!");
     }
 
-    /**
-     * Adds a pointer to the last token to the latest function argument list.
-     * @param obj
-     */
-    private void addArgumentPointer(List argumentPointers) {
-        argumentPointers.add(tokens.get(tokens.size()-1));
-    }
-
     /**
      * Note - Excel function names are 'case aware but not case sensitive'.  This method may end
      * up creating a defined name record in the workbook if the specified name is not an internal
@@ -302,58 +294,23 @@ public final class FormulaParser {
      *
      * @param name case preserved function name (as it was entered/appeared in the formula).
      */
-    private Ptg function(String name) {
-        int numArgs =0 ;
+    private ParseNode function(String name) {
+        NamePtg nameToken = null;
         // Note regarding parameter -
         if(!AbstractFunctionPtg.isInternalFunctionName(name)) {
             // external functions get a Name token which points to a defined name record
-            NamePtg nameToken = new NamePtg(name, this.book);
+            nameToken = new NamePtg(name, this.book);
 
             // in the token tree, the name is more or less the first argument
-            numArgs++;
-            tokens.add(nameToken);
         }
-        //average 2 args per function
-        List argumentPointers = new ArrayList(2);
 
         Match('(');
-        numArgs += Arguments(argumentPointers);
+        ParseNode[] args = Arguments();
         Match(')');
 
-        return getFunction(name, numArgs, argumentPointers);
-    }
-
-    /**
-     * Adds the size of all the ptgs after the provided index (inclusive).
-     * <p>
-     * Initially used to count a goto
-     * @param index
-     * @return int
-     */
-    private int getPtgSize(int index) {
-        int count = 0;
-
-        Iterator ptgIterator = tokens.listIterator(index);
-        while (ptgIterator.hasNext()) {
-            Ptg ptg = (Ptg)ptgIterator.next();
-            count+=ptg.getSize();
-        }
-
-        return count;
+        return getFunction(name, nameToken, args);
     }
 
-    private int getPtgSize(int start, int end) {
-        int count = 0;
-        int index = start;
-        Iterator ptgIterator = tokens.listIterator(index);
-        while (ptgIterator.hasNext() && index <= end) {
-            Ptg ptg = (Ptg)ptgIterator.next();
-            count+=ptg.getSize();
-            index++;
-        }
-
-        return count;
-    }
     /**
      * Generates the variable function ptg for the formula.
      * <p>
@@ -362,84 +319,35 @@ public final class FormulaParser {
      * @param numArgs
      * @return Ptg a null is returned if we're in an IF formula, it needs extreme manipulation and is handled in this function
      */
-    private AbstractFunctionPtg getFunction(String name, int numArgs, List argumentPointers) {
+    private ParseNode getFunction(String name, NamePtg namePtg, ParseNode[] args) {
 
-        boolean isVarArgs;
-        int funcIx;
         FunctionMetadata fm = FunctionMetadataRegistry.getFunctionByName(name.toUpperCase());
+        int numArgs = args.length;
         if(fm == null) {
+        	if (namePtg == null) {
+        		throw new IllegalStateException("NamePtg must be supplied for external functions");
+        	}
             // must be external function
-            isVarArgs = true;
-            funcIx = FunctionMetadataRegistry.FUNCTION_INDEX_EXTERNAL;
-        } else {
-            isVarArgs = !fm.hasFixedArgsLength();
-            funcIx = fm.getIndex();
-            validateNumArgs(numArgs, fm);
+            ParseNode[] allArgs = new ParseNode[numArgs+1];
+            allArgs[0] = new ParseNode(namePtg);
+            System.arraycopy(args, 0, allArgs, 1, numArgs);
+            return new ParseNode(new FuncVarPtg(name, (byte)(numArgs+1)), allArgs);
         }
+
+        if (namePtg != null) {
+    		throw new IllegalStateException("NamePtg no applicable to internal functions");
+    	}
+        boolean isVarArgs = !fm.hasFixedArgsLength();
+        int funcIx = fm.getIndex();
+        validateNumArgs(args.length, fm);
+
         AbstractFunctionPtg retval;
         if(isVarArgs) {
             retval = new FuncVarPtg(name, (byte)numArgs);
         } else {
             retval = new FuncPtg(funcIx);
         }
-        if (!name.equals(AbstractFunctionPtg.FUNCTION_NAME_IF)) {
-            // early return for everything else besides IF()
-            return retval;
-        }
-
-
-        AttrPtg ifPtg = new AttrPtg();
-        ifPtg.setData((short)7); //mirroring excel output
-        ifPtg.setOptimizedIf(true);
-
-        if (argumentPointers.size() != 2  && argumentPointers.size() != 3) {
-            throw new IllegalArgumentException("["+argumentPointers.size()+"] Arguments Found - An IF formula requires 2 or 3 arguments. IF(CONDITION, TRUE_VALUE, FALSE_VALUE [OPTIONAL]");
-        }
-
-        //Biffview of an IF formula record indicates the attr ptg goes after the condition ptgs and are
-        //tracked in the argument pointers
-        //The beginning first argument pointer is the last ptg of the condition
-        int ifIndex = tokens.indexOf(argumentPointers.get(0))+1;
-        tokens.add(ifIndex, ifPtg);
-
-        //we now need a goto ptgAttr to skip to the end of the formula after a true condition
-        //the true condition is should be inserted after the last ptg in the first argument
-
-        int gotoIndex = tokens.indexOf(argumentPointers.get(1))+1;
-
-        AttrPtg goto1Ptg = new AttrPtg();
-        goto1Ptg.setGoto(true);
-
-
-        tokens.add(gotoIndex, goto1Ptg);
-
-
-        if (numArgs > 2) { //only add false jump if there is a false condition
-
-            //second goto to skip past the function ptg
-            AttrPtg goto2Ptg = new AttrPtg();
-            goto2Ptg.setGoto(true);
-            goto2Ptg.setData((short)(retval.getSize()-1));
-            //Page 472 of the Microsoft Excel Developer's kit states that:
-            //The b(or w) field specifies the number byes (or words to skip, minus 1
-
-            tokens.add(goto2Ptg); //this goes after all the arguments are defined
-        }
-
-        //data portion of the if ptg points to the false subexpression (Page 472 of MS Excel Developer's kit)
-        //count the number of bytes after the ifPtg to the False Subexpression
-        //doesn't specify -1 in the documentation
-        ifPtg.setData((short)(getPtgSize(ifIndex+1, gotoIndex)));
-
-        //count all the additional (goto) ptgs but dont count itself
-        int ptgCount = this.getPtgSize(gotoIndex)-goto1Ptg.getSize()+retval.getSize();
-        if (ptgCount > Short.MAX_VALUE) {
-            throw new RuntimeException("Ptg Size exceeds short when being specified for a goto ptg in an if");
-        }
-
-        goto1Ptg.setData((short)(ptgCount-1));
-
-        return retval;
+        return new ParseNode(retval, args);
     }
 
     private void validateNumArgs(int numArgs, FunctionMetadata fm) {
@@ -470,10 +378,12 @@ public final class FormulaParser {
     }
 
     /** get arguments to a function */
-    private int Arguments(List argumentPointers) {
+    private ParseNode[] Arguments() {
+        //average 2 args per function
+        List temp = new ArrayList(2);
         SkipWhite();
         if(look == ')') {
-            return 0;
+            return ParseNode.EMPTY_ARRAY;
         }
 
         boolean missedPrevArg = true;
@@ -482,8 +392,7 @@ public final class FormulaParser {
             SkipWhite();
             if (isArgumentDelimiter(look)) {
                 if (missedPrevArg) {
-                    tokens.add(new MissingArgPtg());
-                    addArgumentPointer(argumentPointers);
+                	temp.add(new ParseNode(new MissingArgPtg()));
                     numArgs++;
                 }
                 if (look == ')') {
@@ -493,8 +402,7 @@ public final class FormulaParser {
                 missedPrevArg = true;
                 continue;
             }
-            comparisonExpression();
-            addArgumentPointer(argumentPointers);
+            temp.add(comparisonExpression());
             numArgs++;
             missedPrevArg = false;
             SkipWhite();
@@ -502,32 +410,34 @@ public final class FormulaParser {
                 throw expected("',' or ')'");
             }
         }
-        return numArgs;
+        ParseNode[] result = new ParseNode[temp.size()];
+        temp.toArray(result);
+        return result;
     }
 
    /** Parse and Translate a Math Factor  */
-    private void powerFactor() {
-        percentFactor();
+    private ParseNode powerFactor() {
+    	ParseNode result = percentFactor();
         while(true) {
             SkipWhite();
             if(look != '^') {
-                return;
+                return result;
             }
             Match('^');
-            percentFactor();
-            tokens.add(new PowerPtg());
+            ParseNode other = percentFactor();
+            result = new ParseNode(new PowerPtg(), result, other);
         }
     }
 
-    private void percentFactor() {
-        tokens.add(parseSimpleFactor());
+    private ParseNode percentFactor() {
+    	ParseNode result = parseSimpleFactor();
         while(true) {
             SkipWhite();
             if(look != '%') {
-                return;
+                return result;
             }
             Match('%');
-            tokens.add(new PercentPtg());
+            result = new ParseNode(new PercentPtg(), result);
         }
     }
 
@@ -535,32 +445,30 @@ public final class FormulaParser {
     /**
      * factors (without ^ or % )
      */
-    private Ptg parseSimpleFactor() {
+    private ParseNode parseSimpleFactor() {
         SkipWhite();
         switch(look) {
             case '#':
-                return parseErrorLiteral();
+                return new ParseNode(parseErrorLiteral());
             case '-':
                 Match('-');
-                powerFactor();
-                return new UnaryMinusPtg();
+                return new ParseNode(new UnaryMinusPtg(), powerFactor());
             case '+':
                 Match('+');
-                powerFactor();
-                return new UnaryPlusPtg();
+                return new ParseNode(new UnaryPlusPtg(), powerFactor());
             case '(':
                 Match('(');
-                comparisonExpression();
+                ParseNode inside = comparisonExpression();
                 Match(')');
-                return new ParenthesisPtg();
+                return new ParseNode(new ParenthesisPtg(), inside);
             case '"':
-                return parseStringLiteral();
+                return new ParseNode(parseStringLiteral());
         }
         if (IsAlpha(look) || look == '\''){
-            return parseIdent();
+            return parseFunctionOrIdentifier();
         }
         // else - assume number
-        return parseNumber();
+        return new ParseNode(parseNumber());
     }
 
 
@@ -716,28 +624,30 @@ public final class FormulaParser {
     }
 
     /** Parse and Translate a Math Term */
-    private void  Term() {
-        powerFactor();
+    private ParseNode  Term() {
+    	ParseNode result = powerFactor();
         while(true) {
             SkipWhite();
+            Ptg operator;
             switch(look) {
                 case '*':
                     Match('*');
-                    powerFactor();
-                    tokens.add(new MultiplyPtg());
-                    continue;
+                    operator = new MultiplyPtg();
+                    break;
                 case '/':
                     Match('/');
-                    powerFactor();
-                    tokens.add(new DividePtg());
-                    continue;
+                    operator = new DividePtg();
+                    break;
+                default:
+                    return result; // finished with Term
             }
-            return; // finished with Term
+            ParseNode other = powerFactor();
+            result = new ParseNode(operator, result, other);
         }
     }
 
-    private void comparisonExpression() {
-        concatExpression();
+    private ParseNode comparisonExpression() {
+    	ParseNode result = concatExpression();
         while (true) {
             SkipWhite();
             switch(look) {
@@ -745,11 +655,11 @@ public final class FormulaParser {
                 case '>':
                 case '<':
                     Ptg comparisonToken = getComparisonToken();
-                    concatExpression();
-                    tokens.add(comparisonToken);
+                    ParseNode other = concatExpression();
+                    result = new ParseNode(comparisonToken, result, other);
                     continue;
             }
-            return; // finished with predicate expression
+            return result; // finished with predicate expression
         }
     }
 
@@ -779,38 +689,41 @@ public final class FormulaParser {
     }
 
 
-    private void concatExpression() {
-        additiveExpression();
+    private ParseNode concatExpression() {
+        ParseNode result = additiveExpression();
         while (true) {
             SkipWhite();
             if(look != '&') {
                 break; // finished with concat expression
             }
             Match('&');
-            additiveExpression();
-            tokens.add(new ConcatPtg());
+            ParseNode other = additiveExpression();
+            result = new ParseNode(new ConcatPtg(), result, other);
         }
+        return result;
     }
 
 
     /** Parse and Translate an Expression */
-    private void additiveExpression() {
-        Term();
+    private ParseNode additiveExpression() {
+    	ParseNode result = Term();
         while (true) {
             SkipWhite();
+            Ptg operator;
             switch(look) {
                 case '+':
                     Match('+');
-                    Term();
-                    tokens.add(new AddPtg());
-                    continue;
+                    operator = new AddPtg();
+                    break;
                 case '-':
                     Match('-');
-                    Term();
-                    tokens.add(new SubtractPtg());
-                    continue;
+                    operator = new SubtractPtg();
+                    break;
+                default:
+                    return result; // finished with additive expression
             }
-            return; // finished with additive expression
+            ParseNode other = Term();
+            result = new ParseNode(operator, result, other);
         }
     }
 
@@ -835,7 +748,7 @@ end;
     public void parse() {
         pointer=0;
         GetChar();
-        comparisonExpression();
+        _rootNode = comparisonExpression();
 
         if(pointer <= formulaLength) {
             String msg = "Unused input [" + formulaString.substring(pointer-1)
@@ -858,87 +771,12 @@ end;
     }
 
     public Ptg[] getRPNPtg(int formulaType) {
-        Node node = createTree();
+    	OperandClassTransformer oct = new OperandClassTransformer(formulaType);
         // RVA is for 'operand class': 'reference', 'value', 'array'
-        setRootLevelRVA(node, formulaType);
-        setParameterRVA(node,formulaType);
-        return (Ptg[]) tokens.toArray(new Ptg[0]);
+    	oct.transformFormula(_rootNode);
+        return ParseNode.toTokenArray(_rootNode);
     }
 
-    private void setRootLevelRVA(Node n, int formulaType) {
-        //Pg 16, excelfileformat.pdf @ openoffice.org
-        Ptg p = n.getValue();
-            if (formulaType == FormulaParser.FORMULA_TYPE_NAMEDRANGE) {
-                if (p.getDefaultOperandClass() == Ptg.CLASS_REF) {
-                    setClass(n,Ptg.CLASS_REF);
-                } else {
-                    setClass(n,Ptg.CLASS_ARRAY);
-                }
-            } else {
-                setClass(n,Ptg.CLASS_VALUE);
-            }
-
-    }
-
-    private void setParameterRVA(Node n, int formulaType) {
-        Ptg p = n.getValue();
-        int numOperands = n.getNumChildren();
-        if (p instanceof AbstractFunctionPtg) {
-            for (int i =0;i<numOperands;i++) {
-                setParameterRVA(n.getChild(i),((AbstractFunctionPtg)p).getParameterClass(i),formulaType);
-//                if (n.getChild(i).getValue() instanceof AbstractFunctionPtg) {
-//                    setParameterRVA(n.getChild(i),formulaType);
-//                }
-                setParameterRVA(n.getChild(i),formulaType);
-            }
-        } else {
-            for (int i =0;i<numOperands;i++) {
-                setParameterRVA(n.getChild(i),formulaType);
-            }
-        }
-    }
-    private void setParameterRVA(Node n, int expectedClass,int formulaType) {
-        Ptg p = n.getValue();
-        if (expectedClass == Ptg.CLASS_REF) { //pg 15, table 1
-            if (p.getDefaultOperandClass() == Ptg.CLASS_REF ) {
-                setClass(n, Ptg.CLASS_REF);
-            }
-            if (p.getDefaultOperandClass() == Ptg.CLASS_VALUE) {
-                if (formulaType==FORMULA_TYPE_CELL || formulaType == FORMULA_TYPE_SHARED) {
-                    setClass(n,Ptg.CLASS_VALUE);
-                } else {
-                    setClass(n,Ptg.CLASS_ARRAY);
-                }
-            }
-            if (p.getDefaultOperandClass() == Ptg.CLASS_ARRAY ) {
-                setClass(n, Ptg.CLASS_ARRAY);
-            }
-        } else if (expectedClass == Ptg.CLASS_VALUE) { //pg 15, table 2
-            if (formulaType == FORMULA_TYPE_NAMEDRANGE) {
-                setClass(n,Ptg.CLASS_ARRAY) ;
-            } else {
-                setClass(n,Ptg.CLASS_VALUE);
-            }
-        } else { //Array class, pg 16.
-            if (p.getDefaultOperandClass() == Ptg.CLASS_VALUE &&
-                 (formulaType==FORMULA_TYPE_CELL || formulaType == FORMULA_TYPE_SHARED)) {
-                 setClass(n,Ptg.CLASS_VALUE);
-            } else {
-                setClass(n,Ptg.CLASS_ARRAY);
-            }
-        }
-    }
-
-     private void setClass(Node n, byte theClass) {
-        Ptg p = n.getValue();
-        if (p instanceof AbstractFunctionPtg || !(p instanceof OperationPtg)) {
-            p.setClass(theClass);
-        } else {
-            for (int i =0;i<n.getNumChildren();i++) {
-                setClass(n.getChild(i),theClass);
-            }
-        }
-     }
     /**
      * Convenience method which takes in a list then passes it to the
      *  other toFormulaString signature.
@@ -988,11 +826,11 @@ end;
                 // TODO - put comment and throw exception in toFormulaString() of these classes
                 continue;
             }
-            if (! (ptg instanceof OperationPtg)) {
-                stack.push(ptg.toFormulaString(book));
+            if (ptg instanceof ParenthesisPtg) {
+                String contents = (String)stack.pop();
+                stack.push ("(" + contents + ")");
                 continue;
             }
-
             if (ptg instanceof AttrPtg) {
                 AttrPtg attrPtg = ((AttrPtg) ptg);
                 if (attrPtg.isOptimizedIf() || attrPtg.isOptimizedChoose() || attrPtg.isGoto()) {
@@ -1009,24 +847,21 @@ end;
                     // similar to tAttrSpace - RPN is violated
                     continue;
                 }
-                if (!attrPtg.isSum()) {
-                    throw new RuntimeException("Unexpected tAttr: " + attrPtg.toString());
+                if (attrPtg.isSum()) {
+                    String[] operands = getOperands(stack, attrPtg.getNumberOfOperands());
+                    stack.push(attrPtg.toFormulaString(operands));
+                    continue;
                 }
+                throw new RuntimeException("Unexpected tAttr: " + attrPtg.toString());
             }
 
-            final OperationPtg o = (OperationPtg) ptg;
-            int nOperands = o.getNumberOfOperands();
-            final String[] operands = new String[nOperands];
-
-            for (int j = nOperands-1; j >= 0; j--) { // reverse iteration because args were pushed in-order
-                if(stack.isEmpty()) {
-                   String msg = "Too few arguments suppled to operation token ("
-                        + o.getClass().getName() + "). Expected (" + nOperands
-                        + ") operands but got (" + (nOperands - j - 1) + ")";
-                    throw new IllegalStateException(msg);
-                }
-                operands[j] = (String) stack.pop();
+            if (! (ptg instanceof OperationPtg)) {
+                stack.push(ptg.toFormulaString(book));
+                continue;
             }
+
+            OperationPtg o = (OperationPtg) ptg;
+            String[] operands = getOperands(stack, o.getNumberOfOperands());
             stack.push(o.toFormulaString(operands));
         }
         if(stack.isEmpty()) {
@@ -1042,6 +877,20 @@ end;
         }
         return result;
     }
+    
+    private static String[] getOperands(Stack stack, int nOperands) {
+        String[] operands = new String[nOperands];
+
+        for (int j = nOperands-1; j >= 0; j--) { // reverse iteration because args were pushed in-order
+            if(stack.isEmpty()) {
+               String msg = "Too few arguments supplied to operation. Expected (" + nOperands
+                    + ") operands but got (" + (nOperands - j - 1) + ")";
+                throw new IllegalStateException(msg);
+            }
+            operands[j] = (String) stack.pop();
+        }
+        return operands;
+    }
     /**
      * Static method to convert an array of Ptgs in RPN order
      *  to a human readable string format in infix mode. Works
@@ -1052,59 +901,4 @@ end;
     public String toFormulaString(Ptg[] ptgs) {
         return toFormulaString(book, ptgs);
     }
-
-
-    /** Create a tree representation of the RPN token array
-     *used to run the class(RVA) change algo
-     */
-    private Node createTree() {
-        Stack stack = new Stack();
-        int numPtgs = tokens.size();
-        OperationPtg o;
-        int numOperands;
-        Node[] operands;
-        for (int i=0;i<numPtgs;i++) {
-            if (tokens.get(i) instanceof OperationPtg) {
-
-                o = (OperationPtg) tokens.get(i);
-                numOperands = o.getNumberOfOperands();
-                operands = new Node[numOperands];
-                for (int j=0;j<numOperands;j++) {
-                    operands[numOperands-j-1] = (Node) stack.pop();
-                }
-                Node result = new Node(o);
-                result.setChildren(operands);
-                stack.push(result);
-            } else {
-                stack.push(new Node((Ptg)tokens.get(i)));
-            }
-        }
-        return (Node) stack.pop();
-    }
-
-    /** toString on the parser instance returns the RPN ordered list of tokens
-     *   Useful for testing
-     */
-    public String toString() {
-        StringBuffer buf = new StringBuffer();
-           for (int i=0;i<tokens.size();i++) {
-            buf.append( ( (Ptg)tokens.get(i)).toFormulaString(book));
-            buf.append(' ');
-        }
-        return buf.toString();
-    }
-
-    /** Private helper class, used to create a tree representation of the formula*/
-    private static final class Node {
-        private Ptg value=null;
-        private Node[] children=new Node[0];
-        private int numChild=0;
-        public Node(Ptg val) {
-            value = val;
-        }
-        public void setChildren(Node[] child) {children = child;numChild=child.length;}
-        public int getNumChildren() {return numChild;}
-        public Node getChild(int number) {return children[number];}
-        public Ptg getValue() {return value;}
-    }
 }

src/java/org/apache/poi/hssf/model/OperandClassTransformer.java

@@ -0,0 +1,206 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.model;
+
+import org.apache.poi.hssf.record.formula.AbstractFunctionPtg;
+import org.apache.poi.hssf.record.formula.ControlPtg;
+import org.apache.poi.hssf.record.formula.ValueOperatorPtg;
+import org.apache.poi.hssf.record.formula.Ptg;
+
+/**
+ * This class performs 'operand class' transformation. Non-base tokens are classified into three 
+ * operand classes:
+ * <ul>
+ * <li>reference</li> 
+ * <li>value</li> 
+ * <li>array</li> 
+ * </ul>
+ * <p/>
+ * 
+ * The final operand class chosen for each token depends on the formula type and the token's place
+ * in the formula. If POI gets the operand class wrong, Excel <em>may</em> interpret the formula
+ * incorrectly.  This condition is typically manifested as a formula cell that displays as '#VALUE!',
+ * but resolves correctly when the user presses F2, enter.<p/>
+ * 
+ * The logic implemented here was partially inspired by the description in
+ * "OpenOffice.org's Documentation of the Microsoft Excel File Format".  The model presented there
+ * seems to be inconsistent with observed Excel behaviour (These differences have not been fully
+ * investigated). The implementation in this class has been heavily modified in order to satisfy
+ * concrete examples of how Excel performs the same logic (see TestRVA).<p/>
+ * 
+ * Hopefully, as additional important test cases are identified and added to the test suite, 
+ * patterns might become more obvious in this code and allow for simplification.
+ * 
+ * @author Josh Micich
+ */
+final class OperandClassTransformer {
+
+	private final int _formulaType;
+
+	public OperandClassTransformer(int formulaType) {
+		_formulaType = formulaType;
+	}
+
+	/**
+	 * Traverses the supplied formula parse tree, calling <tt>Ptg.setClass()</tt> for each non-base
+	 * token to set its operand class.
+	 */
+	public void transformFormula(ParseNode rootNode) {
+		byte rootNodeOperandClass;
+		switch (_formulaType) {
+			case FormulaParser.FORMULA_TYPE_CELL:
+				rootNodeOperandClass = Ptg.CLASS_VALUE;
+				break;
+			default:
+				throw new RuntimeException("Incomplete code - formula type (" 
+						+ _formulaType + ") not supported yet");
+		
+		}
+		transformNode(rootNode, rootNodeOperandClass, false);
+	}
+
+	private void transformNode(ParseNode node, byte desiredOperandClass,
+			boolean callerForceArrayFlag) {
+		Ptg token = node.getToken();
+		ParseNode[] children = node.getChildren();
+		if (token instanceof ValueOperatorPtg || token instanceof ControlPtg) {
+			// Value Operator Ptgs and Control are base tokens, so token will be unchanged
+			
+			// but any child nodes are processed according to desiredOperandClass and callerForceArrayFlag
+			for (int i = 0; i < children.length; i++) {
+				ParseNode child = children[i];
+				transformNode(child, desiredOperandClass, callerForceArrayFlag);
+			}
+			return;
+		}
+		if (token instanceof AbstractFunctionPtg) {
+			transformFunctionNode((AbstractFunctionPtg) token, children, desiredOperandClass,
+					callerForceArrayFlag);
+			return;
+		}
+		if (children.length > 0) {
+			throw new IllegalStateException("Node should not have any children");
+		}
+
+		if (token.isBaseToken()) {
+			// nothing to do
+			return;
+		}
+        if (callerForceArrayFlag) {
+        	switch (desiredOperandClass) {
+        		case Ptg.CLASS_VALUE:
+        		case Ptg.CLASS_ARRAY:
+        			token.setClass(Ptg.CLASS_ARRAY); 
+        			break;
+        		case Ptg.CLASS_REF:
+        			token.setClass(Ptg.CLASS_REF); 
+        			break;
+        		default:
+        			throw new IllegalStateException("Unexpected operand class ("
+        					+ desiredOperandClass + ")");
+        	}
+        } else {
+        	token.setClass(desiredOperandClass);
+        }
+	}
+
+	private void transformFunctionNode(AbstractFunctionPtg afp, ParseNode[] children,
+			byte desiredOperandClass, boolean callerForceArrayFlag) {
+
+		boolean localForceArrayFlag;
+		byte defaultReturnOperandClass = afp.getDefaultOperandClass();
+
+		if (callerForceArrayFlag) {
+			switch (defaultReturnOperandClass) {
+				case Ptg.CLASS_REF:
+					if (desiredOperandClass == Ptg.CLASS_REF) {
+						afp.setClass(Ptg.CLASS_REF);
+					} else {
+						afp.setClass(Ptg.CLASS_ARRAY);
+					}
+					localForceArrayFlag = false;
+					break;
+				case Ptg.CLASS_ARRAY:
+					afp.setClass(Ptg.CLASS_ARRAY);
+					localForceArrayFlag = false;
+					break;
+				case Ptg.CLASS_VALUE:
+					afp.setClass(Ptg.CLASS_ARRAY);
+					localForceArrayFlag = true;
+					break;
+				default:
+					throw new IllegalStateException("Unexpected operand class ("
+							+ defaultReturnOperandClass + ")");
+			}
+		} else {
+			if (defaultReturnOperandClass == desiredOperandClass) {
+				localForceArrayFlag = false;
+				// an alternative would have been to for non-base Ptgs to set their operand class 
+				// from their default, but this would require the call in many subclasses because
+				// the default OC is not known until the end of the constructor
+				afp.setClass(defaultReturnOperandClass); 
+			} else {
+				switch (desiredOperandClass) {
+					case Ptg.CLASS_VALUE:
+						// always OK to set functions to return 'value'
+						afp.setClass(Ptg.CLASS_VALUE); 
+						localForceArrayFlag = false;
+						break;
+					case Ptg.CLASS_ARRAY:
+						switch (defaultReturnOperandClass) {
+							case Ptg.CLASS_REF:
+								afp.setClass(Ptg.CLASS_REF);
+								break;
+							case Ptg.CLASS_VALUE:
+								afp.setClass(Ptg.CLASS_ARRAY);
+								break;
+							default:
+								throw new IllegalStateException("Unexpected operand class ("
+										+ defaultReturnOperandClass + ")");
+						}
+						localForceArrayFlag = (defaultReturnOperandClass == Ptg.CLASS_VALUE);
+						break;
+					case Ptg.CLASS_REF:
+						switch (defaultReturnOperandClass) {
+							case Ptg.CLASS_ARRAY:
+								afp.setClass(Ptg.CLASS_ARRAY);
+								break;
+							case Ptg.CLASS_VALUE:
+								afp.setClass(Ptg.CLASS_VALUE);
+								break;
+							default:
+								throw new IllegalStateException("Unexpected operand class ("
+										+ defaultReturnOperandClass + ")");
+						}
+						localForceArrayFlag = false;
+						break;
+					default:
+						throw new IllegalStateException("Unexpected operand class ("
+								+ desiredOperandClass + ")");
+				}
+
+			}
+		}
+
+		for (int i = 0; i < children.length; i++) {
+			ParseNode child = children[i];
+			byte paramOperandClass = afp.getParameterClass(i);
+			transformNode(child, paramOperandClass, localForceArrayFlag);
+		}
+	}
+}

src/java/org/apache/poi/hssf/model/ParseNode.java

@@ -0,0 +1,201 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.model;
+
+import org.apache.poi.hssf.record.formula.AttrPtg;
+import org.apache.poi.hssf.record.formula.FuncVarPtg;
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.function.FunctionMetadataRegistry;
+/**
+ * Represents a syntactic element from a formula by encapsulating the corresponding <tt>Ptg</tt>
+ * token.  Each <tt>ParseNode</tt> may have child <tt>ParseNode</tt>s in the case when the wrapped
+ * <tt>Ptg</tt> is non-atomic.
+ * 
+ * @author Josh Micich
+ */
+final class ParseNode {
+
+	public static final ParseNode[] EMPTY_ARRAY = { };
+	private final Ptg _token;
+	private final ParseNode[] _children;
+	private boolean _isIf;
+	private final int _tokenCount;
+
+	public ParseNode(Ptg token, ParseNode[] children) {
+		_token = token;
+		_children = children;
+		_isIf = isIf(token);
+		int tokenCount = 1;
+		for (int i = 0; i < children.length; i++) {
+			tokenCount += children[i].getTokenCount();
+		}
+		if (_isIf) {
+			// there will be 2 or 3 extra tAttr tokens according to whether the false param is present
+			tokenCount += children.length;
+		}
+		_tokenCount = tokenCount;
+	}
+	public ParseNode(Ptg token) {
+		this(token, EMPTY_ARRAY);
+	}
+	public ParseNode(Ptg token, ParseNode child0) {
+		this(token, new ParseNode[] { child0, });
+	}
+	public ParseNode(Ptg token, ParseNode child0, ParseNode child1) {
+		this(token, new ParseNode[] { child0, child1, });
+	}
+	private int getTokenCount() {
+		return _tokenCount;
+	}
+
+	/**
+	 * Collects the array of <tt>Ptg</tt> tokens for the specified tree.
+	 */
+	public static Ptg[] toTokenArray(ParseNode rootNode) {
+		TokenCollector temp = new TokenCollector(rootNode.getTokenCount());
+		rootNode.collectPtgs(temp);
+		return temp.getResult();
+	}
+	private void collectPtgs(TokenCollector temp) {
+		if (isIf(getToken())) {
+			collectIfPtgs(temp);
+			return;
+		}
+		for (int i=0; i< getChildren().length; i++) {
+			getChildren()[i].collectPtgs(temp);
+		}
+		temp.add(getToken());
+	}
+	/**
+	 * The IF() function gets marked up with two or three tAttr tokens.
+	 * Similar logic will be required for CHOOSE() when it is supported
+	 * 
+	 * See excelfileformat.pdf sec 3.10.5 "tAttr (19H)
+	 */
+	private void collectIfPtgs(TokenCollector temp) {
+
+		// condition goes first
+		getChildren()[0].collectPtgs(temp);
+		
+		// placeholder for tAttrIf
+		int ifAttrIndex = temp.createPlaceholder();
+		
+		// true parameter
+		getChildren()[1].collectPtgs(temp);
+		
+		// placeholder for first skip attr
+		int skipAfterTrueParamIndex = temp.createPlaceholder();
+		int trueParamSize = temp.sumTokenSizes(ifAttrIndex+1, skipAfterTrueParamIndex);
+
+		AttrPtg attrIf = new AttrPtg();
+		attrIf.setOptimizedIf(true);
+		AttrPtg attrSkipAfterTrue = new AttrPtg();
+		attrSkipAfterTrue.setGoto(true);
+		
+		if (getChildren().length > 2) {
+			// false param present
+			
+			// false parameter
+			getChildren()[2].collectPtgs(temp);
+			
+			int skipAfterFalseParamIndex = temp.createPlaceholder();
+
+			AttrPtg attrSkipAfterFalse = new AttrPtg();
+			attrSkipAfterFalse.setGoto(true);
+
+			int falseParamSize =  temp.sumTokenSizes(skipAfterTrueParamIndex+1, skipAfterFalseParamIndex);
+			
+			attrIf.setData((short)(trueParamSize + 4)); // distance to start of false parameter. +4 for skip after true
+			attrSkipAfterTrue.setData((short)(falseParamSize + 4 + 4 - 1)); // 1 less than distance to end of if FuncVar(size=4). +4 for attr skip before 
+			attrSkipAfterFalse.setData((short)(4 - 1)); // 1 less than distance to end of if FuncVar(size=4). 
+
+			temp.setPlaceholder(ifAttrIndex, attrIf);
+			temp.setPlaceholder(skipAfterTrueParamIndex, attrSkipAfterTrue);
+			temp.setPlaceholder(skipAfterFalseParamIndex, attrSkipAfterFalse);
+		} else {
+			// false parameter not present
+			attrIf.setData((short)(trueParamSize + 4)); // distance to start of FuncVar. +4 for skip after true
+			attrSkipAfterTrue.setData((short)(4 - 1)); // 1 less than distance to end of if FuncVar(size=4). 
+			
+			temp.setPlaceholder(ifAttrIndex, attrIf);
+			temp.setPlaceholder(skipAfterTrueParamIndex, attrSkipAfterTrue);
+		}
+		
+		temp.add(getToken());
+	}
+
+	private static boolean isIf(Ptg token) {
+		if (token instanceof FuncVarPtg) {
+			FuncVarPtg func = (FuncVarPtg) token;
+			if (FunctionMetadataRegistry.FUNCTION_NAME_IF.equals(func.getName())) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	public Ptg getToken() {
+		return _token;
+	}
+
+	public ParseNode[] getChildren() {
+		return _children;
+	}
+
+	private static final class TokenCollector {
+
+		private final Ptg[] _ptgs;
+		private int _offset;
+
+		public TokenCollector(int tokenCount) {
+			_ptgs = new Ptg[tokenCount];
+			_offset = 0;
+		}
+
+		public int sumTokenSizes(int fromIx, int toIx) {
+			int result = 0;
+			for (int i=fromIx; i<toIx; i++) {
+				result += _ptgs[i].getSize();
+			}
+			return result;
+		}
+
+		public int createPlaceholder() {
+			return _offset++;
+		}
+
+		public void add(Ptg token) {
+			if (token == null) {
+				throw new IllegalArgumentException("token must not be null");
+			}
+			_ptgs[_offset] = token;
+			_offset++;
+		}
+
+		public void setPlaceholder(int index, Ptg token) {
+			if (_ptgs[index] != null) {
+				throw new IllegalStateException("Invalid placeholder index (" + index + ")");
+			}
+			_ptgs[index] = token;
+		}
+
+		public Ptg[] getResult() {
+			return _ptgs;
+		}
+	}
+}

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

@@ -108,7 +108,10 @@ public class EmbeddedObjectRefSubRecord
             in.readByte(); // discard
         }
 
-        field_6_stream_id              = in.readInt();
+        // Fetch the stream ID
+        field_6_stream_id = in.readInt();
+        
+        // Store what's left
         remainingBytes = in.readRemainder();
     }
 

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

@@ -44,6 +44,10 @@ public abstract class AbstractFunctionPtg extends OperationPtg {
     protected byte field_1_num_args;
     protected short field_2_fnc_index;
 
+    public final boolean isBaseToken() {
+    	return false;
+    }
+    
     public String toString() {
         StringBuffer sb = new StringBuffer(64);
         sb.append(getClass().getName()).append(" [");

src/java/org/apache/poi/hssf/record/formula/AddPtg.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
@@ -16,11 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-/*
- * AddPtg.java
- *
- * Created on October 29, 2001, 7:48 PM
- */
 package org.apache.poi.hssf.record.formula;
 
 import org.apache.poi.ss.usermodel.Workbook;
@@ -32,10 +26,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  Andrew C. Oliver (acoliver@apache.org)
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
-public class AddPtg
-    extends OperationPtg
-{
+public final class AddPtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x03;
     
@@ -89,8 +80,6 @@ public class AddPtg
         buffer.append(operands[ 1 ]);
         return buffer.toString();
     }
-    
-    public byte getDefaultOperandClass() {return Ptg.CLASS_VALUE;}
            
     public Object clone() {
       return new AddPtg();

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

@@ -35,8 +35,7 @@ import org.apache.poi.util.LittleEndian;
  * @author Jason Height (jheight at chariot dot net dot au)
  * @version 1.0-pre
  */
-public class Area3DPtg extends Ptg implements AreaI
-{
+public class Area3DPtg extends OperandPtg implements AreaI {
 	public final static byte sid = 0x3b;
 	private final static int SIZE = 11; // 10 + 1 for Ptg
 	private short field_1_index_extern_sheet;

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

@@ -31,7 +31,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  andy
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-public class AreaPtg extends Ptg implements AreaI {
+public class AreaPtg extends OperandPtg implements AreaI {
     /**
      * TODO - (May-2008) fix subclasses of AreaPtg 'AreaN~' which are used in shared formulas.
      * see similar comment in ReferencePtg

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

@@ -59,6 +59,10 @@ public class ArrayPtg extends Ptg {
 		}
 	}
 	
+	public boolean isBaseToken() {
+		return false;
+	}
+	
 	/** 
 	 * Read in the actual token (array) values. This occurs 
 	 * AFTER the last Ptg in the expression.
@@ -95,6 +99,10 @@ public class ArrayPtg extends Ptg {
 		return buffer.toString();
 	}
 
+	/**
+	 * Note - (2D) array elements are stored column by column 
+	 * @return the index into the internal 1D array for the specified column and row
+	 */
 	/* package */ int getValueIndex(int colIx, int rowIx) {
 		if(colIx < 0 || colIx >= token_1_columns) {
 			throw new IllegalArgumentException("Specified colIx (" + colIx 
@@ -104,7 +112,7 @@ public class ArrayPtg extends Ptg {
 			throw new IllegalArgumentException("Specified rowIx (" + rowIx 
 					+ ") is outside the allowed range (0.." + (token_2_rows-1) + ")");
 		}
-		return rowIx * token_1_columns + colIx;
+		return rowIx + token_2_rows * colIx;
 	}
 
 	public void writeBytes(byte[] data, int offset) {

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

@@ -15,7 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record.formula;
 
 import org.apache.poi.ss.usermodel.Workbook;
@@ -32,8 +31,7 @@ import org.apache.poi.util.BitFieldFactory;
  * @author  andy
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
-public final class AttrPtg extends OperationPtg {
+public final class AttrPtg extends ControlPtg {
     public final static byte sid  = 0x19;
     private final static int  SIZE = 4;
     private byte              field_1_options;
@@ -289,12 +287,6 @@ public final class AttrPtg extends OperationPtg {
       }
       return "UNKNOWN ATTRIBUTE";
      }
-    
-    
- 
-    public byte getDefaultOperandClass() {
-        return Ptg.CLASS_VALUE;
-    }
 
     public Object clone() {
         int[] jt;

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

@@ -27,10 +27,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author Andrew C. Oliver (acoliver at apache dot org)
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
-public class BoolPtg
-    extends Ptg
-{
+public final class BoolPtg extends ScalarConstantPtg {
     public final static int  SIZE = 2;
     public final static byte sid  = 0x1d;
     private boolean          field_1_value;
@@ -75,8 +72,6 @@ public class BoolPtg
         return field_1_value ? "TRUE" : "FALSE";
     }
 
-    public byte getDefaultOperandClass() {return Ptg.CLASS_VALUE;}
-
     public Object clone() {
         BoolPtg ptg = new BoolPtg();
         ptg.field_1_value = field_1_value;

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

@@ -15,7 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record.formula;
 
 import org.apache.poi.ss.usermodel.Workbook;
@@ -26,10 +25,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  andy
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
-public class ConcatPtg
-    extends OperationPtg
-{
+public final class ConcatPtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x08;
     

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

@@ -15,11 +15,24 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record.formula;
 
-public abstract class ControlPtg
-        extends Ptg
-{
+/**
+ * Common superclass for 
+ * tExp
+ * tTbl
+ * tParen
+ * tNlr
+ * tAttr
+ * tSheet
+ * tEndSheet
+ */
+public abstract class ControlPtg extends Ptg {
 
+	public boolean isBaseToken() {
+		return true;
+	}
+	public final byte getDefaultOperandClass() {
+		throw new IllegalStateException("Control tokens are not classified");
+	}
 }

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

@@ -15,7 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record.formula;
 
 import org.apache.poi.ss.usermodel.Workbook;
@@ -26,10 +25,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  Andrew C. Oliver acoliver at apache dot org
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
-public class DividePtg
-    extends OperationPtg
-{
+public final class DividePtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x06;
 

src/java/org/apache/poi/hssf/record/formula/EqualPtg.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
@@ -25,10 +24,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  *
  * @author  andy
  */
-
-public class EqualPtg
-    extends OperationPtg
-{
+public final class EqualPtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x0b;
 

src/java/org/apache/poi/hssf/record/formula/ErrPtg.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
@@ -16,7 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record.formula;
 
 import org.apache.poi.ss.usermodel.Workbook;
@@ -26,7 +24,7 @@ import org.apache.poi.hssf.usermodel.HSSFErrorConstants;
 /**
  * @author Daniel Noll (daniel at nuix dot com dot au)
  */
-public final class ErrPtg extends Ptg {
+public final class ErrPtg extends ScalarConstantPtg {
     
     // convenient access to namespace
     private static final HSSFErrorConstants EC = null;
@@ -78,10 +76,6 @@ public final class ErrPtg extends Ptg {
         return SIZE;
     }
 
-    public byte getDefaultOperandClass() {
-        return Ptg.CLASS_VALUE;
-    }
-
     public Object clone() {
         return new ErrPtg(field_1_error_code);
     }

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

@@ -31,9 +31,7 @@ import org.apache.poi.util.LittleEndian;
  * @author dmui (save existing implementation)
  */
 
-public class ExpPtg
-    extends Ptg
-{
+public final class ExpPtg extends ControlPtg {
     private final static int  SIZE = 5;
     public final static short sid  = 0x1;
     private short            field_1_first_row;
@@ -52,7 +50,7 @@ public class ExpPtg
       field_1_first_row = in.readShort();
       field_2_first_col = in.readShort();
     }
-
+    
     public void writeBytes(byte [] array, int offset)
     {
       array[offset+0]= (byte) (sid);
@@ -86,8 +84,6 @@ public class ExpPtg
         return buffer.toString();
     }    
     
-    public byte getDefaultOperandClass() {return Ptg.CLASS_VALUE;}
-    
     public Object clone() {
 	ExpPtg result = new ExpPtg();
         result.field_1_first_row = field_1_first_row;

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

@@ -44,6 +44,8 @@ public final class FuncPtg extends AbstractFunctionPtg {
             throw new RuntimeException("Invalid built-in function index (" + field_2_fnc_index + ")");
         }
         numParams = fm.getMinParams();
+        returnClass = fm.getReturnClassCode();
+        paramClass = fm.getParameterClassCodes();
     }
     public FuncPtg(int functionIndex) {
         field_2_fnc_index = (short) functionIndex;

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

@@ -40,6 +40,15 @@ public final class FuncVarPtg extends AbstractFunctionPtg{
     public FuncVarPtg(RecordInputStream in) {
         field_1_num_args = in.readByte();
         field_2_fnc_index  = in.readShort();
+        FunctionMetadata fm = FunctionMetadataRegistry.getFunctionByIndex(field_2_fnc_index);
+        if(fm == null) {
+            // Happens only as a result of a call to FormulaParser.parse(), with a non-built-in function name
+            returnClass = Ptg.CLASS_VALUE;
+            paramClass = new byte[] {Ptg.CLASS_VALUE};
+        } else {
+            returnClass = fm.getReturnClassCode();
+            paramClass = fm.getParameterClassCodes();
+        }
     }
 
     /**

src/java/org/apache/poi/hssf/record/formula/GreaterEqualPtg.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
@@ -16,22 +15,17 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record.formula;
 
 import org.apache.poi.ss.usermodel.Workbook;
 import org.apache.poi.hssf.record.RecordInputStream;
 
-
 /**
  * PTG class to implement greater or equal to
  *
  * @author  fred at stsci dot edu
  */
-
-public class GreaterEqualPtg
-    extends OperationPtg
-{
+public final class GreaterEqualPtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x0c;
 

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

@@ -15,16 +15,8 @@
    limitations under the License.
 ==================================================================== */
 
-
-/*
- * GreaterThanPtg.java
- *
- * Created on January 23, 2003, 9:47 AM
- */
 package org.apache.poi.hssf.record.formula;
 
-import java.util.List;
-
 import org.apache.poi.ss.usermodel.Workbook;
 import org.apache.poi.hssf.record.RecordInputStream;
 
@@ -32,9 +24,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * Greater than operator PTG ">"
  * @author  Cameron Riley (criley at ekmail.com)
  */
-public class GreaterThanPtg
-    extends OperationPtg
-{
+public final class GreaterThanPtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x0D;    
     private final static String GREATERTHAN = ">";
@@ -117,15 +107,6 @@ public class GreaterThanPtg
         return buffer.toString();
     }
     
-    /**
-     * Get the default operands class value
-     * @return byte the Ptg Class Value as a byte from the Ptg Parent object
-     */
-    public byte getDefaultOperandClass() 
-    {
-        return Ptg.CLASS_VALUE;
-    }
-           
     /**
      * Implementation of clone method from Object
      * @return Object a clone of this class as an Object

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

@@ -27,7 +27,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  Andrew C. Oliver (acoliver at apache dot org)
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-public final class IntPtg extends Ptg {
+public final class IntPtg extends ScalarConstantPtg {
     // 16 bit unsigned integer
     private static final int MIN_VALUE = 0x0000;
     private static final int MAX_VALUE = 0xFFFF;
@@ -75,9 +75,6 @@ public final class IntPtg extends Ptg {
     public String toFormulaString(Workbook book) {
         return String.valueOf(getValue());
     }
-    public byte getDefaultOperandClass() {
-        return Ptg.CLASS_VALUE;
-    }
 
     public Object clone() {
      return new IntPtg(field_1_value);

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

@@ -23,8 +23,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
 /**
  * @author Daniel Noll (daniel at nuix dot com dot au)
  */
-public class IntersectionPtg extends OperationPtg
-{
+public final class IntersectionPtg extends OperationPtg {
     public final static byte sid  = 0x0f;
 
 
@@ -37,6 +36,9 @@ public class IntersectionPtg extends OperationPtg
         // doesn't need anything
     }
 
+    public final boolean isBaseToken() {
+        return true;
+    }
 
     public int getSize()
     {

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

@@ -16,7 +16,6 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record.formula;
 
 
@@ -29,9 +28,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  *
  * @author fred at stsci dot edu
  */
-public class LessEqualPtg
-        extends OperationPtg
-{
+public final class LessEqualPtg extends ValueOperatorPtg {
     public final static int SIZE = 1;
     public final static byte sid = 0x0a;
 

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

@@ -15,18 +15,8 @@
    limitations under the License.
 ==================================================================== */
 
-
-/*
- * LessThanPtg.java
- *
- * Created on January 23, 2003, 9:47 AM
- */
 package org.apache.poi.hssf.record.formula;
 
-//JDK
-import java.util.List;
-
-//POI
 import org.apache.poi.ss.usermodel.Workbook;
 import org.apache.poi.hssf.record.RecordInputStream;
 
@@ -36,9 +26,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * Table 3.5.7
  * @author Cameron Riley (criley at ekmail.com)
  */
-public class LessThanPtg
-    extends OperationPtg
-{
+public final class LessThanPtg extends ValueOperatorPtg {
     /** the size of the Ptg  */
     public final static int SIZE = 1;
 
@@ -125,15 +113,6 @@ public class LessThanPtg
         return buffer.toString();
     }
     
-    /**
-     * Get the default operands class value
-     * @return byte the Ptg Class Value as a byte from the Ptg Parent object
-     */
-    public byte getDefaultOperandClass() 
-    {
-        return Ptg.CLASS_VALUE;
-    }
-    
     /**
      * Implementation of clone method from Object
      * @return Object a clone of this class as an Object

src/java/org/apache/poi/hssf/record/formula/MemAreaPtg.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
@@ -16,12 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
-/*
- * MemAreaPtg.java
- *
- * Created on November 21, 2001, 8:46 AM
- */
 package org.apache.poi.hssf.record.formula;
 
 import org.apache.poi.util.LittleEndian;
@@ -31,9 +24,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
 /**
  * @author Daniel Noll (daniel at nuix dot com dot au)
  */
-public class MemAreaPtg
-    extends Ptg
-{
+public class MemAreaPtg extends OperandPtg {
     public final static short sid  = 0x26;
     private final static int  SIZE = 7;
     private int               field_1_reserved;

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

@@ -30,8 +30,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
 /**
  * @author Glen Stampoultzis (glens at apache.org)
  */
-public class MemFuncPtg extends ControlPtg
-{
+public class MemFuncPtg extends OperandPtg {
 
     public final static byte sid = 0x29;
     private short field_1_len_ref_subexpression = 0;

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

@@ -26,9 +26,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * Avik Sengupta <avik at apache.org>
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-public class MissingArgPtg
-    extends  Ptg
-{
+public final class MissingArgPtg extends ScalarConstantPtg {
    
     private final static int SIZE = 1;
     public final static byte sid  = 0x16;
@@ -59,8 +57,6 @@ public class MissingArgPtg
     {
         return " ";
     }
-    
-    public byte getDefaultOperandClass() {return Ptg.CLASS_VALUE;}
         
     public Object clone() {
       return new MissingArgPtg();

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

@@ -25,9 +25,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author Jason Height (jheight at chariot dot net dot au)
  */
 
-public class MultiplyPtg
-    extends OperationPtg
-{
+public final class MultiplyPtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x05;
 

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

@@ -27,10 +27,7 @@ import org.apache.poi.util.LittleEndian;
  * @author  andy
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
-public class NamePtg
-    extends Ptg
-{
+public final class NamePtg extends OperandPtg {
     public final static short sid  = 0x23;
     private final static int  SIZE = 5;
     /** one-based index to defined name record */

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

@@ -25,7 +25,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  *
  * @author  aviks
  */
-public final class NameXPtg extends Ptg {
+public final class NameXPtg extends OperandPtg {
     public final static short sid  = 0x39;
     private final static int  SIZE = 7;
     private short             field_1_ixals;   // index to REF entry in externsheet record

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

@@ -26,9 +26,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  *
  * @author fred at stsci dot edu
  */
-public class NotEqualPtg
-        extends OperationPtg
-{
+public final class NotEqualPtg extends ValueOperatorPtg {
     public final static int SIZE = 1;
     public final static byte sid = 0x0e;
 

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

@@ -28,10 +28,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  Avik Sengupta
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
-public class NumberPtg
-    extends Ptg
-{
+public final class NumberPtg extends ScalarConstantPtg {
     public final static int  SIZE = 9;
     public final static byte sid  = 0x1f;
     private double            field_1_value;
@@ -82,7 +79,6 @@ public class NumberPtg
     {
         return "" + getValue();
     }
-       public byte getDefaultOperandClass() {return Ptg.CLASS_VALUE;}
 
     public Object clone() {
       NumberPtg ptg = new NumberPtg();

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

@@ -0,0 +1,31 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.record.formula;
+
+/**
+ * @author Josh Micich
+ */
+public abstract class OperandPtg extends Ptg {
+
+	/**
+	 * All Operand <tt>Ptg</tt>s are classifed ('relative', 'value', 'array')  
+	 */
+	public final boolean isBaseToken() {
+		return false;
+	}
+}

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

@@ -32,9 +32,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * Andrew C. Oliver (acoliver at apache dot org)
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-public class ParenthesisPtg
-    extends OperationPtg
-{
+public final class ParenthesisPtg extends ControlPtg {
    
     private final static int SIZE = 1;
     public final static byte sid  = 0x15;
@@ -61,16 +59,6 @@ public class ParenthesisPtg
         return SIZE;
     }
 
-    public int getType()
-    {
-        return TYPE_BINARY;
-    }
-
-    public int getNumberOfOperands()
-    {
-        return 1;
-    }
-
     public String toFormulaString(Workbook book)
     {
         return "()";
@@ -80,8 +68,6 @@ public class ParenthesisPtg
     public String toFormulaString(String[] operands) {
         return "("+operands[0]+")";
     }  
-    
-    public byte getDefaultOperandClass() {return Ptg.CLASS_VALUE;}
         
     public Object clone() {
       return new ParenthesisPtg();

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

@@ -32,9 +32,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author Daniel Noll (daniel at nuix.com.au)
  */
 
-public class PercentPtg
-    extends OperationPtg
-{
+public final class PercentPtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x14;
     
@@ -88,8 +86,6 @@ public class PercentPtg
         return buffer.toString();
     }
     
-    public byte getDefaultOperandClass() {return Ptg.CLASS_VALUE;}
-           
     public Object clone() {
       return new PercentPtg();
     }

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

@@ -17,8 +17,6 @@
 
 package org.apache.poi.hssf.record.formula;
 
-import java.util.List;
-
 import org.apache.poi.ss.usermodel.Workbook;
 import org.apache.poi.hssf.record.RecordInputStream;
 
@@ -27,10 +25,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  andy
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
-public class PowerPtg
-    extends OperationPtg
-{
+public final class PowerPtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x07;
 

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

@@ -119,254 +119,14 @@ public abstract class Ptg
         return stack;
     }
 
-    public static Ptg createPtg(RecordInputStream in)
-    {
-        byte id     = in.readByte();
-        Ptg  retval = null;
-
-        switch (id)
-        {
-             case ExpPtg.sid :                  // 0x01
-                 retval = new ExpPtg(in);
-                 break;
-
-             case AddPtg.sid :                  // 0x03
-                 retval = new AddPtg(in);
-                 break;
-
-             case SubtractPtg.sid :             // 0x04
-                 retval = new SubtractPtg(in);
-                 break;
-
-             case MultiplyPtg.sid :             // 0x05
-                 retval = new MultiplyPtg(in);
-                 break;
-
-             case DividePtg.sid :               // 0x06
-                  retval = new DividePtg(in);
-                  break;
-
-             case PowerPtg.sid :                // 0x07
-                 retval = new PowerPtg(in);
-                 break;
-
-             case ConcatPtg.sid :               // 0x08
-                 retval = new ConcatPtg(in);
-                              break;
-
-             case LessThanPtg.sid:              // 0x09
-                 retval = new LessThanPtg(in);
-                              break;
-
-              case LessEqualPtg.sid :            // 0x0a
-                 retval = new LessEqualPtg(in);
-                              break;
-
-             case EqualPtg.sid :                // 0x0b
-                 retval = new EqualPtg(in);
-                              break;
-
-             case GreaterEqualPtg.sid :         // 0x0c
-                 retval = new GreaterEqualPtg(in);
-                              break;
-
-             case GreaterThanPtg.sid :          // 0x0d
-                 retval = new GreaterThanPtg(in);
-                              break;
-
-             case NotEqualPtg.sid :             // 0x0e
-                 retval = new NotEqualPtg(in);
-                              break;
-
-             case IntersectionPtg.sid :         // 0x0f
-                 retval = new IntersectionPtg(in);
-                              break;
-              case UnionPtg.sid :                // 0x10
-                 retval = new UnionPtg(in);
-                              break;
-
-             case RangePtg.sid :                // 0x11
-                 retval = new RangePtg(in);
-                              break;
-
-             case UnaryPlusPtg.sid :            // 0x12
-                 retval = new UnaryPlusPtg(in);
-                              break;
-
-             case UnaryMinusPtg.sid :           // 0x13
-                 retval = new UnaryMinusPtg(in);
-                              break;
-
-             case PercentPtg.sid :              // 0x14
-                 retval = new PercentPtg(in);
-                              break;
-
-             case ParenthesisPtg.sid :          // 0x15
-                 retval = new ParenthesisPtg(in);
-                              break;
-
-             case MissingArgPtg.sid :           // 0x16
-                 retval = new MissingArgPtg(in);
-                              break;
-
-             case StringPtg.sid :               // 0x17
-                retval = new StringPtg(in);
-                break;
-
-             case AttrPtg.sid :                 // 0x19
-             case 0x1a :
-                 retval = new AttrPtg(in);
-                              break;
-
-             case ErrPtg.sid :                  // 0x1c
-                 retval = new ErrPtg(in);
-                              break;
-
-             case BoolPtg.sid :                 // 0x1d
-                retval = new BoolPtg(in);
-                break;
-
-             case IntPtg.sid :                  // 0x1e
-                 retval = new IntPtg(in);
-                              break;
-
-             case NumberPtg.sid :               // 0x1f
-                  retval = new NumberPtg(in);
-                  break;
-
-             case ArrayPtg.sid :                // 0x20
-                 retval = new ArrayPtg(in);
-                 break;
-             case ArrayPtgV.sid :               // 0x40
-                 retval = new ArrayPtgV(in);
-                 break;
-             case ArrayPtgA.sid :               // 0x60
-                 retval = new ArrayPtgA(in);
-                 break;
-
-             case FuncPtg.sid :                 // 0x21
-             case FuncPtg.sid + 0x20 :          // 0x41
-             case FuncPtg.sid + 0x40 :          // 0x61
-                 retval = new FuncPtg(in);
-                 break;
-
-             case FuncVarPtg.sid :              // 0x22
-             case FuncVarPtg.sid + 0x20 :       // 0x42
-             case FuncVarPtg.sid + 0x40 :       // 0x62
-                 retval = new FuncVarPtg(in);
-                              break;
-
-             case ReferencePtg.sid :            // 0x24
-                 retval = new ReferencePtg(in);
-                              break;
-             case RefAPtg.sid :                 // 0x64
-                 retval = new RefAPtg(in);
-                 break;
-             case RefVPtg.sid :                 // 0x44
-                 retval = new RefVPtg(in);
-                 break;
-             case RefNAPtg.sid :                // 0x6C
-                 retval = new RefNAPtg(in);
-                 break;
-             case RefNPtg.sid :                 // 0x2C
-                 retval = new RefNPtg(in);
-                 break;
-             case RefNVPtg.sid :                // 0x4C
-                 retval = new RefNVPtg(in);
-                 break;
-
-             case AreaPtg.sid :                 // 0x25
-                 retval = new AreaPtg(in);
-                              break;
-             case AreaVPtg.sid:                 // 0x45
-                 retval = new AreaVPtg(in);
-                 break;
-             case AreaAPtg.sid:                 // 0x65
-                 retval = new AreaAPtg(in);
-                 break;
-             case AreaNAPtg.sid :               // 0x6D
-                 retval = new AreaNAPtg(in);
-                  break;
-             case AreaNPtg.sid :                // 0x2D
-                 retval = new AreaNPtg(in);
-                 break;
-             case AreaNVPtg.sid :               // 0x4D
-                retval = new AreaNVPtg(in);
-                break;
-
-             case MemAreaPtg.sid :              // 0x26
-             case MemAreaPtg.sid + 0x40 :       // 0x46
-             case MemAreaPtg.sid + 0x20 :       // 0x66
-                 retval = new MemAreaPtg(in);
-                 break;
-
-             case MemErrPtg.sid :               // 0x27
-             case MemErrPtg.sid + 0x20 :        // 0x47
-             case MemErrPtg.sid + 0x40 :        // 0x67
-                 retval = new MemErrPtg(in);
-                              break;
-
-             case MemFuncPtg.sid :              // 0x29
-                 retval = new MemFuncPtg(in);
-                 break;
-
-             case RefErrorPtg.sid :             // 0x2a
-             case RefErrorPtg.sid + 0x20 :      // 0x4a
-             case RefErrorPtg.sid + 0x40 :      // 0x6a
-                 retval = new RefErrorPtg(in);
-                              break;
-
-             case AreaErrPtg.sid :              // 0x2b
-             case AreaErrPtg.sid + 0x20 :       // 0x4b
-             case AreaErrPtg.sid + 0x40 :       // 0x6b
-                 retval = new AreaErrPtg(in);
-                              break;
-
-             case NamePtg.sid :                 // 0x23
-             case NamePtg.sid + 0x20 :          // 0x43
-             case NamePtg.sid + 0x40 :          // 0x63
-                 retval = new NamePtg(in);
-                 break;
-
-             case NameXPtg.sid :                // 0x39
-             case NameXPtg.sid + 0x20 :         // 0x45
-             case NameXPtg.sid + 0x40 :         // 0x79
-                 retval = new NameXPtg(in);
-                              break;
-
-             case Area3DPtg.sid :               // 0x3b
-             case Area3DPtg.sid + 0x20 :        // 0x5b
-             case Area3DPtg.sid + 0x40 :        // 0x7b
-                 retval = new Area3DPtg(in);
-                              break;
-
-             case Ref3DPtg.sid :                // 0x3a
-             case Ref3DPtg.sid + 0x20:          // 0x5a
-             case Ref3DPtg.sid + 0x40:          // 0x7a
-                 retval = new Ref3DPtg(in);
-                              break;
-
-             case DeletedRef3DPtg.sid:          // 0x3c
-             case DeletedRef3DPtg.sid + 0x20:   // 0x5c
-             case DeletedRef3DPtg.sid + 0x40:   // 0x7c
-                 retval = new DeletedRef3DPtg(in);
-                              break;
-
-             case DeletedArea3DPtg.sid :        // 0x3d
-             case DeletedArea3DPtg.sid + 0x20 : // 0x5d
-             case DeletedArea3DPtg.sid + 0x40 : // 0x7d
-                 retval = new DeletedArea3DPtg(in);
-                 break;
-
-             case 0x00:
-                 retval = new UnknownPtg();
-                 break;
-
-            default :
-                 //retval = new UnknownPtg();
-                 throw new java.lang.UnsupportedOperationException(" Unknown Ptg in Formula: 0x"+
-                        Integer.toHexString(( int ) id) + " (" + ( int ) id + ")");
+    public static Ptg createPtg(RecordInputStream in) {
+        byte id = in.readByte();
+        
+        if (id < 0x20) {
+        	return createBasePtg(id, in);
         }
+        
+        Ptg  retval = createClassifiedPtg(id, in);
 
         if (id > 0x60) {
             retval.setClass(CLASS_ARRAY);
@@ -380,6 +140,118 @@ public abstract class Ptg
 
     }
 
+	private static Ptg createClassifiedPtg(byte id, RecordInputStream in) {
+		
+		int baseId = id & 0x1F | 0x20;
+		
+        switch (baseId) {
+             case FuncPtg.sid:     return new FuncPtg(in);     // 0x21, 0x41, 0x61
+             case FuncVarPtg.sid:  return new FuncVarPtg(in);  // 0x22, 0x42, 0x62
+             case NamePtg.sid:     return new NamePtg(in);     // 0x23, 0x43, 0x63
+
+             case MemAreaPtg.sid:  return new MemAreaPtg(in);  // 0x26, 0x46, 0x66
+             case MemErrPtg.sid:   return new MemErrPtg(in);   // 0x27, 0x47, 0x67
+             case MemFuncPtg.sid:  return new MemFuncPtg(in);  // 0x29, 0x49, 0x69
+             case RefErrorPtg.sid: return  new RefErrorPtg(in);// 0x2a, 0x4a, 0x6a
+             case AreaErrPtg.sid:  return new AreaErrPtg(in);  // 0x2b, 0x4b, 0x6b
+
+             case NameXPtg.sid:    return new NameXPtg(in);    // 0x39, 0x49, 0x79
+             case Ref3DPtg.sid:    return  new Ref3DPtg(in);   // 0x3a, 0x5a, 0x7a
+             case Area3DPtg.sid:   return new Area3DPtg(in);   // 0x3b, 0x5b, 0x7b
+             case DeletedRef3DPtg.sid:  return new DeletedRef3DPtg(in);   // 0x3c, 0x5c, 0x7c
+             case DeletedArea3DPtg.sid: return  new DeletedArea3DPtg(in); // 0x3d, 0x5d, 0x7d
+        }
+        
+        
+        switch (id) {
+        // TODO - why are specific subclasses needed for these Ptgs?
+            case ArrayPtg.sid:     return new ArrayPtg(in);    // 0x20
+            case ArrayPtgV.sid:    return new ArrayPtgV(in);   // 0x40
+            case ArrayPtgA.sid:    return new ArrayPtgA(in);   // 0x60
+
+            case ReferencePtg.sid: return new ReferencePtg(in);// 0x24
+            case RefAPtg.sid:      return new RefAPtg(in);     // 0x64
+            case RefVPtg.sid:      return new RefVPtg(in);     // 0x44
+
+            case RefNAPtg.sid:     return new RefNAPtg(in);    // 0x6C
+            case RefNPtg.sid:      return new RefNPtg(in);     // 0x2C
+            case RefNVPtg.sid:     return new RefNVPtg(in);    // 0x4C
+
+            case AreaPtg.sid:      return new AreaPtg(in);     // 0x25
+            case AreaVPtg.sid:      return new AreaVPtg(in);   // 0x45
+            case AreaAPtg.sid:      return new AreaAPtg(in);   // 0x65
+
+            case AreaNAPtg.sid:    return new AreaNAPtg(in);   // 0x6D
+            case AreaNPtg.sid:     return new AreaNPtg(in);    // 0x2D
+            case AreaNVPtg.sid:    return new AreaNVPtg(in);   // 0x4D
+        
+        }
+        throw new UnsupportedOperationException(" Unknown Ptg in Formula: 0x"+
+                   Integer.toHexString(id) + " (" + ( int ) id + ")");
+	}
+
+	private static Ptg createBasePtg(byte id, RecordInputStream in) {
+		switch(id) {
+		    case 0x00:                return new UnknownPtg(); // TODO - not a real Ptg
+    		case ExpPtg.sid:          return new ExpPtg(in);         // 0x01
+    		case AddPtg.sid:          return new AddPtg(in);         // 0x03
+    		case SubtractPtg.sid:     return new SubtractPtg(in);    // 0x04
+    		case MultiplyPtg.sid:     return new MultiplyPtg(in);    // 0x05
+    		case DividePtg.sid:       return new DividePtg(in);      // 0x06
+    		case PowerPtg.sid:        return new PowerPtg(in);       // 0x07
+    		case ConcatPtg.sid:       return new ConcatPtg(in);      // 0x08
+    		case LessThanPtg.sid:     return new LessThanPtg(in);    // 0x09
+    		case LessEqualPtg.sid:    return new LessEqualPtg(in);   // 0x0a
+    		case EqualPtg.sid:        return new EqualPtg(in);       // 0x0b
+    		case GreaterEqualPtg.sid: return new GreaterEqualPtg(in);// 0x0c
+    		case GreaterThanPtg.sid:  return new GreaterThanPtg(in); // 0x0d
+    		case NotEqualPtg.sid:     return new NotEqualPtg(in);    // 0x0e
+    		case IntersectionPtg.sid: return new IntersectionPtg(in);// 0x0f
+    		case UnionPtg.sid:        return new UnionPtg(in);       // 0x10
+    		case RangePtg.sid:        return new RangePtg(in);       // 0x11
+    		case UnaryPlusPtg.sid:    return new UnaryPlusPtg(in);   // 0x12
+    		case UnaryMinusPtg.sid:   return new UnaryMinusPtg(in);  // 0x13
+    		case PercentPtg.sid:      return new PercentPtg(in);     // 0x14
+    		case ParenthesisPtg.sid:  return new ParenthesisPtg(in); // 0x15
+    		case MissingArgPtg.sid:   return new MissingArgPtg(in);  // 0x16
+    		case StringPtg.sid:       return new StringPtg(in);      // 0x17
+    		case AttrPtg.sid:                
+    		case 0x1a:        return new AttrPtg(in); // 0x19
+    		case ErrPtg.sid:          return new ErrPtg(in);         // 0x1c
+    		case BoolPtg.sid:         return new BoolPtg(in);        // 0x1d
+    		case IntPtg.sid:          return new IntPtg(in);         // 0x1e
+    		case NumberPtg.sid:       return new NumberPtg(in);      // 0x1f
+		}
+		throw new RuntimeException("Unexpected base token id (" + id + ")");
+	}
+    /**
+     * 
+     * 
+     */
+	public static int getEncodedSize(Stack ptgs) {
+		return getEncodedSize(toPtgArray(ptgs));
+	}
+	private static Ptg[] toPtgArray(List l) {
+		Ptg[] result = new Ptg[l.size()];
+		l.toArray(result);
+		return result;
+	}
+    private static Stack createStack(Ptg[] formulaTokens) {
+		Stack result = new Stack();
+		for (int i = 0; i < formulaTokens.length; i++) {
+			result.add(formulaTokens[i]);
+		} 
+		return result;
+	}
+	// TODO - several duplicates of this code should be refactored here
+	public static int getEncodedSize(Ptg[] ptgs) {
+		int result = 0;
+		for (int i = 0; i < ptgs.length; i++) {
+			result += ptgs[i].getSize();
+		}
+		return result;
+	}
+
     public static int serializePtgStack(Stack expression, byte[] array, int offset) {
         int pos = 0;
         int size = 0;
@@ -408,7 +280,15 @@ public abstract class Ptg
         return pos;
     }
 
+    /**
+     * @return the encoded length of this Ptg, including the initial Ptg type identifier byte. 
+     */
     public abstract int getSize();
+    
+    /**
+     * @return the encoded length of this Ptg, not including the initial Ptg type identifier byte. 
+     */
+//    public abstract int getDataSize();
 
     public final byte [] getBytes()
     {
@@ -455,10 +335,15 @@ public abstract class Ptg
     protected byte ptgClass = CLASS_REF; //base ptg
 
     public void setClass(byte thePtgClass) {
+    	if (isBaseToken()) {
+    		throw new RuntimeException("setClass should not be called on a base token");
+    	}
         ptgClass = thePtgClass;
     }
 
-    /** returns the class (REF/VALUE/ARRAY) for this Ptg */
+    /**
+     *  @return the 'operand class' (REF/VALUE/ARRAY) for this Ptg
+     */
     public byte getPtgClass() {
         return ptgClass;
     }
@@ -468,5 +353,8 @@ public abstract class Ptg
     public abstract Object clone();
 
 
-
+    /**
+     * @return <code>false</code> if this token is classified as 'reference', 'value', or 'array'
+     */
+    public abstract boolean isBaseToken();
 }

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

@@ -23,8 +23,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
 /**
  * @author Daniel Noll (daniel at nuix dot com dot au)
  */
-public class RangePtg extends OperationPtg
-{
+public final class RangePtg  extends OperationPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x11;
 
@@ -37,6 +36,10 @@ public class RangePtg extends OperationPtg
     	// No contents
     }
 
+    public final boolean isBaseToken() {
+        return true;
+    }
+
 
     public int getSize()
     {

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

@@ -35,8 +35,7 @@ import org.apache.poi.util.LittleEndian;
  * @author Jason Height (jheight at chariot dot net dot au)
  * @version 1.0-pre
  */
-
-public class Ref3DPtg extends Ptg {
+public class Ref3DPtg extends OperandPtg {
     public final static byte sid  = 0x3a;
     private final static int  SIZE = 7; // 6 + 1 for Ptg
     private short             field_1_index_extern_sheet;

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

@@ -28,9 +28,8 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * RefError - handles deleted cell reference
  * @author Jason Height (jheight at chariot dot net dot au)
  */
+public final class RefErrorPtg extends OperandPtg {
 
-public class RefErrorPtg extends Ptg
-{
     private final static int SIZE = 5;
     public final static byte sid  = 0x2a;
     private int              field_1_reserved;

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

@@ -30,14 +30,14 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  Andrew C. Oliver (acoliver@apache.org)
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-public class ReferencePtg extends Ptg {
+public class ReferencePtg extends OperandPtg {
     /**
      * TODO - (May-2008) fix subclasses of ReferencePtg 'RefN~' which are used in shared formulas.
      * (See bugzilla 44921)
-     * The 'RefN~' instances do not work properly, and are expected to be converted by 
-     * SharedFormulaRecord.convertSharedFormulas().  
-     * This conversion currently does not take place for formulas of named ranges, conditional 
-     * format rules and data validation rules.  
+     * The 'RefN~' instances do not work properly, and are expected to be converted by
+     * SharedFormulaRecord.convertSharedFormulas().
+     * This conversion currently does not take place for formulas of named ranges, conditional
+     * format rules and data validation rules.
      * Furthermore, conversion is probably not appropriate in those instances.
      */
     protected final RuntimeException notImplemented() {
@@ -46,14 +46,14 @@ public class ReferencePtg extends Ptg {
 
     private final static int SIZE = 5;
     public final static byte sid  = 0x24;
-    private final static int MAX_ROW_NUMBER = 65536;             
+    private final static int MAX_ROW_NUMBER = 65536;
 
    /** The row index - zero based unsigned 16 bit value */
     private int            field_1_row;
-    /** Field 2 
-     * - lower 8 bits is the zero based unsigned byte column index 
+    /** Field 2
+     * - lower 8 bits is the zero based unsigned byte column index
      * - bit 16 - isRowRelative
-     * - bit 15 - isColumnRelative 
+     * - bit 15 - isColumnRelative
      */
     private int            field_2_col;
     private static final BitField         rowRelative = BitFieldFactory.getInstance(0x8000);
@@ -63,9 +63,9 @@ public class ReferencePtg extends Ptg {
     protected ReferencePtg() {
       //Required for clone methods
     }
-    
+
     /**
-     * Takes in a String represnetation of a cell reference and fills out the 
+     * Takes in a String represnetation of a cell reference and fills out the
      * numeric fields.
      */
     public ReferencePtg(String cellref) {
@@ -75,13 +75,13 @@ public class ReferencePtg extends Ptg {
         setColRelative(!c.isColAbsolute());
         setRowRelative(!c.isRowAbsolute());
     }
-    
+
     public ReferencePtg(int row, int column, boolean isRowRelative, boolean isColumnRelative) {
       setRow(row);
       setColumn(column);
       setRowRelative(isRowRelative);
       setColRelative(isColumnRelative);
-    }    
+    }
 
     /** Creates new ValueReferencePtg */
 
@@ -90,22 +90,19 @@ public class ReferencePtg extends Ptg {
         field_1_row = in.readUShort();
         field_2_col = in.readUShort();
     }
-    
+
     public String getRefPtgName() {
       return "ReferencePtg";
-    }    
-
-    public String toString()
-    {
-        StringBuffer buffer = new StringBuffer("[");
-        buffer.append(getRefPtgName());
-        buffer.append("]\n");
+    }
 
-        buffer.append("row = ").append(getRow()).append("\n");
-        buffer.append("col = ").append(getColumn()).append("\n");
-        buffer.append("rowrelative = ").append(isRowRelative()).append("\n");
-        buffer.append("colrelative = ").append(isColRelative()).append("\n");
-        return buffer.toString();
+    public String toString() {
+        CellReference cr = new CellReference(getRow(), getColumn(), !isRowRelative(),!isColRelative());
+        StringBuffer sb = new StringBuffer();
+        sb.append(getClass().getName());
+        sb.append(" [");
+        sb.append(cr.formatAsString());
+        sb.append("]");
+        return sb.toString();
     }
 
     public void writeBytes(byte [] array, int offset)
@@ -147,16 +144,16 @@ public class ReferencePtg extends Ptg {
     {
         return rowRelative.isSet(field_2_col);
     }
-    
+
     public void setRowRelative(boolean rel) {
         field_2_col=rowRelative.setBoolean(field_2_col,rel);
     }
-    
+
     public boolean isColRelative()
     {
         return colRelative.isSet(field_2_col);
     }
-    
+
     public void setColRelative(boolean rel) {
         field_2_col=colRelative.setBoolean(field_2_col,rel);
     }
@@ -193,11 +190,11 @@ public class ReferencePtg extends Ptg {
         //TODO -- should we store a cellreference instance in this ptg?? but .. memory is an issue, i believe!
         return (new CellReference(getRowAsInt(),getColumn(),!isRowRelative(),!isColRelative())).formatAsString();
     }
-    
+
     public byte getDefaultOperandClass() {
         return Ptg.CLASS_REF;
     }
-    
+
     public Object clone() {
       ReferencePtg ptg = new ReferencePtg();
       ptg.field_1_row = field_1_row;

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

@@ -0,0 +1,31 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.record.formula;
+
+/**
+ * @author Josh Micich
+ */
+abstract class ScalarConstantPtg extends Ptg {
+	public boolean isBaseToken() {
+		return true;
+	}
+    public final byte getDefaultOperandClass() {
+        return Ptg.CLASS_VALUE;
+    }
+	
+}

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

@@ -31,7 +31,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author Jason Height (jheight at chariot dot net dot au)
  * @author Bernard Chesnoy
  */
-public final class StringPtg extends Ptg {
+public final class StringPtg extends ScalarConstantPtg {
     public final static int SIZE = 9;
     public final static byte sid = 0x17;
     private static final BitField fHighByte = BitFieldFactory.getInstance(0x01);
@@ -124,10 +124,6 @@ public final class StringPtg extends Ptg {
         return sb.toString();
     }
 
-    public byte getDefaultOperandClass() {
-        return Ptg.CLASS_VALUE;
-    }
-
     public Object clone() {
         StringPtg ptg = new StringPtg();
         ptg.field_1_length = field_1_length;

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

@@ -26,10 +26,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  andy
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
-public class SubtractPtg
-    extends OperationPtg    
-{
+public final class SubtractPtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x04;
 

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

@@ -28,8 +28,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author Avik Sengupta
  */
 
-public class UnaryMinusPtg extends OperationPtg
-{
+public final class UnaryMinusPtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x13;
     
@@ -82,8 +81,6 @@ public class UnaryMinusPtg extends OperationPtg
         return buffer.toString();
     }
     
-    public byte getDefaultOperandClass() {return Ptg.CLASS_VALUE;}
-           
     public Object clone() {
       return new UnaryPlusPtg();
     }

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

@@ -28,8 +28,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author Avik Sengupta
  */
 
-public class UnaryPlusPtg extends OperationPtg
-{
+public final class UnaryPlusPtg extends ValueOperatorPtg {
     public final static int  SIZE = 1;
     public final static byte sid  = 0x12;
     
@@ -82,8 +81,6 @@ public class UnaryPlusPtg extends OperationPtg
         return buffer.toString();
     }
     
-    public byte getDefaultOperandClass() {return Ptg.CLASS_VALUE;}
-           
     public Object clone() {
       return new UnaryPlusPtg();
     }

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

@@ -23,8 +23,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
 /**
  * @author Glen Stampoultzis (glens at apache.org)
  */
-public class UnionPtg extends OperationPtg
-{
+public final class UnionPtg extends OperationPtg {
     public final static byte sid  = 0x10;
 
 
@@ -37,6 +36,9 @@ public class UnionPtg extends OperationPtg
         // doesn't need anything
     }
 
+    public final boolean isBaseToken() {
+        return true;
+    }
 
     public int getSize()
     {

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

@@ -24,10 +24,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author  andy
  * @author Jason Height (jheight at chariot dot net dot au)
  */
-
-public class UnknownPtg
-    extends Ptg
-{
+public class UnknownPtg extends Ptg {
     private short size = 1;
 
     /** Creates new UnknownPtg */
@@ -36,12 +33,13 @@ public class UnknownPtg
     {
     }
 
-    public UnknownPtg(RecordInputStream in)
-    {
-
+    public UnknownPtg(RecordInputStream in) {
         // doesn't need anything
     }
 
+    public boolean isBaseToken() {
+    	return true;
+    }
     public void writeBytes(byte [] array, int offset)
     {
     }

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

@@ -0,0 +1,37 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.record.formula;
+
+/**
+ * Common superclass of all value operators.
+ * Subclasses include all unary and binary operators except for the reference operators (IntersectionPtg, RangePtg, UnionPtg) 
+ * 
+ * @author Josh Micich
+ */
+public abstract class ValueOperatorPtg extends OperationPtg {
+
+	/**
+	 * All Operator <tt>Ptg</tt>s are base tokens (i.e. are not RVA classifed)  
+	 */
+	public final boolean isBaseToken() {
+		return true;
+	}
+	public final byte getDefaultOperandClass() {
+		return Ptg.CLASS_VALUE;
+	}
+}

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

@@ -55,36 +55,72 @@ public class HSSFObjectData
         this.record = record;
         this.poifs = poifs;
     }
+    
+    /**
+     * Returns the OLE2 Class Name of the object
+     */
+    public String getOLE2ClassName() {
+    	EmbeddedObjectRefSubRecord subRecord = findObjectRecord();
+    	return subRecord.field_5_ole_classname;
+    }
 
     /**
-     * Gets the object data.
+     * Gets the object data. Only call for ones that have
+     *  data though. See {@link #hasDirectoryEntry()}
      *
      * @return the object data as an OLE2 directory.
      * @throws IOException if there was an error reading the data.
      */
-    public DirectoryEntry getDirectory() throws IOException
-    {
+    public DirectoryEntry getDirectory() throws IOException {
+    	EmbeddedObjectRefSubRecord subRecord = findObjectRecord();
+
+    	int streamId = ((EmbeddedObjectRefSubRecord) subRecord).getStreamId();
+        String streamName = "MBD" + HexDump.toHex(streamId);
+
+        Entry entry = poifs.getRoot().getEntry(streamName);
+        if (entry instanceof DirectoryEntry) {
+            return (DirectoryEntry) entry;
+        } else {
+            throw new IOException("Stream " + streamName + " was not an OLE2 directory");
+        }
+    }
+    
+    /**
+     * Returns the data portion, for an ObjectData
+     *  that doesn't have an associated POIFS Directory
+     *  Entry
+     */
+    public byte[] getObjectData() {
+    	EmbeddedObjectRefSubRecord subRecord = findObjectRecord();
+    	return subRecord.remainingBytes;
+    }
+    
+    /**
+     * Does this ObjectData have an associated POIFS 
+     *  Directory Entry?
+     * (Not all do, those that don't have a data portion)
+     */
+    public boolean hasDirectoryEntry() {
+    	EmbeddedObjectRefSubRecord subRecord = findObjectRecord();
+    	
+    	// Field 6 tells you
+    	return (subRecord.field_6_stream_id != 0);
+    }
+    
+    /**
+     * Finds the EmbeddedObjectRefSubRecord, or throws an 
+     *  Exception if there wasn't one
+     */
+    protected EmbeddedObjectRefSubRecord findObjectRecord() {
         Iterator subRecordIter = record.getSubRecords().iterator();
-        while (subRecordIter.hasNext())
-        {
+        
+        while (subRecordIter.hasNext()) {
             Object subRecord = subRecordIter.next();
-            if (subRecord instanceof EmbeddedObjectRefSubRecord)
-            {
-                int streamId = ((EmbeddedObjectRefSubRecord) subRecord).getStreamId();
-                String streamName = "MBD" + HexDump.toHex(streamId);
-
-                Entry entry = poifs.getRoot().getEntry(streamName);
-                if (entry instanceof DirectoryEntry)
-                {
-                    return (DirectoryEntry) entry;
-                }
-                else
-                {
-                    throw new IOException("Stream " + streamName + " was not an OLE2 directory");
-                }
+            if (subRecord instanceof EmbeddedObjectRefSubRecord) {
+            	return (EmbeddedObjectRefSubRecord)subRecord;
             }
         }
-
+        
         throw new IllegalStateException("Object data does not contain a reference to an embedded object OLE2 directory");
     }
 }

src/java/org/apache/poi/ss/usermodel/DateUtil.java

@@ -220,9 +220,13 @@ public class DateUtil
     	//  switching stuff, which we can ignore
     	fs = fs.replaceAll(";@", "");
     	
-    	// If it starts with [$-...], then it is a date, but
+    	// If it starts with [$-...], then could be a date, but
     	//  who knows what that starting bit is all about
-    	fs = fs.replaceAll("\\[\\$\\-.*?\\]", "");
+    	fs = fs.replaceAll("^\\[\\$\\-.*?\\]", "");
+    	
+    	// If it starts with something like [Black] or [Yellow],
+    	//  then it could be a date
+    	fs = fs.replaceAll("^\\[[a-zA-Z]+\\]", "");
     	
     	// Otherwise, check it's only made up, in any case, of:
     	//  y m d h s - / , . :

src/java/org/apache/poi/ss/usermodel/FormulaEvaluator.java

@@ -330,31 +330,27 @@ public class FormulaEvaluator {
     }
     private static ValueEval evaluateCell(Workbook workbook, Sheet sheet, 
             int srcRowNum, short srcColNum, String cellFormulaText) {
-    	
-    	FormulaParser parser =
-    		new FormulaParser(cellFormulaText, workbook);
-    	
-        parser.parse();
-        Ptg[] ptgs = parser.getRPNPtg();
-        // -- parsing over --
-        
+        Ptg[] ptgs = FormulaParser.parse(cellFormulaText, workbook);
 
         Stack stack = new Stack();
         for (int i = 0, iSize = ptgs.length; i < iSize; i++) {
 
             // since we don't know how to handle these yet :(
             Ptg ptg = ptgs[i];
-            if (ptg instanceof ControlPtg) { continue; }
+            if (ptg instanceof ControlPtg) {
+                // skip Parentheses, Attr, etc
+                continue;
+            }
             if (ptg instanceof MemErrPtg) { continue; }
             if (ptg instanceof MissingArgPtg) { continue; }
             if (ptg instanceof NamePtg) { 
-            	// named ranges, macro functions
+                // named ranges, macro functions
                 NamePtg namePtg = (NamePtg) ptg;
                 stack.push(new NameEval(namePtg.getIndex()));
                 continue; 
             }
             if (ptg instanceof NameXPtg) {
-            	// TODO - external functions
+                // TODO - external functions
                 continue;
             }
             if (ptg instanceof UnknownPtg) { continue; }
@@ -362,9 +358,6 @@ public class FormulaEvaluator {
             if (ptg instanceof OperationPtg) {
                 OperationPtg optg = (OperationPtg) ptg;
 
-                // parens can be ignored since we have RPN tokens
-                if (optg instanceof ParenthesisPtg) { continue; }
-                if (optg instanceof AttrPtg) { continue; }
                 if (optg instanceof UnionPtg) { continue; }
 
                 OperationEval operation = OperationEvaluatorFactory.create(optg);

src/resources/main/org/apache/poi/hssf/record/formula/function/functionMetadata.txt

@@ -15,6 +15,7 @@
 
 # Created by (org.apache.poi.hssf.record.formula.function.ExcelFileFormatDocFunctionExtractor)
 # from source file 'excelfileformat.odt' (size=356107, md5=0x8f789cb6e75594caf068f8e193004ef4)
+#  ! + some manual edits !
 #
 #Columns: (index, name, minParams, maxParams, returnClass, paramClasses, isVolatile, hasFootnote )
 
@@ -78,8 +79,8 @@
 58	NPER	3	5	V	V V V V V		
 59	PMT	3	5	V	V V V V V		
 60	RATE	3	6	V	V V V V V V		
-61	MIRR	3	3	V	R V V		
-62	IRR	1	2	V	R V		
+61	MIRR	3	3	V	A V V		
+62	IRR	1	2	V	A V		
 63	RAND	0	0	V	-	x	
 64	MATCH	2	3	V	V R R		
 65	DATE	3	3	V	V V V		
@@ -93,8 +94,8 @@
 73	SECOND	1	1	V	V		
 74	NOW	0	0	V	-	x	
 75	AREAS	1	1	V	R		
-76	ROWS	1	1	V	R		
-77	COLUMNS	1	1	V	R		
+76	ROWS	1	1	V	A		
+77	COLUMNS	1	1	V	A		
 78	OFFSET	3	5	R	R V V V V	x	
 82	SEARCH	2	3	V	V V V		
 83	TRANSPOSE	1	1	A	A		

src/scratchpad/src/org/apache/poi/hssf/usermodel/HSSFChart.java

@@ -213,6 +213,23 @@ public class HSSFChart
     		charts.toArray( new HSSFChart[charts.size()] );
     }
 
+    /** Get the X offset of the chart */
+    public int getChartX() { return chartRecord.getX(); }
+    /** Get the Y offset of the chart */
+    public int getChartY() { return chartRecord.getY(); }
+    /** Get the width of the chart. {@link ChartRecord} */
+    public int getChartWidth() { return chartRecord.getWidth(); }
+    /** Get the height of the chart. {@link ChartRecord} */
+    public int getChartHeight() { return chartRecord.getHeight(); }
+
+    /** Sets the X offset of the chart */
+    public void setChartX(int x) { chartRecord.setX(x); }
+    /** Sets the Y offset of the chart */
+    public void setChartY(int y) { chartRecord.setY(y); }
+    /** Sets the width of the chart. {@link ChartRecord} */
+    public void setChartWidth(int width) { chartRecord.setWidth(width); }
+    /** Sets the height of the chart. {@link ChartRecord} */
+    public void setChartHeight(int height) { chartRecord.setHeight(height); }
     
     /**
      * Returns the series of the chart

src/scratchpad/testcases/org/apache/poi/hssf/usermodel/TestHSSFChart.java

@@ -53,6 +53,12 @@ public class TestHSSFChart extends TestCase {
 		assertEquals("1st Column", charts[0].getSeries()[0].getSeriesTitle());
 		assertEquals("2nd Column", charts[0].getSeries()[1].getSeriesTitle());
 		assertEquals(null, charts[0].getChartTitle());
+		
+		// Check x, y, width, height
+		assertEquals(0, charts[0].getChartX());
+		assertEquals(0, charts[0].getChartY());
+		assertEquals(26492928, charts[0].getChartWidth());
+		assertEquals(15040512, charts[0].getChartHeight());
 	}
 
 	public void testTwoCharts() throws Exception {

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

@@ -33,6 +33,9 @@ public final class AllModelTests {
 		result.addTestSuite(TestDrawingManager2.class);
 		result.addTestSuite(TestFormulaParser.class);
 		result.addTestSuite(TestFormulaParserEval.class);
+		result.addTestSuite(TestFormulaParserIf.class);
+		result.addTestSuite(TestOperandClassTransformer.class);
+		result.addTestSuite(TestRVA.class);
 		result.addTestSuite(TestSheet.class);
 		result.addTestSuite(TestSheetAdditional.class);
 		return result;

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

@@ -33,12 +33,9 @@ import org.apache.poi.hssf.record.formula.ErrPtg;
 import org.apache.poi.hssf.record.formula.FuncPtg;
 import org.apache.poi.hssf.record.formula.FuncVarPtg;
 import org.apache.poi.hssf.record.formula.IntPtg;
-import org.apache.poi.hssf.record.formula.LessEqualPtg;
-import org.apache.poi.hssf.record.formula.LessThanPtg;
 import org.apache.poi.hssf.record.formula.MissingArgPtg;
 import org.apache.poi.hssf.record.formula.MultiplyPtg;
 import org.apache.poi.hssf.record.formula.NamePtg;
-import org.apache.poi.hssf.record.formula.NotEqualPtg;
 import org.apache.poi.hssf.record.formula.NumberPtg;
 import org.apache.poi.hssf.record.formula.PercentPtg;
 import org.apache.poi.hssf.record.formula.PowerPtg;
@@ -62,10 +59,8 @@ public final class TestFormulaParser extends TestCase {
 	/**
 	 * @return parsed token array already confirmed not <code>null</code>
 	 */
-	private static Ptg[] parseFormula(String s) {
-		FormulaParser fp = new FormulaParser(s, null);
-		fp.parse();
-		Ptg[] result = fp.getRPNPtg();
+	/* package */ static Ptg[] parseFormula(String formula) {
+		Ptg[] result = FormulaParser.parse(formula, null);
 		assertNotNull("Ptg array should not be null", result);
 		return result;
 	}
@@ -105,83 +100,6 @@ public final class TestFormulaParser extends TestCase {
 		assertEquals(true, flag.getValue());
 	}
 
-	public void testYN() {
-		Ptg[] ptgs = parseFormula("IF(TRUE,\"Y\",\"N\")");
-		assertEquals(7, ptgs.length);
-
-		BoolPtg flag  = (BoolPtg) ptgs[0];
-		AttrPtg funif = (AttrPtg) ptgs[1];
-		StringPtg y = (StringPtg) ptgs[2];
-		AttrPtg goto1 = (AttrPtg) ptgs[3];
-		StringPtg n = (StringPtg) ptgs[4];
-
-
-		assertEquals(true, flag.getValue());
-		assertEquals("Y", y.getValue());
-		assertEquals("N", n.getValue());
-		assertEquals("IF", funif.toFormulaString((HSSFWorkbook) null));
-		assertTrue("Goto ptg exists", goto1.isGoto());
-	}
-
-	public void testSimpleIf() {
-		String formula = "IF(1=1,0,1)";
-
-		Class[] expectedClasses = {
-			IntPtg.class,
-			IntPtg.class,
-			EqualPtg.class,
-			AttrPtg.class,
-			IntPtg.class,
-			AttrPtg.class,
-			IntPtg.class,
-			AttrPtg.class,
-			FuncVarPtg.class,
-		};
-		confirmTokenClasses(formula, expectedClasses);
-		
-		Ptg[] ptgs = parseFormula(formula);
-
-		AttrPtg ifPtg = (AttrPtg) ptgs[3];
-		AttrPtg ptgGoto= (AttrPtg) ptgs[5];
-		assertEquals("Goto 1 Length", 10, ptgGoto.getData());
-
-		AttrPtg ptgGoto2 = (AttrPtg) ptgs[7];
-		assertEquals("Goto 2 Length", 3, ptgGoto2.getData());
-		assertEquals("If FALSE offset", 7, ifPtg.getData());
-	}
-
-	/**
-	 * Make sure the ptgs are generated properly with two functions embedded
-	 *
-	 */
-	public void testNestedFunctionIf() {
-		Ptg[] ptgs = parseFormula("IF(A1=B1,AVERAGE(A1:B1),AVERAGE(A2:B2))");
-		assertEquals(11, ptgs.length);
-
-		assertTrue("IF Attr set correctly", (ptgs[3] instanceof AttrPtg));
-		AttrPtg ifFunc = (AttrPtg)ptgs[3];
-		assertTrue("It is not an if", ifFunc.isOptimizedIf());
-
-		assertTrue("Average Function set correctly", (ptgs[5] instanceof FuncVarPtg));
-	}
-
-	public void testIfSingleCondition(){
-		Ptg[] ptgs = parseFormula("IF(1=1,10)");
-		assertEquals(7, ptgs.length);
-
-		assertTrue("IF Attr set correctly", (ptgs[3] instanceof AttrPtg));
-		AttrPtg ifFunc = (AttrPtg)ptgs[3];
-		assertTrue("It is not an if", ifFunc.isOptimizedIf());
-
-		assertTrue("Single Value is not an IntPtg", (ptgs[4] instanceof IntPtg));
-		IntPtg intPtg = (IntPtg)ptgs[4];
-		assertEquals("Result", (short)10, intPtg.getValue());
-
-		assertTrue("Ptg is not a Variable Function", (ptgs[6] instanceof FuncVarPtg));
-		FuncVarPtg funcPtg = (FuncVarPtg)ptgs[6];
-		assertEquals("Arguments", 2, funcPtg.getNumberOfOperands());
-	}
-
 	public void testSumIf() {
 		Ptg[] ptgs = parseFormula("SUMIF(A1:A5,\">4000\",B1:B5)");
 		assertEquals(4, ptgs.length);
@@ -203,33 +121,9 @@ public final class TestFormulaParser extends TestCase {
 		//the PTG order isn't 100% correct but it still works - dmui
 	}
 
-	public void testSimpleLogical() {
-	 Ptg[] ptgs = parseFormula("IF(A1<A2,B1,B2)");
-	 assertEquals(9, ptgs.length);
-	 assertEquals("3rd Ptg is less than", LessThanPtg.class, ptgs[2].getClass());
-	}
-
-	public void testParenIf() {
-		Ptg[] ptgs = parseFormula("IF((A1+A2)<=3,\"yes\",\"no\")");
-		assertEquals(12, ptgs.length);
-		assertEquals("6th Ptg is less than equal",LessEqualPtg.class,ptgs[5].getClass());
-		assertEquals("11th Ptg is not a goto (Attr) ptg",AttrPtg.class,ptgs[10].getClass());
-	}
-
-	public void testEmbeddedIf() {
-		Ptg[] ptgs = parseFormula("IF(3>=1,\"*\",IF(4<>1,\"first\",\"second\"))");
-		assertEquals(17, ptgs.length);
-
-		assertEquals("6th Ptg is not a goto (Attr) ptg",AttrPtg.class,ptgs[5].getClass());
-		assertEquals("9th Ptg is not a not equal ptg",NotEqualPtg.class,ptgs[8].getClass());
-		assertEquals("15th Ptg is not the inner IF variable function ptg",FuncVarPtg.class,ptgs[14].getClass());
-	}
-
 	public void testMacroFunction() {
 		HSSFWorkbook w = new HSSFWorkbook();
-		FormulaParser fp = new FormulaParser("FOO()", w);
-		fp.parse();
-		Ptg[] ptg = fp.getRPNPtg();
+		Ptg[] ptg = FormulaParser.parse("FOO()", w);
 
 		// the name gets encoded as the first arg
 		NamePtg tname = (NamePtg) ptg[0];
@@ -597,7 +491,7 @@ public final class TestFormulaParser extends TestCase {
 		confirmTokenClasses("2^200%", expClss);
 	}
 
-	private static void confirmTokenClasses(String formula, Class[] expectedClasses) {
+	/* package */ static Ptg[] confirmTokenClasses(String formula, Class[] expectedClasses) {
 		Ptg[] ptgs = parseFormula(formula);
 		assertEquals(expectedClasses.length, ptgs.length);
 		for (int i = 0; i < expectedClasses.length; i++) {
@@ -607,6 +501,7 @@ public final class TestFormulaParser extends TestCase {
 					+ ptgs[i].getClass().getName() + ")");
 			}
 		}
+		return ptgs;
 	}
 
 	public void testPower() {
@@ -644,8 +539,16 @@ public final class TestFormulaParser extends TestCase {
 
 		Class[] expClss;
 
-		expClss = new Class[] { ReferencePtg.class, MissingArgPtg.class, ReferencePtg.class,
-				FuncVarPtg.class, };
+		expClss = new Class[] { 
+				ReferencePtg.class, 
+				AttrPtg.class, // tAttrIf
+				MissingArgPtg.class, 
+				AttrPtg.class, // tAttrSkip
+				ReferencePtg.class,
+				AttrPtg.class, // tAttrSkip
+				FuncVarPtg.class, 
+		};
+
 		confirmTokenClasses("if(A1, ,C1)", expClss);
 
 		expClss = new Class[] { MissingArgPtg.class, AreaPtg.class, MissingArgPtg.class,
@@ -814,7 +717,7 @@ public final class TestFormulaParser extends TestCase {
 			fail("Expected exception was not thrown");
 		} catch (IllegalStateException e) {
 			// expected during successful test
-			assertTrue(e.getMessage().startsWith("Too few arguments suppled to operation token"));
+			assertTrue(e.getMessage().startsWith("Too few arguments supplied to operation"));
 		}
 	}
 	/**

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

@@ -0,0 +1,239 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.model;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.record.formula.AddPtg;
+import org.apache.poi.hssf.record.formula.AttrPtg;
+import org.apache.poi.hssf.record.formula.BoolPtg;
+import org.apache.poi.hssf.record.formula.FuncPtg;
+import org.apache.poi.hssf.record.formula.FuncVarPtg;
+import org.apache.poi.hssf.record.formula.IntPtg;
+import org.apache.poi.hssf.record.formula.LessEqualPtg;
+import org.apache.poi.hssf.record.formula.LessThanPtg;
+import org.apache.poi.hssf.record.formula.MultiplyPtg;
+import org.apache.poi.hssf.record.formula.NotEqualPtg;
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.ReferencePtg;
+import org.apache.poi.hssf.record.formula.StringPtg;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+
+/**
+ * Tests <tt>FormulaParser</tt> specifically with respect to IF() functions
+ */
+public final class TestFormulaParserIf extends TestCase {
+	private static Ptg[] parseFormula(String formula) {
+		return TestFormulaParser.parseFormula(formula);
+	}
+	
+	private static Ptg[] confirmTokenClasses(String formula, Class[] expectedClasses) {
+		return TestFormulaParser.confirmTokenClasses(formula, expectedClasses);
+	}
+	
+	private static void confirmAttrData(Ptg[] ptgs, int i, int expectedData) {
+		Ptg ptg = ptgs[i];
+		if (!(ptg instanceof AttrPtg)) {
+			throw new AssertionFailedError("Token[" + i + "] was not AttrPtg as expected");
+		}
+		AttrPtg attrPtg = (AttrPtg) ptg;
+		assertEquals(expectedData, attrPtg.getData());
+	}
+	
+	public void testSimpleIf() {
+		
+		Class[] expClss;
+
+		expClss = new Class[] {
+				ReferencePtg.class,
+				AttrPtg.class, // tAttrIf
+				IntPtg.class,
+				AttrPtg.class, // tAttrSkip
+				IntPtg.class,
+				AttrPtg.class, // tAttrSkip
+				FuncVarPtg.class,
+		};
+
+		Ptg[] ptgs = confirmTokenClasses("if(A1,1,2)", expClss);
+
+		confirmAttrData(ptgs, 1, 7);
+		confirmAttrData(ptgs, 3, 10);
+		confirmAttrData(ptgs, 5, 3);
+	}
+
+	public void testSimpleIfNoFalseParam() {
+		
+		Class[] expClss;
+
+		expClss = new Class[] {
+				ReferencePtg.class,
+				AttrPtg.class, // tAttrIf
+				ReferencePtg.class,
+				AttrPtg.class, // tAttrSkip
+				FuncVarPtg.class,
+		};
+
+		Ptg[] ptgs = confirmTokenClasses("if(A1,B1)", expClss);
+
+		confirmAttrData(ptgs, 1, 9);
+		confirmAttrData(ptgs, 3, 3);
+	}
+
+	public void testIfWithLargeParams() {
+		
+		Class[] expClss;
+
+		expClss = new Class[] {
+				ReferencePtg.class,
+				AttrPtg.class, // tAttrIf
+
+				ReferencePtg.class,
+				IntPtg.class,
+				MultiplyPtg.class,
+				ReferencePtg.class,
+				IntPtg.class,
+				AddPtg.class,
+				FuncPtg.class,
+				AttrPtg.class, // tAttrSkip
+				
+				ReferencePtg.class,
+				ReferencePtg.class,
+				FuncPtg.class,
+				
+				AttrPtg.class, // tAttrSkip
+				FuncVarPtg.class,
+		};
+
+		Ptg[] ptgs = confirmTokenClasses("if(A1,round(B1*100,C1+2),round(B1,C1))", expClss);
+
+		confirmAttrData(ptgs, 1, 25);
+		confirmAttrData(ptgs, 9, 20);
+		confirmAttrData(ptgs, 13, 3);
+	}
+	
+	public void testNestedIf() {
+		
+		Class[] expClss;
+
+		expClss = new Class[] {
+
+				ReferencePtg.class,
+				AttrPtg.class,	  // A tAttrIf
+				ReferencePtg.class,
+				AttrPtg.class,    //   B tAttrIf
+				IntPtg.class,
+				AttrPtg.class,    //   B tAttrSkip
+				IntPtg.class,
+				AttrPtg.class,    //   B tAttrSkip
+				FuncVarPtg.class,
+				AttrPtg.class,    // A tAttrSkip
+				ReferencePtg.class,
+				AttrPtg.class,    //   C tAttrIf
+				IntPtg.class,
+				AttrPtg.class,    //   C tAttrSkip
+				IntPtg.class,
+				AttrPtg.class,    //   C tAttrSkip
+				FuncVarPtg.class,
+				AttrPtg.class,    // A tAttrSkip
+				FuncVarPtg.class,
+		};
+
+		Ptg[] ptgs = confirmTokenClasses("if(A1,if(B1,1,2),if(C1,3,4))", expClss);
+		confirmAttrData(ptgs, 1, 31);
+		confirmAttrData(ptgs, 3, 7);
+		confirmAttrData(ptgs, 5, 10);
+		confirmAttrData(ptgs, 7, 3);
+		confirmAttrData(ptgs, 9, 34);
+		confirmAttrData(ptgs, 11, 7);
+		confirmAttrData(ptgs, 13, 10);
+		confirmAttrData(ptgs, 15, 3);
+		confirmAttrData(ptgs, 17, 3);
+	}
+	
+	public void testEmbeddedIf() {
+		Ptg[] ptgs = parseFormula("IF(3>=1,\"*\",IF(4<>1,\"first\",\"second\"))");
+		assertEquals(17, ptgs.length);
+
+		assertEquals("6th Ptg is not a goto (Attr) ptg",AttrPtg.class,ptgs[5].getClass());
+		assertEquals("9th Ptg is not a not equal ptg",NotEqualPtg.class,ptgs[8].getClass());
+		assertEquals("15th Ptg is not the inner IF variable function ptg",FuncVarPtg.class,ptgs[14].getClass());
+	}
+
+
+	public void testSimpleLogical() {
+	 Ptg[] ptgs = parseFormula("IF(A1<A2,B1,B2)");
+	 assertEquals(9, ptgs.length);
+	 assertEquals("3rd Ptg is less than", LessThanPtg.class, ptgs[2].getClass());
+	}
+
+	public void testParenIf() {
+		Ptg[] ptgs = parseFormula("IF((A1+A2)<=3,\"yes\",\"no\")");
+		assertEquals(12, ptgs.length);
+		assertEquals("6th Ptg is less than equal",LessEqualPtg.class,ptgs[5].getClass());
+		assertEquals("11th Ptg is not a goto (Attr) ptg",AttrPtg.class,ptgs[10].getClass());
+	}
+	public void testYN() {
+		Ptg[] ptgs = parseFormula("IF(TRUE,\"Y\",\"N\")");
+		assertEquals(7, ptgs.length);
+
+		BoolPtg flag  = (BoolPtg) ptgs[0];
+		AttrPtg funif = (AttrPtg) ptgs[1];
+		StringPtg y = (StringPtg) ptgs[2];
+		AttrPtg goto1 = (AttrPtg) ptgs[3];
+		StringPtg n = (StringPtg) ptgs[4];
+
+
+		assertEquals(true, flag.getValue());
+		assertEquals("Y", y.getValue());
+		assertEquals("N", n.getValue());
+		assertEquals("IF", funif.toFormulaString((HSSFWorkbook) null));
+		assertTrue("Goto ptg exists", goto1.isGoto());
+	}
+	/**
+	 * Make sure the ptgs are generated properly with two functions embedded
+	 *
+	 */
+	public void testNestedFunctionIf() {
+		Ptg[] ptgs = parseFormula("IF(A1=B1,AVERAGE(A1:B1),AVERAGE(A2:B2))");
+		assertEquals(11, ptgs.length);
+
+		assertTrue("IF Attr set correctly", (ptgs[3] instanceof AttrPtg));
+		AttrPtg ifFunc = (AttrPtg)ptgs[3];
+		assertTrue("It is not an if", ifFunc.isOptimizedIf());
+
+		assertTrue("Average Function set correctly", (ptgs[5] instanceof FuncVarPtg));
+	}
+
+	public void testIfSingleCondition(){
+		Ptg[] ptgs = parseFormula("IF(1=1,10)");
+		assertEquals(7, ptgs.length);
+
+		assertTrue("IF Attr set correctly", (ptgs[3] instanceof AttrPtg));
+		AttrPtg ifFunc = (AttrPtg)ptgs[3];
+		assertTrue("It is not an if", ifFunc.isOptimizedIf());
+
+		assertTrue("Single Value is not an IntPtg", (ptgs[4] instanceof IntPtg));
+		IntPtg intPtg = (IntPtg)ptgs[4];
+		assertEquals("Result", (short)10, intPtg.getValue());
+
+		assertTrue("Ptg is not a Variable Function", (ptgs[6] instanceof FuncVarPtg));
+		FuncVarPtg funcPtg = (FuncVarPtg)ptgs[6];
+		assertEquals("Arguments", 2, funcPtg.getNumberOfOperands());
+	}
+}

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

@@ -0,0 +1,110 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.model;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.record.formula.AbstractFunctionPtg;
+import org.apache.poi.hssf.record.formula.FuncVarPtg;
+import org.apache.poi.hssf.record.formula.Ptg;
+
+/**
+ * Tests specific formula examples in <tt>OperandClassTransformer</tt>.
+ * 
+ * @author Josh Micich
+ */
+public final class TestOperandClassTransformer extends TestCase {
+
+	public void testMdeterm() {
+		String formula = "MDETERM(ABS(A1))";
+		Ptg[] ptgs = FormulaParser.parse(formula, null);
+
+		confirmTokenClass(ptgs, 0, Ptg.CLASS_ARRAY);
+		confirmFuncClass(ptgs, 1, "ABS", Ptg.CLASS_ARRAY);
+		confirmFuncClass(ptgs, 2, "MDETERM", Ptg.CLASS_VALUE);
+	}
+
+	/**
+	 * In the example: <code>INDEX(PI(),1)</code>, Excel encodes PI() as 'array'.  It is not clear
+	 * what rule justifies this. POI currently encodes it as 'value' which Excel(2007) seems to 
+	 * tolerate. Changing the metadata for INDEX to have first parameter as 'array' class breaks 
+	 * other formulas involving INDEX.  It seems like a special case needs to be made.  Perhaps an 
+	 * important observation is that INDEX is one of very few functions that returns 'reference' type.
+	 * 
+	 * This test has been added but disabled in order to document this issue.
+	 */
+	public void DISABLED_testIndexPi1() {
+		String formula = "INDEX(PI(),1)";
+		Ptg[] ptgs = FormulaParser.parse(formula, null);
+
+		confirmFuncClass(ptgs, 1, "PI", Ptg.CLASS_ARRAY); // fails as of POI 3.1
+		confirmFuncClass(ptgs, 2, "INDEX", Ptg.CLASS_VALUE);
+	}
+
+	public void testComplexIRR_bug45041() {
+		String formula = "(1+IRR(SUMIF(A:A,ROW(INDIRECT(MIN(A:A)&\":\"&MAX(A:A))),B:B),0))^365-1";
+		Ptg[] ptgs = FormulaParser.parse(formula, null);
+
+		FuncVarPtg rowFunc = (FuncVarPtg) ptgs[10];
+		FuncVarPtg sumifFunc = (FuncVarPtg) ptgs[12];
+		assertEquals("ROW", rowFunc.getName());
+		assertEquals("SUMIF", sumifFunc.getName());
+
+		if (rowFunc.getPtgClass() == Ptg.CLASS_VALUE || sumifFunc.getPtgClass() == Ptg.CLASS_VALUE) {
+			throw new AssertionFailedError("Identified bug 45041");
+		}
+		confirmTokenClass(ptgs, 1, Ptg.CLASS_REF);
+		confirmTokenClass(ptgs, 2, Ptg.CLASS_REF);
+		confirmFuncClass(ptgs, 3, "MIN", Ptg.CLASS_VALUE);
+		confirmTokenClass(ptgs, 6, Ptg.CLASS_REF);
+		confirmFuncClass(ptgs, 7, "MAX", Ptg.CLASS_VALUE);
+		confirmFuncClass(ptgs, 9, "INDIRECT", Ptg.CLASS_REF);
+		confirmFuncClass(ptgs, 10, "ROW", Ptg.CLASS_ARRAY);
+		confirmTokenClass(ptgs, 11, Ptg.CLASS_REF);
+		confirmFuncClass(ptgs, 12, "SUMIF", Ptg.CLASS_ARRAY);
+		confirmFuncClass(ptgs, 14, "IRR", Ptg.CLASS_VALUE);
+	}
+
+	private void confirmFuncClass(Ptg[] ptgs, int i, String expectedFunctionName, byte operandClass) {
+		confirmTokenClass(ptgs, i, operandClass);
+		AbstractFunctionPtg afp = (AbstractFunctionPtg) ptgs[i];
+		assertEquals(expectedFunctionName, afp.getName());
+	}
+
+	private void confirmTokenClass(Ptg[] ptgs, int i, byte operandClass) {
+		Ptg ptg = ptgs[i];
+		if (operandClass != ptg.getPtgClass()) {
+			throw new AssertionFailedError("Wrong operand class for function ptg ("
+					+ ptg.toString() + "). Expected " + getOperandClassName(operandClass)
+					+ " but got " + getOperandClassName(ptg.getPtgClass()));
+		}
+	}
+
+	private static String getOperandClassName(byte ptgClass) {
+		switch (ptgClass) {
+			case Ptg.CLASS_REF:
+				return "R";
+			case Ptg.CLASS_VALUE:
+				return "V";
+			case Ptg.CLASS_ARRAY:
+				return "A";
+		}
+		throw new RuntimeException("Unknown operand class (" + ptgClass + ")");
+	}
+}

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

@@ -0,0 +1,156 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.model;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.hssf.record.formula.AttrPtg;
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.ReferencePtg;
+import org.apache.poi.hssf.usermodel.FormulaExtractor;
+import org.apache.poi.hssf.usermodel.HSSFCell;
+import org.apache.poi.hssf.usermodel.HSSFRow;
+import org.apache.poi.hssf.usermodel.HSSFSheet;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+
+/**
+ * Tests 'operand class' transformation performed by
+ * <tt>OperandClassTransformer</tt> by comparing its results with those
+ * directly produced by Excel (in a sample spreadsheet).
+ * 
+ * @author Josh Micich
+ */
+public final class TestRVA extends TestCase {
+
+	private static final String NEW_LINE = System.getProperty("line.separator");
+
+	public void testFormulas() {
+		HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("testRVA.xls");
+		HSSFSheet sheet = wb.getSheetAt(0);
+
+		int countFailures = 0;
+		int countErrors = 0;
+
+		int rowIx = 0;
+		while (rowIx < 65535) {
+			HSSFRow row = sheet.getRow(rowIx);
+			if (row == null) {
+				break;
+			}
+			HSSFCell cell = row.getCell(0);
+			if (cell == null || cell.getCellType() == HSSFCell.CELL_TYPE_BLANK) {
+				break;
+			}
+			String formula = cell.getCellFormula();
+			try {
+				confirmCell(cell, formula);
+			} catch (AssertionFailedError e) {
+				System.err.println("Problem with row[" + rowIx + "] formula '" + formula + "'");
+				System.err.println(e.getMessage());
+				countFailures++;
+			} catch (RuntimeException e) {
+				System.err.println("Problem with row[" + rowIx + "] formula '" + formula + "'");
+				countErrors++;
+				e.printStackTrace();
+			}
+			rowIx++;
+		}
+		if (countErrors + countFailures > 0) {
+			String msg = "One or more RVA tests failed: countFailures=" + countFailures
+					+ " countFailures=" + countErrors + ". See stderr for details.";
+			throw new AssertionFailedError(msg);
+		}
+	}
+
+	private void confirmCell(HSSFCell formulaCell, String formula) {
+		Ptg[] excelPtgs = FormulaExtractor.getPtgs(formulaCell);
+		Ptg[] poiPtgs = FormulaParser.parse(formula, null);
+		int nExcelTokens = excelPtgs.length;
+		int nPoiTokens = poiPtgs.length;
+		if (nExcelTokens != nPoiTokens) {
+			if (nExcelTokens == nPoiTokens + 1 && excelPtgs[0].getClass() == AttrPtg.class) {
+				// compensate for missing tAttrVolatile, which belongs in any formula 
+				// involving OFFSET() et al. POI currently does not insert where required
+				Ptg[] temp = new Ptg[nExcelTokens];
+				temp[0] = excelPtgs[0];
+				System.arraycopy(poiPtgs, 0, temp, 1, nPoiTokens);
+				poiPtgs = temp;
+			} else {
+				throw new RuntimeException("Expected " + nExcelTokens + " tokens but got "
+						+ nPoiTokens);
+			}
+		}
+		boolean hasMismatch = false;
+		StringBuffer sb = new StringBuffer();
+		for (int i = 0; i < nExcelTokens; i++) {
+			Ptg poiPtg = poiPtgs[i];
+			Ptg excelPtg = excelPtgs[i];
+			if (!areTokenClassesSame(poiPtg, excelPtg)) {
+				hasMismatch = true;
+				sb.append("  mismatch token type[" + i + "] " + getShortClassName(excelPtg) + " "
+						+ getOperandClassName(excelPtg) + " - " + getShortClassName(poiPtg) + " "
+						+ getOperandClassName(poiPtg));
+				sb.append(NEW_LINE);
+				continue;
+			}
+			if (poiPtg.isBaseToken()) {
+				continue;
+			}
+			sb.append("  token[" + i + "] " + excelPtg.toString() + " "
+					+ getOperandClassName(excelPtg));
+
+			if (excelPtg.getPtgClass() != poiPtg.getPtgClass()) {
+				hasMismatch = true;
+				sb.append(" - was " + getOperandClassName(poiPtg));
+			}
+			sb.append(NEW_LINE);
+		}
+		if (hasMismatch) {
+			throw new AssertionFailedError(sb.toString());
+		}
+	}
+
+	private boolean areTokenClassesSame(Ptg poiPtg, Ptg excelPtg) {
+		if (excelPtg.getClass() == poiPtg.getClass()) {
+			return true;
+		}
+		if (poiPtg.getClass() == ReferencePtg.class) {
+			// TODO - remove funny subclasses of ReferencePtg
+			return excelPtg instanceof ReferencePtg;
+		}
+		return false;
+	}
+
+	private String getShortClassName(Object o) {
+		String cn = o.getClass().getName();
+		int pos = cn.lastIndexOf('.');
+		return cn.substring(pos + 1);
+	}
+
+	private static String getOperandClassName(Ptg ptg) {
+		byte ptgClass = ptg.getPtgClass();
+		switch (ptgClass) {
+			case Ptg.CLASS_REF:   return "R";
+			case Ptg.CLASS_VALUE: return "V";
+			case Ptg.CLASS_ARRAY: return "A";
+		}
+		throw new RuntimeException("Unknown operand class (" + ptgClass + ")");
+	}
+}

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

@@ -19,8 +19,10 @@ package org.apache.poi.hssf.record.formula;
 
 import java.util.Arrays;
 
+import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.record.TestcaseRecordInputStream;
 import org.apache.poi.hssf.record.UnicodeString;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
@@ -77,7 +79,7 @@ public final class TestArrayPtg extends TestCase {
 	}
 
 	/**
-	 * make sure constant elements are stored row by row 
+	 * Excel stores array elements column by column.  This test makes sure POI does the same.
 	 */
 	public void testElementOrdering() {
 		ArrayPtg ptg = new ArrayPtgV(new TestcaseRecordInputStream(ArrayPtgV.sid, ENCODED_PTG_DATA));
@@ -86,10 +88,27 @@ public final class TestArrayPtg extends TestCase {
 		assertEquals(2, ptg.getRowCount());
 		
 		assertEquals(0, ptg.getValueIndex(0, 0));
-		assertEquals(1, ptg.getValueIndex(1, 0));
-		assertEquals(2, ptg.getValueIndex(2, 0));
-		assertEquals(3, ptg.getValueIndex(0, 1));
-		assertEquals(4, ptg.getValueIndex(1, 1));
+		assertEquals(2, ptg.getValueIndex(1, 0));
+		assertEquals(4, ptg.getValueIndex(2, 0));
+		assertEquals(1, ptg.getValueIndex(0, 1));
+		assertEquals(3, ptg.getValueIndex(1, 1));
 		assertEquals(5, ptg.getValueIndex(2, 1));
 	}
+	
+	/**
+	 * Test for a bug which was temporarily introduced by the fix for bug 42564.
+	 * A spreadsheet was added to make the ordering clearer.
+	 */
+	public void testElementOrderingInSpreadsheet() {
+		HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex42564-elementOrder.xls");
+
+		// The formula has an array with 3 rows and 5 column 
+		String formula = wb.getSheetAt(0).getRow(0).getCell((short)0).getCellFormula();
+		// TODO - These number literals should not have '.0'. Excel has different number rendering rules
+
+		if (formula.equals("SUM({1.0,6.0,11.0;2.0,7.0,12.0;3.0,8.0,13.0;4.0,9.0,14.0;5.0,10.0,15.0})")) {
+			throw new AssertionFailedError("Identified bug 42564 b");
+		}
+		assertEquals("SUM({1.0,2.0,3.0;4.0,5.0,6.0;7.0,8.0,9.0;10.0,11.0,12.0;13.0,14.0,15.0})", formula);
+	}
 }

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

@@ -0,0 +1,49 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.usermodel;
+
+import java.util.List;
+
+import org.apache.poi.hssf.record.CellValueRecordInterface;
+import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate;
+import org.apache.poi.hssf.record.formula.Ptg;
+
+/**
+ * Test utility class to get <tt>Ptg</tt> arrays out of formula cells
+ * 
+ * @author Josh Micich
+ */
+public final class FormulaExtractor {
+
+	private FormulaExtractor() {
+		// no instances of this class
+	}
+	
+	public static Ptg[] getPtgs(HSSFCell cell) {
+		CellValueRecordInterface vr = cell.getCellValueRecord();
+		if (!(vr instanceof FormulaRecordAggregate)) {
+			throw new IllegalArgumentException("Not a formula cell");
+		}
+		FormulaRecordAggregate fra = (FormulaRecordAggregate) vr;
+		List tokens = fra.getFormulaRecord().getParsedExpression();
+		Ptg[] result = new Ptg[tokens.size()];
+		tokens.toArray(result);
+		return result;
+	}
+
+}

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

@@ -18,9 +18,11 @@
 package org.apache.poi.hssf.usermodel;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.Iterator;
+import java.util.List;
 
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
@@ -28,6 +30,7 @@ import junit.framework.TestCase;
 import org.apache.poi.ss.util.Region;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.hssf.record.EmbeddedObjectRefSubRecord;
 import org.apache.poi.util.TempFile;
 
 /**
@@ -951,4 +954,40 @@ public final class TestBugs extends TestCase {
         writeOutAndReadBack(wb);
         assertTrue("no errors writing sample xls", true);
     }
+    
+    /**
+     * Problems with extracting check boxes from
+     *  HSSFObjectData
+     * @throws Exception
+     */
+    public void test44840() throws Exception {
+        HSSFWorkbook wb = openSample("WithCheckBoxes.xls");
+
+        // Take a look at the embeded objects
+        List objects = wb.getAllEmbeddedObjects();
+        assertEquals(1, objects.size());
+        
+        HSSFObjectData obj = (HSSFObjectData)objects.get(0);
+        assertNotNull(obj);
+        
+        // Peek inside the underlying record
+        EmbeddedObjectRefSubRecord rec = obj.findObjectRecord();
+        assertNotNull(rec);
+        
+        assertEquals(32, rec.field_1_stream_id_offset);
+        assertEquals(0, rec.field_6_stream_id); // WRONG!
+        assertEquals("Forms.CheckBox.1", rec.field_5_ole_classname);
+        assertEquals(12, rec.remainingBytes.length);
+        
+        // Doesn't have a directory
+        assertFalse(obj.hasDirectoryEntry());
+        assertNotNull(obj.getObjectData());
+        assertEquals(12, obj.getObjectData().length);
+        assertEquals("Forms.CheckBox.1", obj.getOLE2ClassName());
+        
+        try {
+        	obj.getDirectory();
+        	fail();
+        } catch(FileNotFoundException e) {}
+    }
 }

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

@@ -257,9 +257,15 @@ public class TestHSSFDateUtil extends TestCase {
                 // (who knows what they mean though...)
                 "[$-F800]dddd\\,\\ mmm\\ dd\\,\\ yyyy",
                 "[$-F900]ddd/mm/yyy",
+                // These ones specify colours, who knew that was allowed?
+                "[BLACK]dddd/mm/yy",
+                "[yeLLow]yyyy-mm-dd"
         };
         for(int i=0; i<formats.length; i++) {
-            assertTrue( HSSFDateUtil.isADateFormat(formatId, formats[i]) );
+            assertTrue(
+            		formats[i] + " is a date format", 
+            		HSSFDateUtil.isADateFormat(formatId, formats[i])
+            );
         }
         
         // Then time based ones too
@@ -270,7 +276,10 @@ public class TestHSSFDateUtil extends TestCase {
                 "mm/dd HH:MM PM", "mm/dd HH:MM pm" 
         };
         for(int i=0; i<formats.length; i++) {
-            assertTrue( HSSFDateUtil.isADateFormat(formatId, formats[i]) );
+            assertTrue(
+            		formats[i] + " is a datetime format", 
+            		HSSFDateUtil.isADateFormat(formatId, formats[i])
+            );
         }
         
         // Then invalid ones
@@ -278,10 +287,14 @@ public class TestHSSFDateUtil extends TestCase {
                 "yyyy*mm*dd", 
                 "0.0", "0.000",
                 "0%", "0.0%",
+                "[]Foo", "[BLACK]0.00%",
                 "", null
         };
         for(int i=0; i<formats.length; i++) {
-            assertFalse( HSSFDateUtil.isADateFormat(formatId, formats[i]) );
+            assertFalse( 
+            		formats[i] + " is not a date or datetime format",
+            		HSSFDateUtil.isADateFormat(formatId, formats[i])
+            );
         }
         
         // And these are ones we probably shouldn't allow,