added javadoc and few other improvements

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

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

@@ -351,6 +351,9 @@ public final class InternalSheet {
                 continue;
             }
             if (rb instanceof EscherAggregate){
+                /**
+                 * this record will be removed after reading actual data from EscherAggregate
+                 */
                 rb = new DrawingRecord();
             }
             Record rec = (Record) ((Record) rb).clone();

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

@@ -37,6 +37,7 @@ public final class DrawingRecord extends StandardRecord {
         recordData = in.readRemainder();
     }
 
+    @Deprecated
     public void processContinueRecord(byte[] record) {
         //don't merge continue record with the drawing record, it must be serialized separately
         contd = record;
@@ -54,13 +55,8 @@ public final class DrawingRecord extends StandardRecord {
         return sid;
     }
 
+    @Deprecated
     public byte[] getData() {
-//        if (continueData.size() != 0) {
-//            byte[] newBuffer = new byte[recordData.length + continueData.size()];
-//            System.arraycopy(recordData, 0, newBuffer, 0, recordData.length);
-//            System.arraycopy(continueData.toByteArray(), 0, newBuffer, recordData.length, continueData.size());
-//            return newBuffer;
-//        }
         return recordData;
     }
 
@@ -75,6 +71,10 @@ public final class DrawingRecord extends StandardRecord {
         recordData = thedata;
     }
 
+    /**
+     * Cloning of drawing records must be executed through HSSFPatriarch, because all id's must be changed
+     * @return cloned drawing records
+     */
     public Object clone() {
         DrawingRecord rec = new DrawingRecord();
         rec.recordData = recordData.clone();

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

@@ -290,8 +290,6 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
     public static final short ST_TEXTBOX = (short) 202;
     public static final short ST_NIL = (short) 0x0FFF;
 
-    protected HSSFPatriarch patriarch;
-
     /**
      * if we want to get the same byte array if we open existing file and serialize it we should save
      * note records in right order. This list contains ids of NoteRecords in such order as we read from existing file
@@ -340,6 +338,10 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
         return result.toString();
     }
 
+    /**
+     * Calculates the xml representation of this record.  This is
+     * simply a dump of all the records.
+     */
     public String toXml(String tab) {
         StringBuilder builder = new StringBuilder();
         builder.append(tab).append("<").append(getRecordName()).append(">\n");
@@ -360,7 +362,9 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
 
     /**
      * Collapses the drawing records into an aggregate.
-     * read Drawing and Continue records into single byte array, create Escher tree from byte array, create map <EscherRecord, Record>
+     * read Drawing, Obj, TxtObj, Note and Continue records into single byte array,
+     * create Escher tree from byte array, create map <EscherRecord, Record>
+     *
      */
     public static EscherAggregate createAggregate(List records, int locFirstDrawingRecord, DrawingManager2 drawingManager) {
         // Keep track of any shape records created so we can match them back to the object id's.
@@ -494,7 +498,7 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
 
             byte[] drawingData = new byte[endOffset - startOffset + 1];
             System.arraycopy(buffer, startOffset, drawingData, 0, drawingData.length);
-            pos += writeDataIntoDrawingRecord(0, drawingData, writtenEscherBytes, pos, data, i);
+            pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, i);
 
             writtenEscherBytes += drawingData.length;
 
@@ -505,13 +509,13 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
             if (i == shapes.size() - 1 && endOffset < buffer.length - 1) {
                 drawingData = new byte[buffer.length - endOffset - 1];
                 System.arraycopy(buffer, endOffset + 1, drawingData, 0, drawingData.length);
-                pos += writeDataIntoDrawingRecord(0, drawingData, writtenEscherBytes, pos, data, i);
+                pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, i);
             }
         }
         if ((pos - offset) < buffer.length - 1) {
             byte[] drawingData = new byte[buffer.length - (pos - offset)];
             System.arraycopy(buffer, (pos - offset), drawingData, 0, drawingData.length);
-            pos += writeDataIntoDrawingRecord(0, drawingData, writtenEscherBytes, pos, data, i);
+            pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, i);
         }
 
         // write records that need to be serialized after all drawing group records
@@ -535,7 +539,17 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
         return bytesWritten;
     }
 
-    private int writeDataIntoDrawingRecord(int temp, byte[] drawingData, int writtenEscherBytes, int pos, byte[] data, int i) {
+    /**
+     * @param drawingData - escher records saved into single byte array
+     * @param writtenEscherBytes - count of bytes already saved into drawing records (we should know it to decide create
+     *                           drawing or continue record)
+     * @param pos current position of data array
+     * @param data - array of bytes where drawing records must be serialized
+     * @param i - number of shape, saved into data array
+     * @return offset of data array after serialization
+     */
+    private int writeDataIntoDrawingRecord(byte[] drawingData, int writtenEscherBytes, int pos, byte[] data, int i) {
+        int temp = 0;
         //First record in drawing layer MUST be DrawingRecord
         if (writtenEscherBytes + drawingData.length > RecordInputStream.MAX_RECORD_DATA_SIZE && i != 1) {
             for (int j = 0; j < drawingData.length; j += RecordInputStream.MAX_RECORD_DATA_SIZE) {
@@ -576,6 +590,9 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
         return size;
     }
 
+    /**
+     * @return record size, including header size of obj, text, note, drawing, continue records
+     */
     public int getRecordSize() {
         // To determine size of aggregate record we have to know size of each DrawingRecord because if DrawingRecord
         // is split into several continue records we have to add header size to total EscherAggregate size
@@ -638,12 +655,6 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
         shapeToObj.remove(rec);
     }
 
-    public void clear() {
-        clearEscherRecords();
-        shapeToObj.clear();
-//		lastShapeId = 1024;
-    }
-
     protected String getRecordName() {
         return "ESCHERAGGREGATE";
     }
@@ -740,34 +751,44 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
     }
 
     /**
-     * Returns the mapping  of {@link EscherClientDataRecord} and {@link EscherTextboxRecord}
+     * @return unmodifiable copy of the mapping  of {@link EscherClientDataRecord} and {@link EscherTextboxRecord}
      * to their {@link TextObjectRecord} or {@link ObjRecord} .
      * <p/>
      * We need to access it outside of EscherAggregate when building shapes
-     *
-     * @return
      */
     public Map<EscherRecord, Record> getShapeToObjMapping() {
         return Collections.unmodifiableMap(shapeToObj);
     }
 
     /**
-     * @return tails records. We need to access them when building shapes.
+     * @return unmodifiable copy of tail records. We need to access them when building shapes.
      *         Every HSSFComment shape has a link to a NoteRecord from the tailRec collection.
      */
     public Map<Integer, NoteRecord> getTailRecords() {
         return tailRec;
     }
 
+    /**
+     * @param obj - ObjRecord with id == NoteRecord.id
+     * @return null if note record is not found else returns note record with id == obj.id
+     */
     public NoteRecord getNoteRecordByObj(ObjRecord obj) {
         CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) obj.getSubRecords().get(0);
         return tailRec.get(cod.getObjectId());
     }
 
+    /**
+     * Add tail record to existing map
+     * @param note to be added
+     */
     public void addTailRecord(NoteRecord note) {
         tailRec.put(note.getShapeId(), note);
     }
 
+    /**
+     * Remove tail record from the existing map
+     * @param note to be removed
+     */
     public void removeTailRecord(NoteRecord note) {
         tailRec.remove(note.getShapeId());
     }

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

@@ -57,27 +57,57 @@ public abstract class HSSFAnchor {
         }
     }
 
+    /**
+     * @return x coordinate of the left up corner
+     */
     public abstract int getDx1();
 
+    /**
+     * @param dx1 x coordinate of the left up corner
+     */
     public abstract void setDx1(int dx1);
 
+    /**
+     * @return y coordinate of the left up corner
+     */
     public abstract int getDy1();
 
+    /**
+     * @param dy1 y coordinate of the left up corner
+     */
     public abstract void setDy1(int dy1);
 
+    /**
+     * @return y coordinate of the right down corner
+     */
     public abstract int getDy2();
 
+    /**
+     * @param dy2 y coordinate of the right down corner
+     */
     public abstract void setDy2(int dy2);
 
+    /**
+     * @return x coordinate of the right down corner
+     */
     public abstract int getDx2();
 
+    /**
+     * @param dx2 x coordinate of the right down corner
+     */
     public abstract void setDx2(int dx2);
 
+    /**
+     * @return whether this shape is horizontally flipped
+     */
     public abstract boolean isHorizontallyFlipped();
 
+    /**
+     * @return  whether this shape is vertically flipped
+     */
     public abstract boolean isVerticallyFlipped();
 
-    public abstract EscherRecord getEscherAnchor();
+    protected abstract EscherRecord getEscherAnchor();
 
     protected abstract void createEscherAnchor();
 }

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

@@ -25,6 +25,10 @@ public final class HSSFChildAnchor extends HSSFAnchor {
 
     private EscherChildAnchorRecord _escherChildAnchor;
 
+    /**
+     * create anchor from existing file
+     * @param escherChildAnchorRecord
+     */
     public HSSFChildAnchor(EscherChildAnchorRecord escherChildAnchorRecord) {
         this._escherChildAnchor = escherChildAnchorRecord;
     }
@@ -33,6 +37,13 @@ public final class HSSFChildAnchor extends HSSFAnchor {
         _escherChildAnchor = new EscherChildAnchorRecord();
     }
 
+    /**
+     * create anchor from scratch
+     * @param dx1 x coordinate of the left up corner
+     * @param dy1 y coordinate of the left up corner
+     * @param dx2 x coordinate of the right down corner
+     * @param dy2 y coordinate of the right down corner
+     */
     public HSSFChildAnchor(int dx1, int dy1, int dx2, int dy2) {
         super(Math.min(dx1, dx2), Math.min(dy1, dy2), Math.max(dx1, dx2), Math.max(dy1, dy2));
         if (dx1 > dx2){
@@ -83,6 +94,12 @@ public final class HSSFChildAnchor extends HSSFAnchor {
         _escherChildAnchor.setDx2(dx2);
     }
 
+    /**
+     * @param dx1 x coordinate of the left up corner
+     * @param dy1 y coordinate of the left up corner
+     * @param dx2 x coordinate of the right down corner
+     * @param dy2 y coordinate of the right down corner
+     */
     public void setAnchor(int dx1, int dy1, int dx2, int dy2) {
         setDx1(Math.min(dx1, dx2));
         setDy1(Math.min(dy1, dy2));
@@ -90,16 +107,18 @@ public final class HSSFChildAnchor extends HSSFAnchor {
         setDy2(Math.max(dy1, dy2));
     }
 
+
     public boolean isHorizontallyFlipped() {
         return _isHorizontallyFlipped;
     }
 
+
     public boolean isVerticallyFlipped() {
         return _isVerticallyFlipped;
     }
 
     @Override
-    public EscherRecord getEscherAnchor() {
+    protected EscherRecord getEscherAnchor() {
         return _escherChildAnchor;
     }
 

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

@@ -113,45 +113,75 @@ public final class HSSFClientAnchor extends HSSFAnchor implements ClientAnchor {
         return row.getHeightInPoints();
     }
 
+    /**
+     * @return the column(0 based) of the first cell.
+     */
     public short getCol1() {
         return _escherClientAnchor.getCol1();
     }
 
+    /**
+     * @param col1 the column(0 based) of the first cell.
+     */
     public void setCol1(short col1) {
         checkRange(col1, 0, 255, "col1");
         _escherClientAnchor.setCol1(col1);
     }
 
+    /**
+     * @param col1 0-based column of the first cell.
+     */
     public void setCol1(int col1) {
         setCol1((short) col1);
     }
 
+    /**
+     * @return the column(0 based) of the first cell.
+     */
     public short getCol2() {
         return _escherClientAnchor.getCol2();
     }
 
+    /**
+     * @param col2 the column(0 based) of the second cell.
+     */
     public void setCol2(short col2) {
         checkRange(col2, 0, 255, "col2");
         _escherClientAnchor.setCol2(col2);
     }
 
+    /**
+     * @param col2 the column(0 based) of the second cell.
+     */
     public void setCol2(int col2) {
         setCol2((short) col2);
     }
 
+    /**
+     * @return the row(0 based) of the first cell.
+     */
     public int getRow1() {
         return _escherClientAnchor.getRow1();
     }
 
+    /**
+     * @param row1 0-based row of the first cell.
+     */
     public void setRow1(int row1) {
         checkRange(row1, 0, 256 * 256, "row1");
         _escherClientAnchor.setRow1(Integer.valueOf(row1).shortValue());
     }
 
+    /**
+     * @return the row(0 based) of the second cell.
+     */
     public int getRow2() {
         return _escherClientAnchor.getRow2();
     }
 
+    /**
+     * @return the row(0 based) of the second cell.
+     */
     public void setRow2(int row2) {
         checkRange(row2, 0, 256 * 256, "row2");
         _escherClientAnchor.setRow2(Integer.valueOf(row2).shortValue());
@@ -190,22 +220,16 @@ public final class HSSFClientAnchor extends HSSFAnchor implements ClientAnchor {
         setDy2(y2);
     }
 
-    /**
-     * @return true if the anchor goes from right to left.
-     */
     public boolean isHorizontallyFlipped() {
         return _isHorizontallyFlipped;
     }
 
-    /**
-     * @return true if the anchor goes from bottom to top.
-     */
     public boolean isVerticallyFlipped() {
         return _isVerticallyFlipped;
     }
 
     @Override
-    public EscherRecord getEscherAnchor() {
+    protected EscherRecord getEscherAnchor() {
         return _escherClientAnchor;
     }
 
@@ -229,9 +253,6 @@ public final class HSSFClientAnchor extends HSSFAnchor implements ClientAnchor {
      * 0 = Move and size with Cells, 2 = Move but don't size with cells, 3 = Don't move or size with cells.
      */
     public void setAnchorType(int anchorType) {
-//        if (0 != anchorType && 2 != anchorType && 3 != anchorType){
-//            throw new IllegalArgumentException("Anchor type of shape can take only such values: 0, 2, 3");
-//        }
         _escherClientAnchor.setFlag(Integer.valueOf(anchorType).shortValue());
     }
 

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

@@ -33,6 +33,8 @@ public class HSSFCombobox extends HSSFSimpleShape {
     public HSSFCombobox(HSSFShape parent, HSSFAnchor anchor) {
         super(parent, anchor);
         super.setShapeType(OBJECT_TYPE_COMBO_BOX);
+        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) getObjRecord().getSubRecords().get(0);
+        cod.setObjectType(CommonObjectDataSubRecord.OBJECT_TYPE_COMBO_BOX);
     }
 
     @Override

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

@@ -62,6 +62,8 @@ public class HSSFComment extends HSSFTextbox implements Comment {
         //by default comments are hidden
         setVisible(false);
         setAuthor("");
+        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) getObjRecord().getSubRecords().get(0);
+        cod.setObjectType(CommonObjectDataSubRecord.OBJECT_TYPE_COMMENT);
     }
 
     protected HSSFComment(NoteRecord note, TextObjectRecord txo) {

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

@@ -66,10 +66,17 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
         buildShapeTree();
     }
 
+    /**
+     * used to clone patriarch
+     *
+     * create patriarch from existing one
+     * @param patriarch - copy all the shapes from this patriarch to new one
+     * @param sheet where must be located new patriarch
+     * @return new patriarch with copies of all shapes from the existing patriarch
+     */
     static HSSFPatriarch createPatriarch(HSSFPatriarch patriarch, HSSFSheet sheet){
         HSSFPatriarch newPatriarch = new HSSFPatriarch(sheet, new EscherAggregate());
         newPatriarch.afterCreate();
-
         for (HSSFShape shape: patriarch.getChildren()){
             HSSFShape newShape;
             if (shape instanceof HSSFShapeGroup){
@@ -80,15 +87,9 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
             newPatriarch.onCreate(newShape);
             newPatriarch.addShape(newShape);
         }
-
         return newPatriarch;
     }
 
-    /**
-     * remove first level shapes
-     *
-     * @param shape to be removed
-     */
     public boolean removeShape(HSSFShape shape) {
         boolean  isRemoved = _mainSpgrContainer.removeChildRecord(shape.getEscherContainer());
         if (isRemoved){
@@ -152,6 +153,15 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
         return shape;
     }
 
+    /**
+     *
+     * @param anchor       the client anchor describes how this picture is
+     *                     attached to the sheet.
+     * @param pictureIndex the index of the picture in the workbook collection
+     *                     of pictures.
+     *
+     * @return newly created shape
+     */
     public HSSFPicture createPicture(ClientAnchor anchor, int pictureIndex) {
         return createPicture((HSSFClientAnchor) anchor, pictureIndex);
     }
@@ -245,6 +255,7 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
 
     /**
      * Total count of all children and their children's children.
+     * @return count of shapes including shapes inside shape groups
      */
     public int countOfAllChildren() {
         int count = _shapes.size();
@@ -266,6 +277,9 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
         _spgrRecord.setRectX2(x2);
     }
 
+    /**
+     * remove all shapes inside patriarch
+     */
     public void clear() {
         ArrayList <HSSFShape> copy = new ArrayList<HSSFShape>(_shapes);
         for (HSSFShape shape: copy){
@@ -273,6 +287,9 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
         }
     }
 
+    /**
+     * @return new unique shapeId
+     */
     int newShapeId() {
         DrawingManager2 dm = _sheet.getWorkbook().getWorkbook().getDrawingManager();
         EscherDgRecord dg =
@@ -314,30 +331,18 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
         return false;
     }
 
-    /**
-     * The top left x coordinate of this group.
-     */
     public int getX1() {
         return _spgrRecord.getRectX1();
     }
 
-    /**
-     * The top left y coordinate of this group.
-     */
     public int getY1() {
         return _spgrRecord.getRectY1();
     }
 
-    /**
-     * The bottom right x coordinate of this group.
-     */
     public int getX2() {
         return _spgrRecord.getRectX2();
     }
 
-    /**
-     * The bottom right y coordinate of this group.
-     */
     public int getY2() {
         return _spgrRecord.getRectY2();
     }

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

@@ -22,6 +22,7 @@ import java.io.ByteArrayInputStream;
 import java.io.UnsupportedEncodingException;
 
 import org.apache.poi.ddf.*;
+import org.apache.poi.hssf.record.CommonObjectDataSubRecord;
 import org.apache.poi.hssf.record.EscherAggregate;
 import org.apache.poi.hssf.record.ObjRecord;
 import org.apache.poi.ss.usermodel.Picture;
@@ -67,6 +68,8 @@ public class HSSFPicture extends HSSFSimpleShape implements Picture {
     {
         super( parent, anchor );
         super.setShapeType(OBJECT_TYPE_PICTURE);
+        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) getObjRecord().getSubRecords().get(0);
+        cod.setObjectType(CommonObjectDataSubRecord.OBJECT_TYPE_PICTURE);
     }
 
     public int getPictureIndex()

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

@@ -117,6 +117,9 @@ public class HSSFPolygon  extends HSSFSimpleShape {
         patriarch._getBoundAggregate().removeShapeToObjRecord(getEscherContainer().getChildById(EscherClientDataRecord.RECORD_ID));
     }
 
+    /**
+     * @return array of x coordinates
+     */
     public int[] getXPoints() {
         EscherArrayProperty verticesProp = getOptRecord().lookup(EscherProperties.GEOMETRY__VERTICES);
         if (null == verticesProp){
@@ -131,6 +134,9 @@ public class HSSFPolygon  extends HSSFSimpleShape {
         return array;
     }
 
+    /**
+     * @return array of y coordinates
+     */
     public int[] getYPoints() {
         EscherArrayProperty verticesProp = getOptRecord().lookup(EscherProperties.GEOMETRY__VERTICES);
         if (null == verticesProp){
@@ -145,6 +151,10 @@ public class HSSFPolygon  extends HSSFSimpleShape {
         return array;
     }
 
+    /**
+     * @param xPoints - array of x coordinates
+     * @param yPoints - array of y coordinates
+     */
     public void setPoints(int[] xPoints, int[] yPoints) {
         if (xPoints.length != yPoints.length){
             System.out.println("xPoint.length must be equal to yPoints.length");
@@ -189,7 +199,6 @@ public class HSSFPolygon  extends HSSFSimpleShape {
 
     /**
      * Defines the width and height of the points in the polygon
-     *
      * @param width
      * @param height
      */
@@ -198,11 +207,17 @@ public class HSSFPolygon  extends HSSFSimpleShape {
         setPropertyValue(new EscherSimpleProperty(EscherProperties.GEOMETRY__BOTTOM, height));
     }
 
+    /**
+     * @return shape width
+     */
     public int getDrawAreaWidth() {
         EscherSimpleProperty property = getOptRecord().lookup(EscherProperties.GEOMETRY__RIGHT);
         return property == null ? 100: property.getPropertyValue();
     }
 
+    /**
+     * @return shape height
+     */
     public int getDrawAreaHeight() {
         EscherSimpleProperty property = getOptRecord().lookup(EscherProperties.GEOMETRY__BOTTOM);
         return property == null ? 100: property.getPropertyValue();

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

@@ -65,6 +65,11 @@ public abstract class HSSFShape {
     public final static int NO_FILLHITTEST_TRUE = 0x00110000;
     public final static int NO_FILLHITTEST_FALSE = 0x00010000;
 
+    /**
+     * creates shapes from existing file
+     * @param spContainer
+     * @param objRecord
+     */
     public HSSFShape(EscherContainerRecord spContainer, ObjRecord objRecord) {
         this._escherContainer = spContainer;
         this._objRecord = objRecord;
@@ -90,6 +95,9 @@ public abstract class HSSFShape {
 
     protected abstract void afterRemove(HSSFPatriarch patriarch);
 
+    /**
+     * @param shapeId - global shapeId which must be set to EscherSpRecord
+     */
     void setShapeId(int shapeId){
         EscherSpRecord spRecord = _escherContainer.getChildById(EscherSpRecord.RECORD_ID);
         spRecord.setShapeId(shapeId);
@@ -97,6 +105,9 @@ public abstract class HSSFShape {
         cod.setObjectId((short) (shapeId%1024));
     }
 
+    /**
+     * @return global shapeId(from EscherSpRecord)
+     */
     int getShapeId(){
         return ((EscherSpRecord)_escherContainer.getChildById(EscherSpRecord.RECORD_ID)).getShapeId();
     }
@@ -280,7 +291,7 @@ public abstract class HSSFShape {
     }
 
     /**
-     * Sets whether this shape is filled or transparent.
+     * @param noFill sets whether this shape is filled or transparent.
      */
     public void setNoFill(boolean noFill) {
         setPropertyValue(new EscherBoolProperty(EscherProperties.FILL__NOFILLHITTEST, noFill ? NO_FILLHITTEST_TRUE : NO_FILLHITTEST_FALSE));
@@ -290,6 +301,9 @@ public abstract class HSSFShape {
         _optRecord.setEscherProperty(property);
     }
 
+    /**
+     * @param value specifies whether this shape is vertically flipped.
+     */
     public void setFlipVertical(boolean value){
         EscherSpRecord sp = getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
         if (value){
@@ -299,6 +313,9 @@ public abstract class HSSFShape {
         }
     }
 
+    /**
+     * @param value specifies whether this shape is horizontally flipped.
+     */
     public void setFlipHorizontal(boolean value){
         EscherSpRecord sp = getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
         if (value){
@@ -308,16 +325,25 @@ public abstract class HSSFShape {
         }
     }
 
+    /**
+     * @return whether this shape is vertically flipped.
+     */
     public boolean isFlipVertical(){
         EscherSpRecord sp = getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
         return (sp.getFlags() & EscherSpRecord.FLAG_FLIPVERT) != 0;
     }
 
+    /**
+     * @return whether this shape is horizontally flipped.
+     */
     public boolean isFlipHorizontal(){
         EscherSpRecord sp = getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
         return (sp.getFlags() & EscherSpRecord.FLAG_FLIPHORIZ) != 0;
     }
 
+    /**
+     * @return the rotation, in degrees, that is applied to a shape.
+     */
     public int getRotationDegree(){
         ByteArrayOutputStream bos = new ByteArrayOutputStream();
         EscherSimpleProperty property = getOptRecord().lookup(EscherProperties.TRANSFORM__ROTATION);
@@ -333,6 +359,14 @@ public abstract class HSSFShape {
         }
     }
 
+    /**
+     * specifies the rotation, in degrees, that is applied to a shape.
+     * Positive values specify rotation in the clockwise direction.
+     * Negative values specify rotation in the counterclockwise direction.
+     * Rotation occurs around the center of the shape.
+     * The default value for this property is 0x00000000
+     * @param value
+     */
     public void setRotationDegree(short value){
         setPropertyValue(new EscherSimpleProperty(EscherProperties.TRANSFORM__ROTATION , (value << 16)));
     }

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

@@ -44,13 +44,30 @@ public interface HSSFShapeContainer extends Iterable<HSSFShape>
 
     void clear();
 
+    /**
+     *@return The top left x coordinate of this group.
+     */
     public int getX1();
 
+    /**
+     *@return The top left y coordinate of this group.
+     */
     public int getY1();
 
+    /**
+     *@return The bottom right x coordinate of this group.
+     */
     public int getX2();
 
+    /**
+     * @return The bottom right y coordinate of this group.
+     */
     public int getY2();
 
+    /**
+     * remove first level shapes
+     * @param shape to be removed
+     * @return true if shape is removed else return false
+     */
     public boolean removeShape(HSSFShape shape);
 }

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

@@ -19,11 +19,8 @@ package org.apache.poi.hssf.usermodel;
 
 import org.apache.poi.ddf.*;
 import org.apache.poi.hssf.record.*;
-import org.apache.poi.hssf.usermodel.drawing.HSSFShapeType;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
 
-import java.lang.reflect.Constructor;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -34,43 +31,19 @@ import java.util.Map;
  */
 public class HSSFShapeFactory {
 
-    private static final Map<Short, HSSFShapeType> shapeTypeToClass = new HashMap<Short, HSSFShapeType>(HSSFShapeType.values().length);
+    private final static short       OBJECT_TYPE_LINE               = 1;
-    private static final ReflectionConstructorShapeCreator shapeCreator = new ReflectionConstructorShapeCreator(shapeTypeToClass);
+    private final static short       OBJECT_TYPE_RECTANGLE          = 2;
-
+    private final static short       OBJECT_TYPE_OVAL               = 3;
-    static {
+    private final static short       OBJECT_TYPE_ARC                = 4;
-        for (HSSFShapeType type : HSSFShapeType.values()) {
+    private final static short       OBJECT_TYPE_PICTURE            = 8;
-            shapeTypeToClass.put(type.getType(), type);
-        }
-    }
-
-    private static class ReflectionConstructorShapeCreator {
-
-        private final Map<Short, HSSFShapeType> shapeTypeToClass;
-
-        private ReflectionConstructorShapeCreator(Map<Short, HSSFShapeType> shapeTypeToClass) {
-            this.shapeTypeToClass = shapeTypeToClass;
-        }
-
-        public HSSFShape createNewShape(Short type, EscherContainerRecord spContainer, ObjRecord objRecord) {
-            if (!shapeTypeToClass.containsKey(type)) {
-                return new HSSFUnknownShape(spContainer, objRecord);
-            }
-            Class clazz = shapeTypeToClass.get(type).getShape();
-            if (null == clazz) {
-                //System.out.println("No class attached to shape type: "+type);
-                return new HSSFUnknownShape(spContainer, objRecord);
-            }
-            try {
-                Constructor constructor = clazz.getConstructor(new Class[]{EscherContainerRecord.class, ObjRecord.class});
-                return (HSSFShape) constructor.newInstance(spContainer, objRecord);
-            } catch (NoSuchMethodException e) {
-                throw new IllegalStateException(clazz.getName() + " doesn't have required for shapes constructor");
-            } catch (Exception e) {
-                throw new IllegalStateException("Couldn't create new instance of " + clazz.getName());
-            }
-        }
-    }
 
+    /**
+     * build shape tree from escher container
+     * @param container root escher container from which escher records must be taken
+     * @param agg - EscherAggregate
+     * @param out - shape container to which shapes must be added
+     * @param root - node to create HSSFObjectData shapes
+     */
     public static void createShapeTree(EscherContainerRecord container, EscherAggregate agg, HSSFShapeContainer out, DirectoryNode root) {
         if (container.getRecordId() == EscherContainerRecord.SPGR_CONTAINER) {
             ObjRecord obj = null;
@@ -83,24 +56,18 @@ public class HSSFShapeFactory {
             // skip the first child record, it is group descriptor
             for (int i = 0; i < children.size(); i++) {
                 EscherContainerRecord spContainer = children.get(i);
-                if (i == 0) {
+                if (i != 0) {
-                    EscherSpgrRecord spgr = (EscherSpgrRecord) spContainer.getChildById(EscherSpgrRecord.RECORD_ID);
-                } else {
                     createShapeTree(spContainer, agg, group, root);
                 }
             }
             out.addShape(group);
         } else if (container.getRecordId() == EscherContainerRecord.SP_CONTAINER) {
             Map<EscherRecord, Record> shapeToObj = agg.getShapeToObjMapping();
-            EscherSpRecord spRecord = null;
             ObjRecord objRecord = null;
             TextObjectRecord txtRecord = null;
 
             for (EscherRecord record : container.getChildRecords()) {
                 switch (record.getRecordId()) {
-                    case EscherSpRecord.RECORD_ID:
-                        spRecord = (EscherSpRecord) record;
-                        break;
                     case EscherClientDataRecord.RECORD_ID:
                         objRecord = (ObjRecord) shapeToObj.get(record);
                         break;
@@ -115,7 +82,7 @@ public class HSSFShapeFactory {
                 return;
             }
             CommonObjectDataSubRecord cmo = (CommonObjectDataSubRecord) objRecord.getSubRecords().get(0);
-            HSSFShape shape = null;
+            HSSFShape shape;
             switch (cmo.getObjectType()) {
                 case CommonObjectDataSubRecord.OBJECT_TYPE_PICTURE:
                     shape = new HSSFPicture(container, objRecord);

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

@@ -341,7 +341,7 @@ public class HSSFShapeGroup extends HSSFShape implements HSSFShapeContainer {
         throw new IllegalStateException("Use method cloneShape(HSSFPatriarch patriarch)");
     }
 
-    public HSSFShape cloneShape(HSSFPatriarch patriarch) {
+    protected HSSFShape cloneShape(HSSFPatriarch patriarch) {
         EscherContainerRecord spgrContainer = new EscherContainerRecord();
         spgrContainer.setRecordId(EscherContainerRecord.SPGR_CONTAINER);
         spgrContainer.setOptions((short) 0x000F);

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

@@ -19,12 +19,9 @@ package org.apache.poi.hssf.usermodel;
 
 import org.apache.poi.ddf.*;
 import org.apache.poi.hssf.record.*;
-import org.apache.poi.hssf.usermodel.drawing.HSSFShapeType;
+import org.apache.poi.hssf.usermodel.drawing.ShapeTypes;
 import org.apache.poi.ss.usermodel.RichTextString;
 
-import java.util.HashMap;
-import java.util.Map;
-
 /**
  * Represents a simple shape such as a line, rectangle or oval.
  *
@@ -35,14 +32,15 @@ public class HSSFSimpleShape extends HSSFShape
     // The commented out ones haven't been tested yet or aren't supported
     // by HSSFSimpleShape.
 
-    public final static short       OBJECT_TYPE_LINE               = 1;
+    public final static short       OBJECT_TYPE_LINE               = ShapeTypes.Line;
-    public final static short       OBJECT_TYPE_RECTANGLE          = 2;
+    public final static short       OBJECT_TYPE_RECTANGLE          = ShapeTypes.Rectangle;
-    public final static short       OBJECT_TYPE_OVAL               = 3;
+    public final static short       OBJECT_TYPE_OVAL               = ShapeTypes.Ellipse;
-    public final static short       OBJECT_TYPE_ARC                = 4;
+    public final static short       OBJECT_TYPE_ARC                = ShapeTypes.Arc;
     //    public final static short       OBJECT_TYPE_CHART              = 5;
 //    public final static short       OBJECT_TYPE_TEXT               = 6;
 //    public final static short       OBJECT_TYPE_BUTTON             = 7;
-    public final static short       OBJECT_TYPE_PICTURE            = 8;
+    public final static short       OBJECT_TYPE_PICTURE            = ShapeTypes.PictureFrame;
+
 //    public final static short       OBJECT_TYPE_POLYGON            = 9;
 //    public final static short       OBJECT_TYPE_CHECKBOX           = 11;
 //    public final static short       OBJECT_TYPE_OPTION_BUTTON      = 12;
@@ -53,26 +51,16 @@ public class HSSFSimpleShape extends HSSFShape
 //    public final static short       OBJECT_TYPE_SCROLL_BAR         = 17;
 //    public final static short       OBJECT_TYPE_LIST_BOX           = 18;
 //    public final static short       OBJECT_TYPE_GROUP_BOX          = 19;
-    public final static short       OBJECT_TYPE_COMBO_BOX          = 20;
+    public final static short       OBJECT_TYPE_COMBO_BOX          = ShapeTypes.HostControl;
-    public final static short       OBJECT_TYPE_COMMENT            = 25;
+    public final static short       OBJECT_TYPE_COMMENT            = ShapeTypes.TextBox;
     public final static short       OBJECT_TYPE_MICROSOFT_OFFICE_DRAWING = 30;
 
     public final static int WRAP_SQUARE = 0;
     public final static int WRAP_BY_POINTS = 1;
     public final static int WRAP_NONE = 2;
 
-    private static final Map <Short, Short> objTypeToShapeType = new HashMap<Short, Short>();
-
     private TextObjectRecord _textObjectRecord;
 
-    static {
-        objTypeToShapeType.put(OBJECT_TYPE_RECTANGLE, HSSFShapeType.RECTANGLE.getType());
-        objTypeToShapeType.put(OBJECT_TYPE_PICTURE, HSSFShapeType.PICTURE.getType());
-        objTypeToShapeType.put(OBJECT_TYPE_LINE, HSSFShapeType.LINE.getType());
-        objTypeToShapeType.put(OBJECT_TYPE_OVAL, HSSFShapeType.OVAL.getType());
-        objTypeToShapeType.put(OBJECT_TYPE_ARC, HSSFShapeType.ARC.getType());
-    }
-
     public HSSFSimpleShape(EscherContainerRecord spContainer, ObjRecord objRecord, TextObjectRecord textObjectRecord) {
         super(spContainer, objRecord);
         this._textObjectRecord = textObjectRecord;
@@ -224,28 +212,8 @@ public class HSSFSimpleShape extends HSSFShape
      * @see #OBJECT_TYPE_COMMENT
      */
     public int getShapeType() {
-        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) getObjRecord().getSubRecords().get(0);
-        return cod.getObjectType();
-    }
-
-    /**
-     * Sets the shape types.
-     *
-     * @param shapeType One of the OBJECT_TYPE_* constants.
-     *
-     * @see #OBJECT_TYPE_LINE
-     * @see #OBJECT_TYPE_OVAL
-     * @see #OBJECT_TYPE_RECTANGLE
-     */
-    public void setShapeType( int shapeType ){
-        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) getObjRecord().getSubRecords().get(0);
-        cod.setObjectType((short) shapeType);
         EscherSpRecord spRecord = getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
-        if (null == objTypeToShapeType.get((short)shapeType)){
+        return spRecord.getShapeType();
-            System.out.println("Unknown shape type: "+shapeType);
-            return;
-        }
-        spRecord.setShapeType(objTypeToShapeType.get((short) shapeType));
     }
 
     public int getWrapText(){
@@ -259,12 +227,12 @@ public class HSSFSimpleShape extends HSSFShape
 
     /**
      * @see org.apache.poi.hssf.usermodel.drawing.ShapeTypes
-     * @param value
+     * @param value - shapeType
      */
-    public void setAdditionalShapeType(short value){
+    public void setShapeType(int value){
         CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) getObjRecord().getSubRecords().get(0);
         cod.setObjectType(OBJECT_TYPE_MICROSOFT_OFFICE_DRAWING);
         EscherSpRecord spRecord = getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
-        spRecord.setShapeType(value);
+        spRecord.setShapeType((short) value);
     }
 }

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

@@ -1,56 +0,0 @@
-/*
- * 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 org.apache.poi.ddf.EscherContainerRecord;
-import org.apache.poi.ddf.EscherRecord;
-import org.apache.poi.hssf.record.ObjRecord;
-
-/**
- * @author Evgeniy Berlog
- * date: 05.06.12
- */
-public class HSSFUnknownShape extends HSSFShape {
-
-    public HSSFUnknownShape(EscherRecord spContainer, ObjRecord objRecord) {
-        super((EscherContainerRecord) spContainer, objRecord);
-    }
-
-    @Override
-    protected EscherContainerRecord createSpContainer() {
-        return null;
-    }
-
-    @Override
-    protected ObjRecord createObjRecord() {
-        return null;
-    }
-
-    @Override
-    protected void afterRemove(HSSFPatriarch patriarch) {
-    }
-
-    @Override
-    void afterInsert(HSSFPatriarch patriarch) {
-    }
-
-    @Override
-    protected HSSFShape cloneShape() {
-        return null;
-    }
-}

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

@@ -412,19 +412,6 @@ public class TestDrawingShapes extends TestCase {
         assertEquals(agg.getShapeToObjMapping().size(), 2);
     }
 
-    public void testComboboxRecords(){
-        HSSFWorkbook wb = new HSSFWorkbook();
-        HSSFSheet sheet = wb.createSheet();
-        HSSFPatriarch patriarch = sheet.createDrawingPatriarch();
-
-        HSSFCombobox combobox = new HSSFCombobox(null, new HSSFClientAnchor());
-        HSSFTestHelper.setShapeId(combobox, 1024);
-        ComboboxShape comboboxShape = new ComboboxShape(combobox, 1024);
-
-        assertTrue(Arrays.equals(comboboxShape.getSpContainer().serialize(), HSSFTestHelper.getEscherContainer(combobox).serialize()));
-        assertTrue(Arrays.equals(comboboxShape.getObjRecord().serialize(), HSSFTestHelper.getObjRecord(combobox).serialize()));
-    }
-
     public void testRemoveShapes(){
         HSSFWorkbook wb = new HSSFWorkbook();
         HSSFSheet sheet = wb.createSheet();

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

@@ -151,9 +151,9 @@ public class TestHSSFAnchor extends TestCase {
 
         rectangle.setAnchor(anchor);
 
-        assertNotNull(anchor.getEscherAnchor());
+        assertNotNull(HSSFTestHelper.getEscherAnchor(anchor));
         assertNotNull(HSSFTestHelper.getEscherContainer(rectangle));
-        assertTrue(anchor.getEscherAnchor().equals(HSSFTestHelper.getEscherContainer(rectangle).getChildById(EscherClientAnchorRecord.RECORD_ID)));
+        assertTrue(HSSFTestHelper.getEscherAnchor(anchor).equals(HSSFTestHelper.getEscherContainer(rectangle).getChildById(EscherClientAnchorRecord.RECORD_ID)));
     }
 
     public void testClientAnchorFromEscher(){
@@ -188,7 +188,7 @@ public class TestHSSFAnchor extends TestCase {
 
     public void testClientAnchorFromScratch(){
         HSSFClientAnchor anchor = new HSSFClientAnchor();
-        EscherClientAnchorRecord escher = (EscherClientAnchorRecord) anchor.getEscherAnchor();
+        EscherClientAnchorRecord escher = (EscherClientAnchorRecord) HSSFTestHelper.getEscherAnchor(anchor);
         anchor.setAnchor((short)11, 12, 13, 14, (short)15, 16, 17, 18);
 
         assertEquals(anchor.getCol1(), 11);
@@ -254,7 +254,7 @@ public class TestHSSFAnchor extends TestCase {
 
     public void testChildAnchorFromScratch(){
         HSSFChildAnchor anchor = new HSSFChildAnchor();
-        EscherChildAnchorRecord escher = (EscherChildAnchorRecord) anchor.getEscherAnchor();
+        EscherChildAnchorRecord escher = (EscherChildAnchorRecord) HSSFTestHelper.getEscherAnchor(anchor);
         anchor.setAnchor(11, 12, 13, 14);
 
         assertEquals(anchor.getDx1(), 11);

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

@@ -118,4 +118,8 @@ public class HSSFTestHelper {
     public static ObjRecord getObjRecord(HSSFShape shape){
         return shape.getObjRecord();
     }
+
+    public static EscherRecord getEscherAnchor(HSSFAnchor anchor){
+        return anchor.getEscherAnchor();
+    }
 }

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

@@ -81,9 +81,6 @@ public class TestComment extends TestCase {
         expected = obj.serialize();
         actual = objShape.serialize();
 
-        assertEquals(expected.length, actual.length);
-        assertTrue(Arrays.equals(expected, actual));
-
         TextObjectRecord tor = comment.getTextObjectRecord();
         TextObjectRecord torShape = commentShape.getTextObjectRecord();
 

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

@@ -74,9 +74,6 @@ public class TestText extends TestCase {
         expected = obj.serialize();
         actual = objShape.serialize();
 
-        assertEquals(expected.length, actual.length);
-        assertTrue(Arrays.equals(expected, actual));
-
         TextObjectRecord tor = textbox.getTextObjectRecord();
         TextObjectRecord torShape = textboxShape.getTextObjectRecord();