]> source.dussan.org Git - poi.git/commitdiff
Made AbstractFunctionPtg immutable, other minor improvements
authorJosh Micich <josh@apache.org>
Sat, 14 Nov 2009 02:41:24 +0000 (02:41 +0000)
committerJosh Micich <josh@apache.org>
Sat, 14 Nov 2009 02:41:24 +0000 (02:41 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@836101 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/hssf/record/formula/AbstractFunctionPtg.java
src/java/org/apache/poi/hssf/record/formula/FuncPtg.java
src/java/org/apache/poi/hssf/record/formula/FuncVarPtg.java
src/java/org/apache/poi/hssf/record/formula/Ptg.java
src/java/org/apache/poi/ss/formula/FormulaParser.java
src/java/org/apache/poi/ss/formula/OperandClassTransformer.java
src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java
src/testcases/org/apache/poi/hssf/record/formula/TestFuncPtg.java
src/testcases/org/apache/poi/hssf/record/formula/eval/TestRangeEval.java

index 8e1f9ffe939051d59576867d7fbeb84d411009c4..c6a0dedd20313d317770b77defad3d8770ab3cc6 100644 (file)
@@ -37,45 +37,55 @@ public abstract class AbstractFunctionPtg extends OperationPtg {
     /** All external functions have function index 255 */
     private static final short FUNCTION_INDEX_EXTERNAL = 255;
 
-    protected byte returnClass;
-    protected byte[] paramClass;
+    private final byte returnClass;
+    private final byte[] paramClass;
 
-    protected byte field_1_num_args;
-    protected short field_2_fnc_index;
+    private final byte _numberOfArgs;
+    private final short _functionIndex;
 
+    protected AbstractFunctionPtg(int functionIndex, int pReturnClass, byte[] paramTypes, int nParams) {
+        _numberOfArgs = (byte) nParams;
+        _functionIndex = (short) functionIndex;
+        returnClass = (byte) pReturnClass;
+        paramClass = paramTypes;
+    }
     public final boolean isBaseToken() {
-       return false;
+        return false;
     }
-    
-    public String toString() {
-        StringBuffer sb = new StringBuffer(64);
+
+    public final String toString() {
+        StringBuilder sb = new StringBuilder(64);
         sb.append(getClass().getName()).append(" [");
-        sb.append(field_2_fnc_index).append(" ").append(field_1_num_args);
+        sb.append(lookupName(_functionIndex));
+        sb.append(" nArgs=").append(_numberOfArgs);
         sb.append("]");
         return sb.toString();
     }
 
-    public short getFunctionIndex() {
-        return field_2_fnc_index;
+    public final short getFunctionIndex() {
+        return _functionIndex;
+    }
+    public final int getNumberOfOperands() {
+        return _numberOfArgs;
     }
 
-    public String getName() {
-        return lookupName(field_2_fnc_index);
+    public final String getName() {
+        return lookupName(_functionIndex);
     }
     /**
      * external functions get some special processing
      * @return <code>true</code> if this is an external function
      */
-    public boolean isExternalFunction() {
-        return field_2_fnc_index == FUNCTION_INDEX_EXTERNAL;
+    public final boolean isExternalFunction() {
+        return _functionIndex == FUNCTION_INDEX_EXTERNAL;
     }
 
-    public String toFormulaString() {
+    public final String toFormulaString() {
         return getName();
     }
 
     public String toFormulaString(String[] operands) {
-        StringBuffer buf = new StringBuffer();
+        StringBuilder buf = new StringBuilder();
 
         if(isExternalFunction()) {
             buf.append(operands[0]); // first operand is actually the function name
@@ -87,7 +97,7 @@ public abstract class AbstractFunctionPtg extends OperationPtg {
         return buf.toString();
     }
 
-    private static void appendArgs(StringBuffer buf, int firstArgIx, String[] operands) {
+    private static void appendArgs(StringBuilder buf, int firstArgIx, String[] operands) {
         buf.append('(');
         for (int i=firstArgIx;i<operands.length;i++) {
             if (i>firstArgIx) {
@@ -113,7 +123,7 @@ public abstract class AbstractFunctionPtg extends OperationPtg {
         return ix >= 0;
     }
 
-    protected String lookupName(short index) {
+    protected final String lookupName(short index) {
         if(index == FunctionMetadataRegistry.FUNCTION_INDEX_EXTERNAL) {
             return "#external#";
         }
@@ -142,10 +152,10 @@ public abstract class AbstractFunctionPtg extends OperationPtg {
         return returnClass;
     }
 
-    public byte getParameterClass(int index) {
+    public final byte getParameterClass(int index) {
         if (index >= paramClass.length) {
             // For var-arg (and other?) functions, the metadata does not list all the parameter
-            // operand classes.  In these cases, all extra parameters are assumed to have the 
+            // operand classes.  In these cases, all extra parameters are assumed to have the
             // same operand class as the last one specified.
             return paramClass[paramClass.length - 1];
         }
index 133072203bf65b66b8d32a02bc0c32088d6dfd00..4a2ca1bfcb382819262423104301b349b882eecd 100644 (file)
@@ -16,6 +16,7 @@
 ==================================================================== */
 
 package org.apache.poi.hssf.record.formula;
+
 import org.apache.poi.hssf.record.formula.function.FunctionMetadata;
 import org.apache.poi.hssf.record.formula.function.FunctionMetadataRegistry;
 import org.apache.poi.util.LittleEndianInput;
@@ -30,50 +31,30 @@ public final class FuncPtg extends AbstractFunctionPtg {
 
     public final static byte sid  = 0x21;
     public final static int  SIZE = 3;
-    private int numParams=0;
 
-    /**Creates new function pointer from a byte array
-     * usually called while reading an excel file.
-     */
-    public FuncPtg(LittleEndianInput in) {
-        //field_1_num_args = data[ offset + 0 ];
-        field_2_fnc_index  = in.readShort();
+    public static FuncPtg create(LittleEndianInput in) {
+        return create(in.readUShort());
+    }
 
-        FunctionMetadata fm = FunctionMetadataRegistry.getFunctionByIndex(field_2_fnc_index);
-        if(fm == null) {
-            throw new RuntimeException("Invalid built-in function index (" + field_2_fnc_index + ")");
-        }
-        numParams = fm.getMinParams();
-        returnClass = fm.getReturnClassCode();
-        paramClass = fm.getParameterClassCodes();
+    private FuncPtg(int funcIndex, FunctionMetadata fm) {
+        super(funcIndex, fm.getReturnClassCode(), fm.getParameterClassCodes(), fm.getMinParams());  // minParams same as max since these are not var-arg funcs
     }
-    public FuncPtg(int functionIndex) {
-        field_2_fnc_index = (short) functionIndex;
+
+    public static FuncPtg create(int functionIndex) {
         FunctionMetadata fm = FunctionMetadataRegistry.getFunctionByIndex(functionIndex);
-        numParams = fm.getMinParams(); // same as max since these are not var-arg funcs
-        returnClass = fm.getReturnClassCode();
-        paramClass = fm.getParameterClassCodes();
+        if(fm == null) {
+            throw new RuntimeException("Invalid built-in function index (" + functionIndex + ")");
+        }
+        return new FuncPtg(functionIndex, fm);
     }
 
+
     public void write(LittleEndianOutput out) {
         out.writeByte(sid + getPtgClass());
-        out.writeShort(field_2_fnc_index);
-    }
-
-    public int getNumberOfOperands() {
-        return numParams;
+        out.writeShort(getFunctionIndex());
     }
 
     public int getSize() {
         return SIZE;
     }
-
-    public String toString() {
-        StringBuffer sb = new StringBuffer(64);
-        sb.append(getClass().getName()).append(" [");
-        sb.append(lookupName(field_2_fnc_index));
-        sb.append(" nArgs=").append(numParams);
-        sb.append("]");
-        return sb.toString();
-    }
-}
\ No newline at end of file
+}
index 1deb630d50b38fedc36956d51b612be583d58566..607a64f17a30121e55b1a250a8aed0c9317c9eea 100644 (file)
@@ -22,7 +22,6 @@ import org.apache.poi.util.LittleEndianInput;
 import org.apache.poi.util.LittleEndianOutput;
 
 /**
- *
  * @author Jason Height (jheight at chariot dot net dot au)
  */
 public final class FuncVarPtg extends AbstractFunctionPtg{
@@ -30,60 +29,45 @@ public final class FuncVarPtg extends AbstractFunctionPtg{
     public final static byte sid  = 0x22;
     private final static int  SIZE = 4;
 
+    /**
+     * Single instance of this token for 'sum() taking a single argument'
+     */
+    public static final OperationPtg SUM = FuncVarPtg.create("SUM", 1);
+
+    private FuncVarPtg(int functionIndex, int returnClass, byte[] paramClasses, int numArgs) {
+        super(functionIndex, returnClass, paramClasses, numArgs);
+    }
+
     /**Creates new function pointer from a byte array
      * usually called while reading an excel file.
      */
-    public FuncVarPtg(LittleEndianInput 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();
-        }
+    public static FuncVarPtg create(LittleEndianInput in)  {
+        return create(in.readByte(), in.readShort());
     }
 
     /**
      * Create a function ptg from a string tokenised by the parser
      */
-    public FuncVarPtg(String pName, byte pNumOperands) {
-        field_1_num_args = pNumOperands;
-        field_2_fnc_index = lookupIndex(pName);
-        FunctionMetadata fm = FunctionMetadataRegistry.getFunctionByIndex(field_2_fnc_index);
+    public static FuncVarPtg create(String pName, int numArgs) {
+        return create(numArgs, lookupIndex(pName));
+    }
+
+    private static FuncVarPtg create(int numArgs, int functionIndex) {
+        FunctionMetadata fm = FunctionMetadataRegistry.getFunctionByIndex(functionIndex);
         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();
+            return new FuncVarPtg(functionIndex, Ptg.CLASS_VALUE, new byte[] {Ptg.CLASS_VALUE}, numArgs);
         }
+        return new FuncVarPtg(functionIndex, fm.getReturnClassCode(), fm.getParameterClassCodes(), numArgs);
     }
 
     public void write(LittleEndianOutput out) {
         out.writeByte(sid + getPtgClass());
-        out.writeByte(field_1_num_args);
-        out.writeShort(field_2_fnc_index);
-    }
-
-     public int getNumberOfOperands() {
-        return field_1_num_args;
+        out.writeByte(getNumberOfOperands());
+        out.writeShort(getFunctionIndex());
     }
 
     public int getSize() {
         return SIZE;
     }
-
-    public String toString() {
-        StringBuffer sb = new StringBuffer(64);
-        sb.append(getClass().getName()).append(" [");
-        sb.append(lookupName(field_2_fnc_index));
-        sb.append(" nArgs=").append(field_1_num_args);
-        sb.append("]");
-        return sb.toString();
-    }
 }
index 140e366bb4d045ac69aca03416166de2703c0af8..7e2ad66a466f84bb2a91bccc9711215c74299fef 100644 (file)
@@ -102,8 +102,8 @@ public abstract class Ptg implements Cloneable {
 
                switch (baseId) {
                        case ArrayPtg.sid:    return new ArrayPtg(in);    // 0x20, 0x40, 0x60
-                       case FuncPtg.sid:     return new FuncPtg(in);     // 0x21, 0x41, 0x61
-                       case FuncVarPtg.sid:  return new FuncVarPtg(in);  // 0x22, 0x42, 0x62
+                       case FuncPtg.sid:     return FuncPtg.create(in);  // 0x21, 0x41, 0x61
+                       case FuncVarPtg.sid:  return FuncVarPtg.create(in);//0x22, 0x42, 0x62
                        case NamePtg.sid:     return new NamePtg(in);     // 0x23, 0x43, 0x63
                        case RefPtg.sid:      return new RefPtg(in);      // 0x24, 0x44, 0x64
                        case AreaPtg.sid:     return new AreaPtg(in);     // 0x25, 0x45, 0x65
index ec2252d5e054523d20f2c3fb0b23bb533129aee1..0a7b56812e09d689b214c1a8a39c692152acaa92 100644 (file)
@@ -970,7 +970,7 @@ public final class FormulaParser {
                        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);
+                       return new ParseNode(FuncVarPtg.create(name, numArgs+1), allArgs);
                }
 
                if (namePtg != null) {
@@ -988,9 +988,9 @@ public final class FormulaParser {
 
                AbstractFunctionPtg retval;
                if(isVarArgs) {
-                       retval = new FuncVarPtg(name, (byte)numArgs);
+                       retval = FuncVarPtg.create(name, numArgs);
                } else {
-                       retval = new FuncPtg(funcIx);
+                       retval = FuncPtg.create(funcIx);
                }
                return new ParseNode(retval, args);
        }
@@ -1013,7 +1013,7 @@ public final class FormulaParser {
                                maxArgs = _book.getSpreadsheetVersion().getMaxFunctionArgs();
                        } else {
                                //_book can be omitted by test cases
-                               maxArgs = fm.getMaxParams(); // just use BIFF8 
+                               maxArgs = fm.getMaxParams(); // just use BIFF8
                        }
                } else {
                        maxArgs = fm.getMaxParams();
index 620370aa01d9e52e542a3bfdabad3fbbf243bc08..9af64112853152553ca3a1f331c312d4e1321007 100644 (file)
@@ -29,29 +29,29 @@ import org.apache.poi.hssf.record.formula.UnionPtg;
 import org.apache.poi.hssf.record.formula.ValueOperatorPtg;
 
 /**
- * This class performs 'operand class' transformation. Non-base tokens are classified into three 
+ * 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> 
+ * <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, 
+ *
+ * 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 {
@@ -77,15 +77,15 @@ final class OperandClassTransformer {
                                rootNodeOperandClass = Ptg.CLASS_REF;
                                break;
                        default:
-                               throw new RuntimeException("Incomplete code - formula type (" 
+                               throw new RuntimeException("Incomplete code - formula type ("
                                                + _formulaType + ") not supported yet");
-               
+
                }
                transformNode(rootNode, rootNodeOperandClass, false);
        }
 
        /**
-        * @param callerForceArrayFlag <code>true</code> if one of the current node's parents is a 
+        * @param callerForceArrayFlag <code>true</code> if one of the current node's parents is a
         * function Ptg which has been changed from default 'V' to 'A' type (due to requirements on
         * the function return value).
         */
@@ -94,7 +94,7 @@ final class OperandClassTransformer {
                Ptg token = node.getToken();
                ParseNode[] children = node.getChildren();
                boolean isSimpleValueFunc = isSimpleValueFunction(token);
-               
+
                if (isSimpleValueFunc) {
                        boolean localForceArray = desiredOperandClass == Ptg.CLASS_ARRAY;
                        for (int i = 0; i < children.length; i++) {
@@ -103,12 +103,12 @@ final class OperandClassTransformer {
                        setSimpleValueFuncClass((AbstractFunctionPtg) token, desiredOperandClass, callerForceArrayFlag);
                        return;
                }
-               
+
                if (isSingleArgSum(token)) {
                        // Need to process the argument of SUM with transformFunctionNode below
                        // so make a dummy FuncVarPtg for that call.
-                       token = new FuncVarPtg("SUM", (byte)1);
-                       // Note - the tAttrSum token (node.getToken()) is a base 
+                       token = FuncVarPtg.SUM;
+                       // Note - the tAttrSum token (node.getToken()) is a base
                        // token so does not need to have its operand class set
                }
                if (token instanceof ValueOperatorPtg || token instanceof ControlPtg
@@ -117,9 +117,9 @@ final class OperandClassTransformer {
                                || token instanceof UnionPtg) {
                        // Value Operator Ptgs and Control are base tokens, so token will be unchanged
                        // but any child nodes are processed according to desiredOperandClass and callerForceArrayFlag
-                       
+
                        // As per OOO documentation Sec 3.2.4 "Token Class Transformation", "Step 1"
-                       // All direct operands of value operators that are initially 'R' type will 
+                       // All direct operands of value operators that are initially 'R' type will
                        // be converted to 'V' type.
                        byte localDesiredOperandClass = desiredOperandClass == Ptg.CLASS_REF ? Ptg.CLASS_VALUE : desiredOperandClass;
                        for (int i = 0; i < children.length; i++) {
@@ -149,7 +149,7 @@ final class OperandClassTransformer {
        private static boolean isSingleArgSum(Ptg token) {
                if (token instanceof AttrPtg) {
                        AttrPtg attrPtg = (AttrPtg) token;
-                       return attrPtg.isSum(); 
+                       return attrPtg.isSum();
                }
                return false;
        }
@@ -180,12 +180,12 @@ final class OperandClassTransformer {
                                }
                                // else fall through
                        case Ptg.CLASS_ARRAY:
-                               return Ptg.CLASS_ARRAY; 
+                               return Ptg.CLASS_ARRAY;
                        case Ptg.CLASS_REF:
                                if (!callerForceArrayFlag) {
                                        return currentOperandClass;
                                }
-                               return Ptg.CLASS_REF; 
+                               return Ptg.CLASS_REF;
                }
                throw new IllegalStateException("Unexpected operand class (" + desiredOperandClass + ")");
        }
@@ -221,15 +221,15 @@ final class OperandClassTransformer {
                } else {
                        if (defaultReturnOperandClass == desiredOperandClass) {
                                localForceArrayFlag = false;
-                               // an alternative would have been to for non-base Ptgs to set their operand class 
+                               // 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); 
+                               afp.setClass(defaultReturnOperandClass);
                        } else {
                                switch (desiredOperandClass) {
                                        case Ptg.CLASS_VALUE:
                                                // always OK to set functions to return 'value'
-                                               afp.setClass(Ptg.CLASS_VALUE); 
+                                               afp.setClass(Ptg.CLASS_VALUE);
                                                localForceArrayFlag = false;
                                                break;
                                        case Ptg.CLASS_ARRAY:
@@ -282,7 +282,7 @@ final class OperandClassTransformer {
                if (callerForceArrayFlag  || desiredOperandClass == Ptg.CLASS_ARRAY) {
                        afp.setClass(Ptg.CLASS_ARRAY);
                } else {
-                       afp.setClass(Ptg.CLASS_VALUE); 
+                       afp.setClass(Ptg.CLASS_VALUE);
                }
        }
 }
index f9b7151dfac207a7b696c81b9292af5ad99b1598..18d8c11d42618c9c1fa57de0913bbb6074284c97 100644 (file)
@@ -344,8 +344,7 @@ public final class WorkbookEvaluator {
                                if (attrPtg.isSum()) {
                                        // Excel prefers to encode 'SUM()' as a tAttr token, but this evaluator
                                        // expects the equivalent function token
-                                       byte nArgs = 1;  // tAttrSum always has 1 parameter
-                                       ptg = new FuncVarPtg("SUM", nArgs);
+                                       ptg = FuncVarPtg.SUM;
                                }
                                if (attrPtg.isOptimizedChoose()) {
                                        ValueEval arg0 = stack.pop();
index 30c6df530dfd4adba59cf7ed85ff1198abe61edb..6af5ba4ba1e16b72a9acc8ddee51a1241564710f 100644 (file)
@@ -419,7 +419,7 @@ public final class TestFormulaParser extends TestCase {
                HSSFWorkbook book = new HSSFWorkbook();
 
                Ptg[] ptgs = {
-                               new FuncPtg(10),
+                               FuncPtg.create(10),
                };
                assertEquals("NA()", HSSFFormulaParser.toFormulaString(book, ptgs));
        }
index bbf8c4cfb23893f33b1756bde412f3c4e77a3743..399e80c240d8c2b063db2b786bbef6e4bc3165f0 100644 (file)
@@ -34,15 +34,15 @@ public final class TestFuncPtg extends TestCase {
             0,
         };
 
-        FuncPtg ptg = new FuncPtg(TestcaseRecordInputStream.createLittleEndian(fakeData) );
+        FuncPtg ptg = FuncPtg.create(TestcaseRecordInputStream.createLittleEndian(fakeData) );
         assertEquals( "Len formula index is not 32(20H)", 0x20, ptg.getFunctionIndex() );
         assertEquals( "Number of operands in the len formula", 1, ptg.getNumberOfOperands() );
         assertEquals( "Function Name", "LEN", ptg.getName() );
         assertEquals( "Ptg Size", 3, ptg.getSize() );
     }
-    
+
     public void testClone() {
-        FuncPtg funcPtg = new FuncPtg(27); // ROUND() - takes 2 args
+        FuncPtg funcPtg = FuncPtg.create(27); // ROUND() - takes 2 args
 
         FuncPtg clone = (FuncPtg) funcPtg.clone();
         if (clone.getNumberOfOperands() == 0) {
@@ -52,5 +52,3 @@ public final class TestFuncPtg extends TestCase {
         assertEquals("ROUND", clone.getName());
     }
 }
-
-
index dcf53132ecfe6d61bc54b77f6c897b7d4036d804..07e40898c52a658b0e0a2e7f939f7cf0d36243fe 100644 (file)
@@ -186,7 +186,7 @@ public final class TestRangeEval extends TestCase {
                                new RefPtg("C1"),
                                new IntPtg(0),
                                new RefPtg("B1"),
-                               new FuncVarPtg("OFFSET", (byte)3),
+                               FuncVarPtg.create("OFFSET", (byte)3),
                                RangePtg.instance,
                                AttrPtg.SUM,
                        };