From efb9160c1a5443a7524dd3953e391cbdcb63bfe3 Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Sun, 15 Nov 2009 09:01:35 +0000 Subject: [PATCH] Clean-up of copy/clone methods in Ptg hierarchy git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@836343 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/hssf/record/SharedFormulaRecord.java | 27 +++++++++---------- .../poi/hssf/record/TextObjectRecord.java | 6 +++-- .../poi/hssf/record/formula/Area3DPtg.java | 4 +-- .../poi/hssf/record/formula/AreaPtgBase.java | 13 +-------- .../poi/hssf/record/formula/AttrPtg.java | 10 ------- .../poi/hssf/record/formula/OperandPtg.java | 13 +++++++-- .../apache/poi/hssf/record/formula/Ptg.java | 23 +--------------- .../poi/hssf/record/formula/Ref3DPtg.java | 6 ++--- .../poi/hssf/record/formula/RefPtgBase.java | 2 +- .../poi/hssf/record/formula/UnknownPtg.java | 15 ++++------- .../poi/hssf/usermodel/HSSFWorkbook.java | 8 +++--- .../poi/hssf/record/formula/TestFuncPtg.java | 18 ++++--------- 12 files changed, 50 insertions(+), 95 deletions(-) diff --git a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java index 4878299981..c01cf77a34 100644 --- a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java +++ b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java @@ -64,7 +64,7 @@ public final class SharedFormulaRecord extends SharedValueRecordBase { out.writeShort(field_5_reserved); field_7_parsed_expr.serialize(out); } - + protected int getExtraDataSize() { return 2 + field_7_parsed_expr.getEncodedSize(); } @@ -98,8 +98,8 @@ public final class SharedFormulaRecord extends SharedValueRecordBase { /** * Creates a non shared formula from the shared formula counterpart
- * - * Perhaps this functionality could be implemented in terms of the raw + * + * Perhaps this functionality could be implemented in terms of the raw * byte array inside {@link Formula}. */ public static Ptg[] convertSharedFormulas(Ptg[] ptgs, int formulaRow, int formulaColumn) { @@ -113,14 +113,15 @@ public final class SharedFormulaRecord extends SharedValueRecordBase { originalOperandClass = ptg.getPtgClass(); } if (ptg instanceof RefPtgBase) { - RefPtgBase refNPtg = (RefPtgBase)ptg; - ptg = new RefPtg(fixupRelativeRow(formulaRow,refNPtg.getRow(),refNPtg.isRowRelative()), + RefPtgBase refNPtg = (RefPtgBase)ptg; + ptg = new RefPtg(fixupRelativeRow(formulaRow,refNPtg.getRow(),refNPtg.isRowRelative()), fixupRelativeColumn(formulaColumn,refNPtg.getColumn(),refNPtg.isColRelative()), refNPtg.isRowRelative(), refNPtg.isColRelative()); + ptg.setClass(originalOperandClass); } else if (ptg instanceof AreaPtgBase) { - AreaPtgBase areaNPtg = (AreaPtgBase)ptg; - ptg = new AreaPtg(fixupRelativeRow(formulaRow,areaNPtg.getFirstRow(),areaNPtg.isFirstRowRelative()), + AreaPtgBase areaNPtg = (AreaPtgBase)ptg; + ptg = new AreaPtg(fixupRelativeRow(formulaRow,areaNPtg.getFirstRow(),areaNPtg.isFirstRowRelative()), fixupRelativeRow(formulaRow,areaNPtg.getLastRow(),areaNPtg.isLastRowRelative()), fixupRelativeColumn(formulaColumn,areaNPtg.getFirstColumn(),areaNPtg.isFirstColRelative()), fixupRelativeColumn(formulaColumn,areaNPtg.getLastColumn(),areaNPtg.isLastColRelative()), @@ -128,15 +129,13 @@ public final class SharedFormulaRecord extends SharedValueRecordBase { areaNPtg.isLastRowRelative(), areaNPtg.isFirstColRelative(), areaNPtg.isLastColRelative()); - } else { - if (false) {// do we need a ptg clone here? - ptg = ptg.copy(); - } - } - if (!ptg.isBaseToken()) { ptg.setClass(originalOperandClass); + } else if (ptg instanceof OperandPtg) { + // Any subclass of OperandPtg is mutable, so it's safest to not share these instances. + ptg = ((OperandPtg) ptg).copy(); + } else { + // all other Ptgs are immutable and can be shared } - newPtgStack[k] = ptg; } return newPtgStack; diff --git a/src/java/org/apache/poi/hssf/record/TextObjectRecord.java b/src/java/org/apache/poi/hssf/record/TextObjectRecord.java index 06fb3e4319..68398bb70c 100644 --- a/src/java/org/apache/poi/hssf/record/TextObjectRecord.java +++ b/src/java/org/apache/poi/hssf/record/TextObjectRecord.java @@ -19,6 +19,7 @@ package org.apache.poi.hssf.record; import org.apache.poi.hssf.record.cont.ContinuableRecord; import org.apache.poi.hssf.record.cont.ContinuableRecordOutput; +import org.apache.poi.hssf.record.formula.OperandPtg; import org.apache.poi.hssf.record.formula.Ptg; import org.apache.poi.hssf.usermodel.HSSFRichTextString; import org.apache.poi.util.BitField; @@ -74,13 +75,14 @@ public final class TextObjectRecord extends ContinuableRecord { */ private int _unknownPreFormulaInt; /** expect tRef, tRef3D, tArea, tArea3D or tName */ - private Ptg _linkRefPtg; + private OperandPtg _linkRefPtg; /** * Not clear if needed . Excel seems to be OK if this byte is not present. * Value is often the same as the earlier firstColumn byte. */ private Byte _unknownPostFormulaByte; public TextObjectRecord() { + // } public TextObjectRecord(RecordInputStream in) { @@ -106,7 +108,7 @@ public final class TextObjectRecord extends ContinuableRecord { throw new RecordFormatException("Read " + ptgs.length + " tokens but expected exactly 1"); } - _linkRefPtg = ptgs[0]; + _linkRefPtg = (OperandPtg) ptgs[0]; if (in.remaining() > 0) { _unknownPostFormulaByte = Byte.valueOf(in.readByte()); } else { diff --git a/src/java/org/apache/poi/hssf/record/formula/Area3DPtg.java b/src/java/org/apache/poi/hssf/record/formula/Area3DPtg.java index aec3e03503..264b448ca6 100644 --- a/src/java/org/apache/poi/hssf/record/formula/Area3DPtg.java +++ b/src/java/org/apache/poi/hssf/record/formula/Area3DPtg.java @@ -32,10 +32,10 @@ import org.apache.poi.util.LittleEndianOutput; * @author avik * @author Jason Height (jheight at chariot dot net dot au) */ -public final class Area3DPtg extends AreaPtgBase implements WorkbookDependentFormula, ExternSheetReferenceToken { +public final class Area3DPtg extends AreaPtgBase implements WorkbookDependentFormula, ExternSheetReferenceToken { public final static byte sid = 0x3b; private final static int SIZE = 11; // 10 + 1 for Ptg - + private int field_1_index_extern_sheet; diff --git a/src/java/org/apache/poi/hssf/record/formula/AreaPtgBase.java b/src/java/org/apache/poi/hssf/record/formula/AreaPtgBase.java index 5cd5a9b748..d50fca02f9 100644 --- a/src/java/org/apache/poi/hssf/record/formula/AreaPtgBase.java +++ b/src/java/org/apache/poi/hssf/record/formula/AreaPtgBase.java @@ -29,7 +29,7 @@ import org.apache.poi.util.LittleEndianOutput; * @author andy * @author Jason Height (jheight at chariot dot net dot au) */ -public abstract class AreaPtgBase extends OperandPtg implements AreaI { +public abstract class AreaPtgBase> extends OperandPtg implements AreaI { /** * TODO - (May-2008) fix subclasses of AreaPtg 'AreaN~' which are used in shared formulas. * see similar comment in ReferencePtg @@ -96,17 +96,6 @@ public abstract class AreaPtgBase extends OperandPtg implements AreaI { } } - private static void $checkColumnBounds(int colIx) { - if((colIx & 0x0FF) != colIx) { - throw new IllegalArgumentException("colIx (" + colIx + ") is out of range"); - } - } - private static void $checkRowBounds(int rowIx) { - if((rowIx & 0x0FFFF) != rowIx) { - throw new IllegalArgumentException("rowIx (" + rowIx + ") is out of range"); - } - } - protected final void readCoordinates(LittleEndianInput in) { field_1_first_row = in.readUShort(); field_2_last_row = in.readUShort(); 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 5cec763e13..bbfb95a729 100644 --- a/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java +++ b/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java @@ -260,14 +260,4 @@ public final class AttrPtg extends ControlPtg { } return "UNKNOWN ATTRIBUTE"; } - - public Object clone() { - int[] jt; - if (_jumpTable == null) { - jt = null; - } else { - jt = _jumpTable.clone(); - } - return new AttrPtg(_options, _data, jt, _chooseFuncOffset); - } } diff --git a/src/java/org/apache/poi/hssf/record/formula/OperandPtg.java b/src/java/org/apache/poi/hssf/record/formula/OperandPtg.java index 0c1d1faeb8..bd1490289c 100644 --- a/src/java/org/apache/poi/hssf/record/formula/OperandPtg.java +++ b/src/java/org/apache/poi/hssf/record/formula/OperandPtg.java @@ -21,12 +21,21 @@ package org.apache.poi.hssf.record.formula; /** * @author Josh Micich */ -public abstract class OperandPtg extends Ptg { +public abstract class OperandPtg> extends Ptg implements Cloneable { /** - * All Operand Ptgs are classifed ('relative', 'value', 'array') + * All Operand {@link Ptg}s are classified ('relative', 'value', 'array') */ public final boolean isBaseToken() { return false; } + public final Y copy() { + try { + @SuppressWarnings("unchecked") + Y result = (Y) clone(); + return result; + } catch (CloneNotSupportedException e) { + throw new RuntimeException(e); + } + } } diff --git a/src/java/org/apache/poi/hssf/record/formula/Ptg.java b/src/java/org/apache/poi/hssf/record/formula/Ptg.java index f2857a7dfa..2426249c15 100644 --- a/src/java/org/apache/poi/hssf/record/formula/Ptg.java +++ b/src/java/org/apache/poi/hssf/record/formula/Ptg.java @@ -40,7 +40,7 @@ import org.apache.poi.util.LittleEndianOutput; * @author avik * @author Jason Height (jheight at chariot dot net dot au) */ -public abstract class Ptg implements Cloneable { +public abstract class Ptg { public static final Ptg[] EMPTY_PTG_ARRAY = { }; @@ -158,28 +158,7 @@ public abstract class Ptg implements Cloneable { } throw new RuntimeException("Unexpected base token id (" + id + ")"); } - /** - * @return a distinct copy of this Ptg if the class is mutable, or the same instance - * if the class is immutable. - */ - public final Ptg copy() { - // TODO - all base tokens are logically immutable, but AttrPtg needs some clean-up - if (this instanceof ValueOperatorPtg) { - return this; - } - if (this instanceof ScalarConstantPtg) { - return this; - } - return (Ptg) clone(); - } - protected Object clone() { - try { - return super.clone(); - } catch (CloneNotSupportedException e) { - throw new RuntimeException(e); - } - } private static Ptg[] toPtgArray(List l) { if (l.isEmpty()) { return EMPTY_PTG_ARRAY; diff --git a/src/java/org/apache/poi/hssf/record/formula/Ref3DPtg.java b/src/java/org/apache/poi/hssf/record/formula/Ref3DPtg.java index c695932a1c..13a7f05899 100644 --- a/src/java/org/apache/poi/hssf/record/formula/Ref3DPtg.java +++ b/src/java/org/apache/poi/hssf/record/formula/Ref3DPtg.java @@ -31,7 +31,7 @@ import org.apache.poi.util.LittleEndianOutput; * @author Libin Roman (Vista Portal LDT. Developer) * @author Jason Height (jheight at chariot dot net dot au) */ -public final class Ref3DPtg extends RefPtgBase implements WorkbookDependentFormula, ExternSheetReferenceToken { +public final class Ref3DPtg extends RefPtgBase implements WorkbookDependentFormula, ExternSheetReferenceToken { public final static byte sid = 0x3a; private final static int SIZE = 7; // 6 + 1 for Ptg @@ -42,7 +42,7 @@ public final class Ref3DPtg extends RefPtgBase implements WorkbookDependentFormu field_1_index_extern_sheet = in.readShort(); readCoordinates(in); } - + public Ref3DPtg(String cellref, int externIdx ) { this(new CellReference(cellref), externIdx); } @@ -82,7 +82,7 @@ public final class Ref3DPtg extends RefPtgBase implements WorkbookDependentFormu } /** - * @return text representation of this cell reference that can be used in text + * @return text representation of this cell reference that can be used in text * formulas. The sheet name will get properly delimited if required. */ public String toFormulaString(FormulaRenderingWorkbook book) { diff --git a/src/java/org/apache/poi/hssf/record/formula/RefPtgBase.java b/src/java/org/apache/poi/hssf/record/formula/RefPtgBase.java index 70ed4da87a..28e7ebd9fb 100644 --- a/src/java/org/apache/poi/hssf/record/formula/RefPtgBase.java +++ b/src/java/org/apache/poi/hssf/record/formula/RefPtgBase.java @@ -29,7 +29,7 @@ import org.apache.poi.util.LittleEndianOutput; * @author Andrew C. Oliver (acoliver@apache.org) * @author Jason Height (jheight at chariot dot net dot au) */ -public abstract class RefPtgBase extends OperandPtg { +public abstract class RefPtgBase> extends OperandPtg { /** The row index - zero based unsigned 16 bit value */ private int field_1_row; diff --git a/src/java/org/apache/poi/hssf/record/formula/UnknownPtg.java b/src/java/org/apache/poi/hssf/record/formula/UnknownPtg.java index d441a0098d..ca94766a5e 100644 --- a/src/java/org/apache/poi/hssf/record/formula/UnknownPtg.java +++ b/src/java/org/apache/poi/hssf/record/formula/UnknownPtg.java @@ -19,8 +19,7 @@ package org.apache.poi.hssf.record.formula; import org.apache.poi.util.LittleEndianOutput; /** - * - * @author andy + * @author andy * @author Jason Height (jheight at chariot dot net dot au) */ public class UnknownPtg extends Ptg { @@ -38,18 +37,14 @@ public class UnknownPtg extends Ptg { out.writeByte(_sid); } - public int getSize() - { + public int getSize() { return size; } - public String toFormulaString() - { + public String toFormulaString() { return "UNKNOWN"; } - public byte getDefaultOperandClass() {return Ptg.CLASS_VALUE;} - - public Object clone() { - return this; + public byte getDefaultOperandClass() { + return Ptg.CLASS_VALUE; } } diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index 3d81c478f4..612eb67b18 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java @@ -676,16 +676,16 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm Ptg[] ptgs = origNameRecord.getNameDefinition(); for (int i=0; i< ptgs.length; i++) { Ptg ptg = ptgs[i]; - ptg = ptg.copy(); if (ptg instanceof Area3DPtg) { - Area3DPtg a3p = (Area3DPtg) ptg; + Area3DPtg a3p = ((Area3DPtg) ptg).copy(); a3p.setExternSheetIndex(newExtSheetIx); + ptgs[i] = a3p; } else if (ptg instanceof Ref3DPtg) { - Ref3DPtg r3p = (Ref3DPtg) ptg; + Ref3DPtg r3p = ((Ref3DPtg) ptg).copy(); r3p.setExternSheetIndex(newExtSheetIx); + ptgs[i] = r3p; } - ptgs[i] = ptg; } NameRecord newNameRecord = workbook.createBuiltInName(NameRecord.BUILTIN_FILTER_DB, newSheetIndex+1); newNameRecord.setNameDefinition(ptgs); diff --git a/src/testcases/org/apache/poi/hssf/record/formula/TestFuncPtg.java b/src/testcases/org/apache/poi/hssf/record/formula/TestFuncPtg.java index 399e80c240..52121728c3 100644 --- a/src/testcases/org/apache/poi/hssf/record/formula/TestFuncPtg.java +++ b/src/testcases/org/apache/poi/hssf/record/formula/TestFuncPtg.java @@ -28,11 +28,8 @@ import org.apache.poi.hssf.record.TestcaseRecordInputStream; public final class TestFuncPtg extends TestCase { public void testRead() { - // This ptg represents a LEN function extracted from excel - byte[] fakeData = { - 0x20, //function index - 0, - }; + // This function index represents the LEN() function + byte[] fakeData = { 0x20, 0x00,}; FuncPtg ptg = FuncPtg.create(TestcaseRecordInputStream.createLittleEndian(fakeData) ); assertEquals( "Len formula index is not 32(20H)", 0x20, ptg.getFunctionIndex() ); @@ -41,14 +38,9 @@ public final class TestFuncPtg extends TestCase { assertEquals( "Ptg Size", 3, ptg.getSize() ); } - public void testClone() { + public void testNumberOfOperands() { FuncPtg funcPtg = FuncPtg.create(27); // ROUND() - takes 2 args - - FuncPtg clone = (FuncPtg) funcPtg.clone(); - if (clone.getNumberOfOperands() == 0) { - fail("clone() did copy field numberOfOperands"); - } - assertEquals(2, clone.getNumberOfOperands()); - assertEquals("ROUND", clone.getName()); + assertEquals(2, funcPtg.getNumberOfOperands()); + assertEquals("ROUND", funcPtg.getName()); } } -- 2.39.5