Cleaned up AttrPtg - made immutable, fixed property accessors.

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

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

@@ -33,8 +33,8 @@ import org.apache.poi.util.LittleEndianOutput;
 public final class AttrPtg extends ControlPtg {
     public final static byte sid  = 0x19;
     private final static int  SIZE = 4;
-    private byte              field_1_options;
+    private final byte _options;
-    private short             field_2_data;
+    private final short _data;
 
     /** only used for tAttrChoose: table of offsets to starts of args */
     private final int[] _jumpTable;
@@ -46,8 +46,8 @@ public final class AttrPtg extends ControlPtg {
     private static final BitField semiVolatile = BitFieldFactory.getInstance(0x01);
     private static final BitField optiIf       = BitFieldFactory.getInstance(0x02);
     private static final BitField optiChoose   = BitFieldFactory.getInstance(0x04);
-    private static final BitField optGoto      = BitFieldFactory.getInstance(0x08); // skip
+    private static final BitField optiSkip     = BitFieldFactory.getInstance(0x08);
-    private static final BitField sum          = BitFieldFactory.getInstance(0x10);
+    private static final BitField optiSum      = BitFieldFactory.getInstance(0x10);
     private static final BitField baxcel       = BitFieldFactory.getInstance(0x20); // 'assignment-style formula in a macro sheet'
     private static final BitField space        = BitFieldFactory.getInstance(0x40);
 
@@ -74,16 +74,11 @@ public final class AttrPtg extends ControlPtg {
         public static final int SPACE_AFTER_EQUALITY = 0x06;
     }
 
-    public AttrPtg() {
-        _jumpTable = null;
-        _chooseFuncOffset = -1;
-    }
-
     public AttrPtg(LittleEndianInput in) {
-        field_1_options = in.readByte();
+        _options = in.readByte();
-        field_2_data    = in.readShort();
+        _data    = in.readShort();
         if (isOptimizedChoose()) {
-            int nCases = field_2_data;
+            int nCases = _data;
             int[] jumpTable = new int[nCases];
             for (int i = 0; i < jumpTable.length; i++) {
                 jumpTable[i] = in.readUShort();
@@ -97,8 +92,8 @@ public final class AttrPtg extends ControlPtg {
 
     }
     private AttrPtg(int options, int data, int[] jt, int chooseFuncOffset) {
-        field_1_options = (byte) options;
+        _options = (byte) options;
-        field_2_data = (short) data;
+        _data = (short) data;
         _jumpTable = jt;
         _chooseFuncOffset = chooseFuncOffset;
     }
@@ -112,60 +107,65 @@ public final class AttrPtg extends ControlPtg {
         return new AttrPtg(space.set(0), data, null, -1);
     }
 
+    /**
+     * @param dist distance (in bytes) to start of either <ul><li>false parameter</li>
+     * <li>tFuncVar(IF) token (when false parameter is not present)</li></ul>
+     */
+    public static AttrPtg createIf(int dist) {
+        return new AttrPtg(optiIf.set(0), dist, null, -1);
+    }
+
+    /**
+     * @param dist distance (in bytes) to position behind tFuncVar(IF) token (minus 1)
+     */
+    public static AttrPtg createSkip(int dist) {
+        return new AttrPtg(optiSkip.set(0), dist, null, -1);
+    }
+
     public static AttrPtg getSumSingle() {
-        return new AttrPtg(sum.set(0), 0, null, -1);
+        return new AttrPtg(optiSum.set(0), 0, null, -1);
     }
 
 
     public boolean isSemiVolatile() {
-        return semiVolatile.isSet(field_1_options);
+        return semiVolatile.isSet(_options);
     }
 
     public boolean isOptimizedIf() {
-        return optiIf.isSet(field_1_options);
+        return optiIf.isSet(_options);
     }
 
     public boolean isOptimizedChoose() {
-        return optiChoose.isSet(field_1_options);
+        return optiChoose.isSet(_options);
-    }
-
-    // lets hope no one uses this anymore
-    public boolean isGoto() {
-        return optGoto.isSet(field_1_options);
     }
 
     public boolean isSum() {
-        return sum.isSet(field_1_options);
+        return optiSum.isSet(_options);
     }
-
+    public boolean isSkip() {
-    public void setOptimizedIf(boolean bif) {
+        return optiSkip.isSet(_options);
-        field_1_options=optiIf.setByteBoolean(field_1_options,bif);
-    }
-
-    /**
-     * Flags this ptg as a goto/jump
-     * @param isGoto
-     */
-    public void setGoto(boolean isGoto) {
-        field_1_options=optGoto.setByteBoolean(field_1_options, isGoto);
     }
 
     // lets hope no one uses this anymore
     private boolean isBaxcel() {
-        return baxcel.isSet(field_1_options);
+        return baxcel.isSet(_options);
     }
 
-    // biff3&4 only  shouldn't happen anymore
     public boolean isSpace() {
-        return space.isSet(field_1_options);
+        return space.isSet(_options);
-    }
-
-    public void setData(short data) {
-        field_2_data = data;
     }
 
     public short getData() {
-        return field_2_data;
+        return _data;
+    }
+    public int[] getJumpTable() {
+        return _jumpTable.clone();
+    }
+    public int getChooseFuncOffset() {
+        if (_jumpTable == null) {
+            throw new IllegalStateException("Not tAttrChoose");
+        }
+        return _chooseFuncOffset;
     }
 
     public String toString() {
@@ -176,16 +176,16 @@ public final class AttrPtg extends ControlPtg {
             sb.append("volatile ");
         }
         if(isSpace()) {
-            sb.append("space count=").append((field_2_data >> 8) & 0x00FF);
+            sb.append("space count=").append((_data >> 8) & 0x00FF);
-            sb.append(" type=").append(field_2_data & 0x00FF).append(" ");
+            sb.append(" type=").append(_data & 0x00FF).append(" ");
         }
         // the rest seem to be mutually exclusive
         if(isOptimizedIf()) {
-            sb.append("if dist=").append(getData());
+            sb.append("if dist=").append(_data);
         } else if(isOptimizedChoose()) {
-            sb.append("choose nCases=").append(getData());
+            sb.append("choose nCases=").append(_data);
-        } else if(isGoto()) {
+        } else if(isSkip()) {
-            sb.append("skip dist=").append(getData());
+            sb.append("skip dist=").append(_data);
         } else if(isSum()) {
             sb.append("sum ");
         } else if(isBaxcel()) {
@@ -197,8 +197,8 @@ public final class AttrPtg extends ControlPtg {
 
     public void write(LittleEndianOutput out) {
         out.writeByte(sid + getPtgClass());
-        out.writeByte(field_1_options);
+        out.writeByte(_options);
-        out.writeShort(field_2_data);
+        out.writeShort(_data);
         int[] jt = _jumpTable;
         if (jt != null) {
             for (int i = 0; i < jt.length; i++) {
@@ -216,11 +216,11 @@ public final class AttrPtg extends ControlPtg {
     }
 
     public String toFormulaString(String[] operands) {
-        if(space.isSet(field_1_options)) {
+        if(space.isSet(_options)) {
             return operands[ 0 ];
-        } else if (optiIf.isSet(field_1_options)) {
+        } else if (optiIf.isSet(_options)) {
             return toFormulaString() + "(" + operands[0] + ")";
-        } else if (optGoto.isSet(field_1_options)) {
+        } else if (optiSkip.isSet(_options)) {
             return toFormulaString() + operands[0];   //goto isn't a real formula element should not show up
         } else {
             return toFormulaString() + "(" + operands[0] + ")";
@@ -237,25 +237,25 @@ public final class AttrPtg extends ControlPtg {
     }
 
    public String toFormulaString() {
-      if(semiVolatile.isSet(field_1_options)) {
+      if(semiVolatile.isSet(_options)) {
         return "ATTR(semiVolatile)";
       }
-      if(optiIf.isSet(field_1_options)) {
+      if(optiIf.isSet(_options)) {
         return "IF";
       }
-      if( optiChoose.isSet(field_1_options)) {
+      if( optiChoose.isSet(_options)) {
         return "CHOOSE";
       }
-      if(optGoto.isSet(field_1_options)) {
+      if(optiSkip.isSet(_options)) {
         return "";
       }
-      if(sum.isSet(field_1_options)) {
+      if(optiSum.isSet(_options)) {
         return "SUM";
       }
-      if(baxcel.isSet(field_1_options)) {
+      if(baxcel.isSet(_options)) {
         return "ATTR(baxcel)";
       }
-      if(space.isSet(field_1_options)) {
+      if(space.isSet(_options)) {
         return "";
       }
       return "UNKNOWN ATTRIBUTE";
@@ -268,6 +268,6 @@ public final class AttrPtg extends ControlPtg {
         } else {
             jt = _jumpTable.clone();
         }
-        return new AttrPtg(field_1_options, field_2_data, jt, _chooseFuncOffset);
+        return new AttrPtg(_options, _data, jt, _chooseFuncOffset);
     }
 }

src/java/org/apache/poi/ss/formula/FormulaRenderer.java

@@ -65,7 +65,7 @@ public class FormulaRenderer {
             }
             if (ptg instanceof AttrPtg) {
                 AttrPtg attrPtg = ((AttrPtg) ptg);
-                if (attrPtg.isOptimizedIf() || attrPtg.isOptimizedChoose() || attrPtg.isGoto()) {
+                if (attrPtg.isOptimizedIf() || attrPtg.isOptimizedChoose() || attrPtg.isSkip()) {
                     continue;
                 }
                 if (attrPtg.isSpace()) {

src/java/org/apache/poi/ss/formula/ParseNode.java

@@ -28,7 +28,7 @@ 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 {
@@ -103,58 +103,49 @@ final class ParseNode {
 	/**
 	 * 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();
+		AttrPtg attrIf = AttrPtg.createIf(trueParamSize + 4); // distance to start of false parameter/tFuncVar. +4 for tAttrSkip after true
-		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
+			AttrPtg attrSkipAfterTrue = AttrPtg.createSkip(falseParamSize + 4 + 4 - 1); // 1 less than distance to end of if FuncVar(size=4). +4 for attr skip before
-			attrSkipAfterTrue.setData((short)(falseParamSize + 4 + 4 - 1)); // 1 less than distance to end of if FuncVar(size=4). +4 for attr skip before 
+			AttrPtg attrSkipAfterFalse = AttrPtg.createSkip(4 - 1); // 1 less than distance to end of if FuncVar(size=4).
-			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
+			AttrPtg attrSkipAfterTrue = AttrPtg.createSkip(4 - 1); // 1 less than distance to end of if FuncVar(size=4).
-			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(_token);
 	}
 

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

@@ -202,7 +202,7 @@ public final class TestFormulaParserIf extends TestCase {
 		assertEquals("Y", y.getValue());
 		assertEquals("N", n.getValue());
 		assertEquals("IF", funif.toFormulaString());
-		assertTrue("Goto ptg exists", goto1.isGoto());
+		assertTrue("tAttrSkip ptg exists", goto1.isSkip());
 	}
 	/**
 	 * Make sure the ptgs are generated properly with two functions embedded