From a76bd88d9b8290dc5500927540e125f7fd1eecce Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Fri, 13 Nov 2009 20:46:02 +0000 Subject: [PATCH] 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 --- .../poi/hssf/record/formula/AttrPtg.java | 124 +++++++++--------- .../poi/ss/formula/FormulaRenderer.java | 2 +- .../org/apache/poi/ss/formula/ParseNode.java | 37 ++---- .../poi/hssf/model/TestFormulaParserIf.java | 2 +- 4 files changed, 78 insertions(+), 87 deletions(-) diff --git a/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java b/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java index 1fe6fa3d7..5cec763e1 100644 --- a/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java +++ b/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 short field_2_data; + private final byte _options; + 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 sum = BitFieldFactory.getInstance(0x10); + private static final BitField optiSkip = BitFieldFactory.getInstance(0x08); + 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(); - field_2_data = in.readShort(); + _options = in.readByte(); + _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; - field_2_data = (short) data; + _options = (byte) options; + _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 + */ + 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); - } - - // lets hope no one uses this anymore - public boolean isGoto() { - return optGoto.isSet(field_1_options); + return optiChoose.isSet(_options); } public boolean isSum() { - return sum.isSet(field_1_options); + return optiSum.isSet(_options); } - - public void setOptimizedIf(boolean bif) { - 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); + public boolean isSkip() { + return optiSkip.isSet(_options); } // 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); - } - - public void setData(short data) { - field_2_data = data; + return space.isSet(_options); } 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(" type=").append(field_2_data & 0x00FF).append(" "); + sb.append("space count=").append((_data >> 8) & 0x00FF); + 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()); - } else if(isGoto()) { - sb.append("skip dist=").append(getData()); + sb.append("choose nCases=").append(_data); + } else if(isSkip()) { + 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.writeShort(field_2_data); + out.writeByte(_options); + 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); } } diff --git a/src/java/org/apache/poi/ss/formula/FormulaRenderer.java b/src/java/org/apache/poi/ss/formula/FormulaRenderer.java index 76dce7f55..d37e84359 100644 --- a/src/java/org/apache/poi/ss/formula/FormulaRenderer.java +++ b/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()) { diff --git a/src/java/org/apache/poi/ss/formula/ParseNode.java b/src/java/org/apache/poi/ss/formula/ParseNode.java index 40d99444e..9b2859daa 100644 --- a/src/java/org/apache/poi/ss/formula/ParseNode.java +++ b/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 Ptg * token. Each ParseNode may have child ParseNodes in the case when the wrapped * Ptg 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(); - attrIf.setOptimizedIf(true); - AttrPtg attrSkipAfterTrue = new AttrPtg(); - attrSkipAfterTrue.setGoto(true); - + AttrPtg attrIf = AttrPtg.createIf(trueParamSize + 4); // distance to start of false parameter/tFuncVar. +4 for tAttrSkip after 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). + + AttrPtg attrSkipAfterTrue = AttrPtg.createSkip(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). 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). - + AttrPtg attrSkipAfterTrue = AttrPtg.createSkip(4 - 1); // 1 less than distance to end of if FuncVar(size=4). + temp.setPlaceholder(ifAttrIndex, attrIf); temp.setPlaceholder(skipAfterTrueParamIndex, attrSkipAfterTrue); } - temp.add(_token); } diff --git a/src/testcases/org/apache/poi/hssf/model/TestFormulaParserIf.java b/src/testcases/org/apache/poi/hssf/model/TestFormulaParserIf.java index ae4245ec4..51f9cf5d9 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestFormulaParserIf.java +++ b/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