#58204 - STYLE: ShapeContainer interface makes internal getShapesList() redundant

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

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFGroupShape.java

@@ -144,8 +144,7 @@ public class XSLFGroupShape extends XSLFShape implements XSLFShapeContainer, Gro
     }
 
     /**
-     *
+     * @return child shapes contained within this group
-     * @return child shapes contained witin this group
      */
     @Override
     public List<XSLFShape> getShapes(){

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java

@@ -25,7 +25,6 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-
 import javax.xml.namespace.QName;
 
 import org.apache.poi.POIXMLDocumentPart;
@@ -126,57 +125,70 @@ public abstract class XSLFSheet extends POIXMLDocumentPart implements XSLFShapeC
     }
 
     private XSLFDrawing getDrawing(){
-        if(_drawing == null) {
+        initDrawingAndShapes();
-            _drawing = new XSLFDrawing(this, getSpTree());
-        }
         return _drawing;
     }
 
-    private List<XSLFShape> getShapeList(){
+    /**
-        if(_shapes == null){
+     * Returns an array containing all of the shapes in this sheet
-            _shapes = buildShapes(getSpTree());
+     *
-        }
+     * @return an array of all shapes in this sheet
+     */
+    @Override
+    public List<XSLFShape> getShapes(){
+        initDrawingAndShapes();
         return _shapes;
     }
     
+    /**
+     * Helper method for initializing drawing and shapes in one go.
+     * If they are initialized separately, there's a risk that shapes
+     * get added twice, e.g. a shape is added to the drawing, then
+     * buildShapes is called and at last the shape is added to shape list
+     */
+    private void initDrawingAndShapes() {
+        CTGroupShape cgs = getSpTree();
+        if(_drawing == null) {
+            _drawing = new XSLFDrawing(this, cgs);
+        }
+        if (_shapes == null) {
+            _shapes = buildShapes(cgs);
+        }
+    }
+
     // shape factory methods
 
     public XSLFAutoShape createAutoShape(){
-        List<XSLFShape> shapes = getShapeList();
         XSLFAutoShape sh = getDrawing().createAutoShape();
-        shapes.add(sh);
+        getShapes().add(sh);
         sh.setParent(this);
         return sh;
     }
 
     public XSLFFreeformShape createFreeform(){
-        List<XSLFShape> shapes = getShapeList();
         XSLFFreeformShape sh = getDrawing().createFreeform();
-        shapes.add(sh);
+        getShapes().add(sh);
         sh.setParent(this);
         return sh;
     }
 
     public XSLFTextBox createTextBox(){
-        List<XSLFShape> shapes = getShapeList();
         XSLFTextBox sh = getDrawing().createTextBox();
-        shapes.add(sh);
+        getShapes().add(sh);
         sh.setParent(this);
         return sh;
     }
 
     public XSLFConnectorShape createConnector(){
-        List<XSLFShape> shapes = getShapeList();
         XSLFConnectorShape sh = getDrawing().createConnector();
-        shapes.add(sh);
+        getShapes().add(sh);
         sh.setParent(this);
         return sh;
     }
 
     public XSLFGroupShape createGroup(){
-        List<XSLFShape> shapes = getShapeList();
         XSLFGroupShape sh = getDrawing().createGroup();
-        shapes.add(sh);
+        getShapes().add(sh);
         sh.setParent(this);
         return sh;
     }
@@ -190,36 +202,25 @@ public abstract class XSLFSheet extends POIXMLDocumentPart implements XSLFShapeC
 
         XSLFPictureShape sh = getDrawing().createPicture(rel.getId());
         sh.resize();
-
+        getShapes().add(sh);
-        getShapeList().add(sh);
         sh.setParent(this);
         return sh;
     }
 
     public XSLFTable createTable(){
-        List<XSLFShape> shapes = getShapeList();
         XSLFTable sh = getDrawing().createTable();
-        shapes.add(sh);
+        getShapes().add(sh);
         sh.setParent(this);
         return sh;
     }
 
-    /**
-     * Returns an array containing all of the shapes in this sheet
-     *
-     * @return an array of all shapes in this sheet
-     */
-    public List<XSLFShape> getShapes(){
-        return getShapeList();
-    }
-
     /**
      * Returns an iterator over the shapes in this sheet
      *
      * @return an iterator over the shapes in this sheet
      */
     public Iterator<XSLFShape> iterator(){
-        return getShapeList().iterator();
+        return getShapes().iterator();
     }
 
     public void addShape(XSLFShape shape) {
@@ -250,7 +251,7 @@ public abstract class XSLFSheet extends POIXMLDocumentPart implements XSLFShapeC
         } else {
             throw new IllegalArgumentException("Unsupported shape: " + xShape);
         }
-        return getShapeList().remove(xShape);
+        return getShapes().remove(xShape);
     }
 
     /**
@@ -319,8 +320,8 @@ public abstract class XSLFSheet extends POIXMLDocumentPart implements XSLFShapeC
         getSpTree().set(src.getSpTree());
 
         // recursively update each shape
-        List<XSLFShape> tgtShapes = getShapeList();
+        List<XSLFShape> tgtShapes = getShapes();
-        List<XSLFShape> srcShapes = src.getShapeList();
+        List<XSLFShape> srcShapes = src.getShapes();
         for(int i = 0; i < tgtShapes.size(); i++){
             XSLFShape s1 = srcShapes.get(i);
             XSLFShape s2 = tgtShapes.get(i);
@@ -338,7 +339,7 @@ public abstract class XSLFSheet extends POIXMLDocumentPart implements XSLFShapeC
      */
     public XSLFSheet appendContent(XSLFSheet src){
         CTGroupShape spTree = getSpTree();
-        int numShapes = getShapeList().size();
+        int numShapes = getShapes().size();
 
         CTGroupShape srcTree = src.getSpTree();
         for(XmlObject ch : srcTree.selectPath("*")){
@@ -362,8 +363,8 @@ public abstract class XSLFSheet extends POIXMLDocumentPart implements XSLFShapeC
         _placeholders = null;
 
         // recursively update each shape
-        List<XSLFShape> tgtShapes = getShapeList();
+        List<XSLFShape> tgtShapes = getShapes();
-        List<XSLFShape> srcShapes = src.getShapeList();
+        List<XSLFShape> srcShapes = src.getShapes();
         for(int i = 0; i < srcShapes.size(); i++){
             XSLFShape s1 = srcShapes.get(i);
             XSLFShape s2 = tgtShapes.get(numShapes + i);

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFGroupShape.java

@@ -52,11 +52,6 @@ public class HSLFGroupShape extends HSLFShape implements GroupShape<HSLFShape> {
         super(escherRecord, parent);
     }
 
-    @Override
-    public List<HSLFShape> getShapes() {
-        return getShapeList();
-    }
-
     /**
      * Sets the anchor (the bounding box rectangle) of this shape.
      * All coordinates should be expressed in Master units (576 dpi).
@@ -238,7 +233,7 @@ public class HSLFGroupShape extends HSLFShape implements GroupShape<HSLFShape> {
     }
 
     public Iterator<HSLFShape> iterator() {
-        return getShapeList().iterator();
+        return getShapes().iterator();
     }
 
     public boolean removeShape(HSLFShape shape) {
@@ -249,7 +244,8 @@ public class HSLFGroupShape extends HSLFShape implements GroupShape<HSLFShape> {
     /**
      * @return the shapes contained in this group container
      */
-    protected List<HSLFShape> getShapeList() {
+    @Override
+    public List<HSLFShape> getShapes() {
         // Out escher container record should contain several
         //  SpContainers, the first of which is the group shape itself
         Iterator<EscherRecord> iter = _escherContainer.getChildIterator();

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTable.java

@@ -162,7 +162,7 @@ public final class HSLFTable extends HSLFGroupShape implements TableShape {
     }
 
     protected void initTable(){
-        List<HSLFShape> shapeList = getShapeList();
+        List<HSLFShape> shapeList = getShapes();
 
         Iterator<HSLFShape> shapeIter = shapeList.iterator();
         while (shapeIter.hasNext()) {