Review EscherContainerRecord#getChildRecords() call sites for unnecessary work

This started off as wanting to add the EscherContainerRecord#getChildCount() function in order to do an efficient check for how many children the container has. This was desirable in new code for editing HSSF pictures. The existing option of calling getChildRecords().size() was undesirable as this requires a list copy first.

In the process of finding call sites that would benefit from replacing getChildRecords().size(), I realized that several other patterns would benefit from eliminating a copy, such as iterating over the children in a for-each loop, and indexed access to specific children.

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

src/java/org/apache/poi/ddf/EscherContainerRecord.java

@@ -158,6 +158,10 @@ public final class EscherContainerRecord extends EscherRecord implements Iterabl
         return new ArrayList<>(_childRecords);
     }
 
+    public int getChildCount() {
+        return _childRecords.size();
+    }
+
     /**
      * @return an iterator over the child records
      */

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

@@ -1880,7 +1880,7 @@ public final class InternalWorkbook {
 
         DrawingManager2 dm = new DrawingManager2(dgg);
         if(bStore != null){
-            for(EscherRecord bs : bStore.getChildRecords()){
+            for (EscherRecord bs : bStore) {
                 if(bs instanceof EscherBSERecord) {
                     escherBSERecords.add((EscherBSERecord)bs);
                 }

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

@@ -186,9 +186,9 @@ public abstract class HSSFShape implements Shape {
                 throw new IllegalArgumentException("Must use client anchors for shapes directly attached to sheet.");
             EscherClientAnchorRecord anch = _escherContainer.getChildById(EscherClientAnchorRecord.RECORD_ID);
             if (null != anch) {
-                for (i=0; i< _escherContainer.getChildRecords().size(); i++){
+                for (i=0; i< _escherContainer.getChildCount(); i++){
                     if (_escherContainer.getChild(i).getRecordId() == EscherClientAnchorRecord.RECORD_ID){
-                        if (i != _escherContainer.getChildRecords().size() -1){
+                        if (i != _escherContainer.getChildCount() -1){
                             recordId = _escherContainer.getChild(i+1).getRecordId();
                         }
                     }
@@ -200,9 +200,9 @@ public abstract class HSSFShape implements Shape {
                 throw new IllegalArgumentException("Must use child anchors for shapes attached to groups.");
             EscherChildAnchorRecord anch = _escherContainer.getChildById(EscherChildAnchorRecord.RECORD_ID);
             if (null != anch) {
-                for (i=0; i< _escherContainer.getChildRecords().size(); i++){
+                for (i=0; i< _escherContainer.getChildCount(); i++){
                     if (_escherContainer.getChild(i).getRecordId() == EscherChildAnchorRecord.RECORD_ID){
-                        if (i != _escherContainer.getChildRecords().size() -1){
+                        if (i != _escherContainer.getChildCount() -1){
                             recordId = _escherContainer.getChild(i+1).getRecordId();
                         }
                     }

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

@@ -54,7 +54,7 @@ public class HSSFShapeGroup extends HSSFShape implements HSSFShapeContainer {
         // read internal and external coordinates from spgrContainer
         EscherContainerRecord spContainer = spgrContainer.getChildContainers().get(0);
         _spgrRecord = (EscherSpgrRecord) spContainer.getChild(0);
-        for (EscherRecord ch : spContainer.getChildRecords()) {
+        for (EscherRecord ch : spContainer) {
             switch (EscherRecordTypes.forTypeID(ch.getRecordId())) {
                 case SPGR:
                     break;

src/scratchpad/src/org/apache/poi/hslf/dev/SlideShowRecordDumper.java

@@ -213,11 +213,10 @@ public final class SlideShowRecordDumper {
         ps.println(ind + "  options: 0x" + HexDump.toHex( ecr.getOptions() ));
         ps.println(ind + "  recordId: 0x" + HexDump.toHex( ecr.getRecordId() ));
         
-        List<EscherRecord> childRecords = ecr.getChildRecords();
-        ps.println(ind + "  numchildren: " + childRecords.size());
+        ps.println(ind + "  numchildren: " + ecr.getChildCount());
         ps.println(ind + "  children: ");
         int count = 0;
-        for ( EscherRecord record : childRecords ) {
+        for ( EscherRecord record : ecr ) {
             ps.println(ind + "   Child " + count + ":");
             printEscherRecord(record, indent+1);
             count++;

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

@@ -459,8 +459,7 @@ public final class HSLFFill {
             LOG.atDebug().log("EscherContainerRecord.BSTORE_CONTAINER was not found ");
             return null;
         }
-        List<EscherRecord> lst = bstore.getChildRecords();
-        return (EscherBSERecord)lst.get(idx-1);
+        return (EscherBSERecord) bstore.getChild(idx-1);
     }
 
     /**
@@ -563,12 +562,11 @@ public final class HSLFFill {
         EscherContainerRecord dggContainer = doc.getPPDrawingGroup().getDggContainer();
         EscherContainerRecord bstore = HSLFShape.getEscherChild(dggContainer, EscherContainerRecord.BSTORE_CONTAINER);
 
-        List<EscherRecord> lst = bstore.getChildRecords();
         int idx = p.getPropertyValue();
         if (idx == 0){
             LOG.atWarn().log("no reference to picture data found ");
         } else {
-            EscherBSERecord bse = (EscherBSERecord)lst.get(idx - 1);
+            EscherBSERecord bse = (EscherBSERecord) bstore.getChild(idx - 1);
             for (HSLFPictureData pd : pict) {
 
                 // Reference equals is safe because these BSE belong to the same slideshow

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

@@ -146,13 +146,13 @@ public class HSLFPictureShape extends HSLFSimpleShape implements PictureShape<HS
             LOG.atDebug().log("EscherContainerRecord.BSTORE_CONTAINER was not found ");
             return null;
         }
-        List<EscherRecord> lst = bstore.getChildRecords();
+
         int idx = getPictureIndex();
         if (idx == 0){
             LOG.atDebug().log("picture index was not found, returning ");
             return null;
         }
-        return (EscherBSERecord)lst.get(idx-1);
+        return (EscherBSERecord) bstore.getChild(idx - 1);
     }
 
     /**

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

@@ -236,10 +236,7 @@ public abstract class HSLFSheet implements HSLFShapeContainer, Sheet<HSLFShape,H
             return false;
         }
 
-        List<EscherRecord> lst = spgr.getChildRecords();
-        boolean result = lst.remove(shape.getSpContainer());
-        spgr.setChildRecords(lst);
-        return result;
+        return spgr.removeChildRecord(shape.getSpContainer());
     }
 
     /**

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

@@ -1270,7 +1270,7 @@ public final class HSLFSlideShow extends POIDocument implements SlideShow<HSLFSh
 		record.setOffset(offset);
 
 		blipStore.addChildRecord(record);
-		int count = blipStore.getChildRecords().size();
+		int count = blipStore.getChildCount();
 		blipStore.setOptions((short) ((count << 4) | 0xF));
 
 		return record;

src/scratchpad/src/org/apache/poi/hwpf/model/OfficeArtContent.java

@@ -141,7 +141,7 @@ public final class OfficeArtContent {
                 1);
         for ( EscherContainerRecord dgContainer : getDgContainers() )
         {
-            for ( EscherRecord escherRecord : dgContainer.getChildRecords() )
+            for ( EscherRecord escherRecord : dgContainer )
             {
                 if ( escherRecord.getRecordId() == (short) 0xF003 )
                 {
@@ -158,7 +158,7 @@ public final class OfficeArtContent {
                 1);
         for ( EscherContainerRecord spgrContainer : getSpgrContainers() )
         {
-            for ( EscherRecord escherRecord : spgrContainer.getChildRecords() )
+            for ( EscherRecord escherRecord : spgrContainer )
             {
                 if ( escherRecord.getRecordId() == (short) 0xF004 )
                 {

src/scratchpad/src/org/apache/poi/hwpf/usermodel/OfficeDrawingsImpl.java

@@ -56,12 +56,10 @@ public class OfficeDrawingsImpl implements OfficeDrawings
         if (bContainer == null)
             return null;
 
-        final List<EscherRecord> bitmapRecords = bContainer.getChildRecords();
-
-        if ( bitmapRecords.size() < bitmapIndex )
+        if ( bContainer.getChildCount() < bitmapIndex )
             return null;
 
-        EscherRecord imageRecord = bitmapRecords.get( bitmapIndex - 1 );
+        EscherRecord imageRecord = bContainer.getChild( bitmapIndex - 1 );
 
         if ( imageRecord instanceof EscherBlipRecord )
         {

src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBackground.java

@@ -211,8 +211,7 @@ public final class TestBackground {
             Document doc = ppt.getDocumentRecord();
             EscherContainerRecord dggContainer = doc.getPPDrawingGroup().getDggContainer();
             EscherContainerRecord bstore = HSLFShape.getEscherChild(dggContainer, EscherContainerRecord.BSTORE_CONTAINER);
-            List<EscherRecord> lst = bstore.getChildRecords();
-            return ((EscherBSERecord)lst.get(idx-1)).getRef();
+            return ((EscherBSERecord) bstore.getChild(idx - 1)).getRef();
         }
         return 0;
     }

src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestPictures.java

@@ -712,7 +712,7 @@ public final class TestPictures {
         ByteArrayOutputStream inMemory = new ByteArrayOutputStream();
         try (HSLFSlideShow ppt = HSLFTestDataSamples.getSlideShow("pictures.ppt")) {
             originalOffsets = ppt.getPictureData().stream().mapToInt(HSLFPictureData::getOffset).toArray();
-            originalNumberOfRecords = ppt.getPictureData().get(0).bStore.getChildRecords().size();
+            originalNumberOfRecords = ppt.getPictureData().get(0).bStore.getChildCount();
 
             Random random = new Random();
             for (HSLFPictureData picture : ppt.getPictureData()) {
@@ -729,7 +729,7 @@ public final class TestPictures {
             assertArrayEquals(originalOffsets, offsets);
 
             // Verify that there are the same number of records as in the original slideshow.
-            int numberOfRecords = ppt.getPictureData().get(0).bStore.getChildRecords().size();
+            int numberOfRecords = ppt.getPictureData().get(0).bStore.getChildCount();
             assertEquals(originalNumberOfRecords, numberOfRecords);
         }
     }

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

@@ -315,7 +315,7 @@ class TestDrawingShapes {
         EscherContainerRecord spgrContainer =
                 agg1.getEscherContainer().getChildContainers().get(0);
         // root spContainer + 2 spContainers for shapes
-        assertEquals(3, spgrContainer.getChildRecords().size());
+        assertEquals(3, spgrContainer.getChildCount());
 
         EscherSpRecord sp0 =
                 ((EscherContainerRecord) spgrContainer.getChild(0)).getChildById(EscherSpRecord.RECORD_ID);

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

@@ -213,7 +213,7 @@ final class TestHSSFComment extends BaseTestCellComment {
         HSSFCell cell = row.createCell(0);
         cell.setCellComment(comment);
 
-        assertEquals(comment.getEscherContainer().getChildRecords().size(), 5);
+        assertEquals(comment.getEscherContainer().getChildCount(), 5);
 
         //sp record
         byte[] expected = decompress("H4sIAAAAAAAAAFvEw/WBg4GBgZEFSHAxMAAA9gX7nhAAAAA=");

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

@@ -43,7 +43,7 @@ class TestPolygon {
         polygon.setPoints( new int[]{0, 90, 50}, new int[]{5, 5, 44} );
         polygon.setShapeId(1024);
 
-        assertEquals(polygon.getEscherContainer().getChildRecords().size(), 4);
+        assertEquals(polygon.getEscherContainer().getChildCount(), 4);
 
         //sp record
         byte[] expected = decompress("H4sIAAAAAAAAAGNi4PrAwQAELEDMxcAAAAU6ZlwQAAAA");

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

@@ -41,7 +41,7 @@ class TestText {
         HSSFPatriarch patriarch = sh.createDrawingPatriarch();
         HSSFTextbox textbox = patriarch.createTextbox(new HSSFClientAnchor());
 
-        assertEquals(textbox.getEscherContainer().getChildRecords().size(), 5);
+        assertEquals(textbox.getEscherContainer().getChildCount(), 5);
 
         //sp record
         byte[] expected = decompress("H4sIAAAAAAAAAFvEw/WBg4GBgZEFSHAxMAAA9gX7nhAAAAA=");