SonarQube fixes

add Iterable interface to EscherContainerRecord / deprecate getChildIterator()

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

13 files changed, 138 insertions, 157 deletions

diff --git a/src/java/org/apache/poi/ddf/EscherContainerRecord.java b/src/java/org/apache/poi/ddf/EscherContainerRecord.java
index 3b26d4211a..e570084b24 100644
--- a/src/java/org/apache/poi/ddf/EscherContainerRecord.java
+++ b/src/java/org/apache/poi/ddf/EscherContainerRecord.java
@@ -18,12 +18,16 @@
 package org.apache.poi.ddf;
 
 import java.io.PrintWriter;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
 
 import org.apache.poi.util.HexDump;
 import org.apache.poi.util.LittleEndian;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
+import org.apache.poi.util.Removal;
 
 /**
  * Escher container records store other escher records as children.
@@ -31,7 +35,7 @@ import org.apache.poi.util.POILogger;
  * the standard header used by all escher records.  This one record is
  * used to represent many different types of records.
  */
-public final class EscherContainerRecord extends EscherRecord {
+public final class EscherContainerRecord extends EscherRecord implements Iterable<EscherRecord> {
     public static final short DGG_CONTAINER    = (short)0xF000;
     public static final short BSTORE_CONTAINER = (short)0xF001;
     public static final short DG_CONTAINER     = (short)0xF002;
@@ -97,17 +101,13 @@ public final class EscherContainerRecord extends EscherRecord {
         LittleEndian.putShort(data, offset, getOptions());
         LittleEndian.putShort(data, offset+2, getRecordId());
         int remainingBytes = 0;
-        Iterator<EscherRecord> iterator = _childRecords.iterator();
-        while (iterator.hasNext()) {
-            EscherRecord r = iterator.next();
+        for (EscherRecord r : this) {
             remainingBytes += r.getRecordSize();
         }
         remainingBytes += _remainingLength;
         LittleEndian.putInt(data, offset+4, remainingBytes);
         int pos = offset+8;
-        iterator = _childRecords.iterator();
-        while (iterator.hasNext()) {
-            EscherRecord r = iterator.next();
+        for (EscherRecord r : this) {
             pos += r.serialize(pos, data, listener );
         }
 
@@ -118,9 +118,7 @@ public final class EscherContainerRecord extends EscherRecord {
     @Override
     public int getRecordSize() {
         int childRecordsSize = 0;
-        Iterator<EscherRecord> iterator = _childRecords.iterator();
-        while (iterator.hasNext()) {
-            EscherRecord r = iterator.next();
+        for (EscherRecord r : this) {
             childRecordsSize += r.getRecordSize();
         }
         return 8 + childRecordsSize;
@@ -134,9 +132,7 @@ public final class EscherContainerRecord extends EscherRecord {
      * @return true, if any child has the given recordId
      */
     public boolean hasChildOfType(short recordId) {
-        Iterator<EscherRecord> iterator = _childRecords.iterator();
-        while (iterator.hasNext()) {
-            EscherRecord r = iterator.next();
+        for (EscherRecord r : this) {
             if(r.getRecordId() == recordId) {
                 return true;
             }
@@ -158,8 +154,20 @@ public final class EscherContainerRecord extends EscherRecord {
 
     /**
      * @return an iterator over the child records
+     * @deprecated POI 3.16 beta 1. use iterator() or loop over the container record instead,
+     *     e.g. "for (EscherRecord r : container) ..."
      */
+    @Removal(version="3.18")
+    @Deprecated
     public Iterator<EscherRecord> getChildIterator() {
+        return iterator();
+    }
+    
+    /**
+     * @return an iterator over the child records
+     */
+    @Override
+    public Iterator<EscherRecord> iterator() {
         return Collections.unmodifiableList(_childRecords).iterator();
     }
 
@@ -195,9 +203,7 @@ public final class EscherContainerRecord extends EscherRecord {
      */
     public List<EscherContainerRecord> getChildContainers() {
         List<EscherContainerRecord> containers = new ArrayList<EscherContainerRecord>();
-        Iterator<EscherRecord> iterator = _childRecords.iterator();
-        while (iterator.hasNext()) {
-            EscherRecord r = iterator.next();
+        for (EscherRecord r : this) {
             if(r instanceof EscherContainerRecord) {
                 containers.add((EscherContainerRecord) r);
             }
@@ -228,9 +234,7 @@ public final class EscherContainerRecord extends EscherRecord {
     @Override
     public void display(PrintWriter w, int indent) {
         super.display(w, indent);
-        for (Iterator<EscherRecord> iterator = _childRecords.iterator(); iterator.hasNext();)
-        {
-            EscherRecord escherRecord = iterator.next();
+        for (EscherRecord escherRecord : this) {
             escherRecord.display(w, indent + 1);
         }
     }
@@ -252,8 +256,10 @@ public final class EscherContainerRecord extends EscherRecord {
      */
     public void addChildBefore(EscherRecord record, int insertBeforeRecordId) {
         int idx = 0;
-        for (EscherRecord rec : _childRecords) {
-            if(rec.getRecordId() == (short)insertBeforeRecordId) break;
+        for (EscherRecord rec : this) {
+            if(rec.getRecordId() == (short)insertBeforeRecordId) {
+                break;
+            }
             // TODO - keep looping? Do we expect multiple matches?
             idx++;
         }
@@ -271,10 +277,7 @@ public final class EscherContainerRecord extends EscherRecord {
             children.append( "  children: " + nl );
 
             int count = 0;
-            for ( Iterator<EscherRecord> iterator = _childRecords.iterator(); iterator
-                    .hasNext(); )
-            {
-                EscherRecord record = iterator.next();
+            for ( EscherRecord record : this ) {
                 children.append( "   Child " + count + ":" + nl );
                 String childResult = String.valueOf( record );
                 childResult = childResult.replaceAll( "\n", "\n    " );
@@ -298,22 +301,16 @@ public final class EscherContainerRecord extends EscherRecord {
     public String toXml(String tab) {
         StringBuilder builder = new StringBuilder();
         builder.append(tab).append(formatXmlRecordHeader(getRecordName(), HexDump.toHex(getRecordId()), HexDump.toHex(getVersion()), HexDump.toHex(getInstance())));
-        for ( Iterator<EscherRecord> iterator = _childRecords.iterator(); iterator
-                .hasNext(); )
-        {
-            EscherRecord record = iterator.next();
+        for ( EscherRecord record : this ) {
             builder.append(record.toXml(tab+"\t"));
         }
         builder.append(tab).append("</").append(getRecordName()).append(">\n");
         return builder.toString();
     }
 
-    public <T extends EscherRecord> T getChildById( short recordId )
-    {
-        for ( EscherRecord childRecord : _childRecords )
-        {
-            if ( childRecord.getRecordId() == recordId )
-            {
+    public <T extends EscherRecord> T getChildById( short recordId ) {
+        for ( EscherRecord childRecord : this ) {
+            if ( childRecord.getRecordId() == recordId ) {
                 @SuppressWarnings( "unchecked" )
                 final T result = (T) childRecord;
                 return result;
@@ -329,9 +326,7 @@ public final class EscherContainerRecord extends EscherRecord {
      * @param out - list to store found records
      */
     public void getRecordsById(short recordId, List<EscherRecord> out){
-        Iterator<EscherRecord> iterator = _childRecords.iterator();
-        while (iterator.hasNext()) {
-            EscherRecord r = iterator.next();
+        for (EscherRecord r : this) {
             if(r instanceof EscherContainerRecord) {
                 EscherContainerRecord c = (EscherContainerRecord)r;
                 c.getRecordsById(recordId, out );
diff --git a/src/java/org/apache/poi/hssf/dev/BiffViewer.java b/src/java/org/apache/poi/hssf/dev/BiffViewer.java
index 5465f0ff10..04e965d441 100644
--- a/src/java/org/apache/poi/hssf/dev/BiffViewer.java
+++ b/src/java/org/apache/poi/hssf/dev/BiffViewer.java
@@ -194,6 +194,7 @@ import org.apache.poi.hssf.record.pivottable.ViewSourceRecord;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
 import org.apache.poi.util.HexDump;
+import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.LittleEndian;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
@@ -251,12 +252,10 @@ public final class BiffViewer {
                 }
                 temp.add(record);
 
-                if (dumpInterpretedRecords) {
-                    for (String header : recListener.getRecentHeaders()) {
-                        ps.println(header);
-                    }
-                    ps.print(record.toString());
+                for (String header : recListener.getRecentHeaders()) {
+                    ps.println(header);
                 }
+                ps.print(record.toString());
             } else {
                 recStream.readRemainder();
             }
@@ -560,33 +559,26 @@ public final class BiffViewer {
 			pw = new PrintWriter(new OutputStreamWriter(System.out, Charset.defaultCharset()));
 		}
 
+		NPOIFSFileSystem fs = null;
+		InputStream is = null;
         try {
-            NPOIFSFileSystem fs = new NPOIFSFileSystem(cmdArgs.getFile(), true);
-            try {
-                InputStream is = getPOIFSInputStream(fs);
+            fs = new NPOIFSFileSystem(cmdArgs.getFile(), true);
+            is = getPOIFSInputStream(fs);
 
-                try {
-                    if (cmdArgs.shouldOutputRawHexOnly()) {
-                        int size = is.available();
-                        byte[] data = new byte[size];
-
-                        is.read(data);
-                        HexDump.dump(data, 0, System.out, 0);
-                    } else {
-                        boolean dumpInterpretedRecords = cmdArgs.shouldDumpRecordInterpretations();
-                        boolean dumpHex = cmdArgs.shouldDumpBiffHex();
-                        boolean zeroAlignHexDump = dumpInterpretedRecords;  // TODO - fix non-zeroAlign
-                        runBiffViewer(pw, is, dumpInterpretedRecords, dumpHex, zeroAlignHexDump,
-                                cmdArgs.suppressHeader());
-                    }
-                } finally {
-                    is.close();
-                }
-            } finally {
-                fs.close();
+            if (cmdArgs.shouldOutputRawHexOnly()) {
+                byte[] data = IOUtils.toByteArray(is);
+                HexDump.dump(data, 0, System.out, 0);
+            } else {
+                boolean dumpInterpretedRecords = cmdArgs.shouldDumpRecordInterpretations();
+                boolean dumpHex = cmdArgs.shouldDumpBiffHex();
+                boolean zeroAlignHexDump = dumpInterpretedRecords;  // TODO - fix non-zeroAlign
+                runBiffViewer(pw, is, dumpInterpretedRecords, dumpHex, zeroAlignHexDump,
+                        cmdArgs.suppressHeader());
             }
         } finally {
-            pw.close();
+            IOUtils.closeQuietly(is);
+            IOUtils.closeQuietly(fs);
+            IOUtils.closeQuietly(pw);
         }
 	}
 
diff --git a/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java b/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java
index 8cc1d96bd8..3d6bc3515c 100644
--- a/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java
+++ b/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java
@@ -82,28 +82,18 @@ public class OldExcelExtractor implements Closeable {
             return;
         } catch (OldExcelFormatException e) {
             // will be handled by workaround below
-            if (poifs != null) {
-                poifs.close();
-            }
         } catch (NotOLE2FileException e) {
             // will be handled by workaround below
-            if (poifs != null) {
-                poifs.close();
-            }
         } catch (IOException e) {
             // ensure streams are closed correctly
-            if (poifs != null) {
-                poifs.close();
-            }
-
             throw e;
         } catch (RuntimeException e) {
             // ensure streams are closed correctly
-            if (poifs != null) {
-                poifs.close();
-            }
-
             throw e;
+        } finally {
+            if (toClose == null) {
+                IOUtils.closeQuietly(poifs);
+            }
         }
 
         @SuppressWarnings("resource")
diff --git a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
index 3a5fc1ef55..843f142289 100644
--- a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
+++ b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
@@ -110,6 +110,7 @@ import org.apache.poi.util.Internal;
 import org.apache.poi.util.LocaleUtil;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
+import org.apache.poi.util.RecordFormatException;
 
 /**
  * Low level model implementation of a Workbook.  Provides creational methods
@@ -2292,8 +2293,7 @@ public final class InternalWorkbook {
 
                 EscherDggRecord dgg = null;
                 EscherContainerRecord bStore = null;
-                for(Iterator<EscherRecord> it = cr.getChildIterator(); it.hasNext();) {
-                    EscherRecord er = it.next();
+                for(EscherRecord er : cr) {
                     if(er instanceof EscherDggRecord) {
                         dgg = (EscherDggRecord)er;
                     } else if (er.getRecordId() == EscherContainerRecord.BSTORE_CONTAINER) {
@@ -2590,8 +2590,7 @@ public final class InternalWorkbook {
             dgg.setDrawingsSaved(dgg.getDrawingsSaved() + 1);
 
             EscherDgRecord dg = null;
-            for(Iterator<EscherRecord> it = escherContainer.getChildIterator(); it.hasNext();) {
-                EscherRecord er = it.next();
+            for(EscherRecord er : escherContainer) {
                 if(er instanceof EscherDgRecord) {
                     dg = (EscherDgRecord)er;
                     //update id of the drawing in the cloned sheet
@@ -2604,6 +2603,9 @@ public final class InternalWorkbook {
                         for(EscherRecord shapeChildRecord : shapeContainer.getChildRecords()) {
                             int recordId = shapeChildRecord.getRecordId();
                             if (recordId == EscherSpRecord.RECORD_ID){
+                                if (dg == null) {
+                                    throw new RecordFormatException("EscherDgRecord wasn't set/processed before.");
+                                }
                                 EscherSpRecord sp = (EscherSpRecord)shapeChildRecord;
                                 int shapeId = drawingManager.allocateShapeId((short)dgId, dg);
                                 //allocateShapeId increments the number of shapes. roll back to the previous value
diff --git a/src/java/org/apache/poi/hssf/record/common/UnicodeString.java b/src/java/org/apache/poi/hssf/record/common/UnicodeString.java
index 4cefdee102..9e758782d2 100644
--- a/src/java/org/apache/poi/hssf/record/common/UnicodeString.java
+++ b/src/java/org/apache/poi/hssf/record/common/UnicodeString.java
@@ -748,7 +748,7 @@ public class UnicodeString implements Comparable<UnicodeString> {
           }
         }
 
-        if (extendedDataSize > 0) {
+        if (extendedDataSize > 0 && field_5_ext_rst != null) {
            field_5_ext_rst.serialize(out);
         }
     }
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java b/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java
index 772ee5821d..5135078c00 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java
@@ -17,14 +17,26 @@
 

 package org.apache.poi.hssf.usermodel;

 

-import org.apache.poi.ddf.*;

-import org.apache.poi.hssf.record.*;

-import org.apache.poi.poifs.filesystem.DirectoryNode;

-

-import java.util.Iterator;

 import java.util.List;

 import java.util.Map;

 

+import org.apache.poi.ddf.EscherClientDataRecord;

+import org.apache.poi.ddf.EscherContainerRecord;

+import org.apache.poi.ddf.EscherOptRecord;

+import org.apache.poi.ddf.EscherProperties;

+import org.apache.poi.ddf.EscherProperty;

+import org.apache.poi.ddf.EscherRecord;

+import org.apache.poi.ddf.EscherTextboxRecord;

+import org.apache.poi.hssf.record.CommonObjectDataSubRecord;

+import org.apache.poi.hssf.record.EmbeddedObjectRefSubRecord;

+import org.apache.poi.hssf.record.EscherAggregate;

+import org.apache.poi.hssf.record.ObjRecord;

+import org.apache.poi.hssf.record.Record;

+import org.apache.poi.hssf.record.SubRecord;

+import org.apache.poi.hssf.record.TextObjectRecord;

+import org.apache.poi.poifs.filesystem.DirectoryNode;

+import org.apache.poi.util.RecordFormatException;

+

 /**

  * Factory class for producing Excel Shapes from Escher records

  */

@@ -58,7 +70,7 @@ public class HSSFShapeFactory {
             ObjRecord objRecord = null;

             TextObjectRecord txtRecord = null;

 

-            for (EscherRecord record : container.getChildRecords()) {

+            for (EscherRecord record : container) {

                 switch (record.getRecordId()) {

                     case EscherClientDataRecord.RECORD_ID:

                         objRecord = (ObjRecord) shapeToObj.get(record);

@@ -70,6 +82,9 @@ public class HSSFShapeFactory {
                         break;

                 }

             }

+            if (objRecord == null) {

+                throw new RecordFormatException("EscherClientDataRecord can't be found.");

+            }

             if (isEmbeddedObject(objRecord)) {

                 HSSFObjectData objectData = new HSSFObjectData(container, objRecord, root);

                 out.addShape(objectData);

@@ -117,9 +132,7 @@ public class HSSFShapeFactory {
     }

 

     private static boolean isEmbeddedObject(ObjRecord obj) {

-        Iterator<SubRecord> subRecordIter = obj.getSubRecords().iterator();

-        while (subRecordIter.hasNext()) {

-            SubRecord sub = subRecordIter.next();

+        for (SubRecord sub : obj.getSubRecords()) {

             if (sub instanceof EmbeddedObjectRefSubRecord) {

                 return true;

             }

diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
index 471856a9e5..4e5f280951 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
@@ -1688,7 +1688,7 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
                 // Need to walk backward to find the last non-blank row
                 // NOTE: n is always negative here
                 _lastrow = Math.min(endRow + n, SpreadsheetVersion.EXCEL97.getLastRowIndex());
-                for (int i = endRow - 1; i > endRow + n; i++) {
+                for (int i = endRow - 1; i > endRow + n; i--) {
                     if (getRow(i) != null) {
                         _lastrow = i;
                         break;
diff --git a/src/scratchpad/src/org/apache/poi/hslf/record/PPDrawingGroup.java b/src/scratchpad/src/org/apache/poi/hslf/record/PPDrawingGroup.java
index 0cdb1f6273..d655e5a4fe 100644
--- a/src/scratchpad/src/org/apache/poi/hslf/record/PPDrawingGroup.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/record/PPDrawingGroup.java
@@ -71,17 +71,14 @@ public final class PPDrawingGroup extends RecordAtom {
 
     public void writeOut(OutputStream out) throws IOException {
         ByteArrayOutputStream bout = new ByteArrayOutputStream();
-        Iterator<EscherRecord> iter = dggContainer.getChildIterator();
-        while (iter.hasNext()) {
-        	EscherRecord r = iter.next();
+        for (EscherRecord r : dggContainer) {
             if (r.getRecordId() == EscherContainerRecord.BSTORE_CONTAINER){
                 EscherContainerRecord bstore = (EscherContainerRecord)r;
 
                 ByteArrayOutputStream b2 = new ByteArrayOutputStream();
-                for (Iterator<EscherRecord> it= bstore.getChildIterator(); it.hasNext();) {
-                    EscherBSERecord bse = (EscherBSERecord)it.next();
+                for (EscherRecord br : bstore) {
                     byte[] b = new byte[36+8];
-                    bse.serialize(0, b);
+                    br.serialize(0, b);
                     b2.write(b);
                 }
                 byte[] bstorehead = new byte[8];
@@ -120,8 +117,7 @@ public final class PPDrawingGroup extends RecordAtom {
 
     public EscherDggRecord getEscherDggRecord(){
         if(dgg == null){
-            for(Iterator<EscherRecord> it = dggContainer.getChildIterator(); it.hasNext();){
-                EscherRecord r = it.next();
+            for(EscherRecord r : dggContainer){
                 if(r instanceof EscherDggRecord){
                     dgg = (EscherDggRecord)r;
                     break;
diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFGroupShape.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFGroupShape.java
index bd90edfec4..3d4d57fbdb 100644
--- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFGroupShape.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFGroupShape.java
@@ -272,17 +272,17 @@ implements HSLFShapeContainer, GroupShape<HSLFShape,HSLFTextParagraph> {
 
     @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 = getSpContainer().getChildIterator();
-
-        // Don't include the first SpContainer, it is always NotPrimitive
-        if (iter.hasNext()) {
-            iter.next();
-        }
+        // Our escher container record should contain several
+        // SpContainers, the first of which is the group shape itself
         List<HSLFShape> shapeList = new ArrayList<HSLFShape>();
-        while (iter.hasNext()) {
-            EscherRecord r = iter.next();
+        boolean isFirst = true;
+        for (EscherRecord r : getSpContainer()) {
+            if (isFirst) {
+                // Don't include the first SpContainer, it is always NotPrimitive
+                isFirst = false;
+                continue;
+            }
+            
             if(r instanceof EscherContainerRecord) {
                 // Create the Shape for it
                 EscherContainerRecord container = (EscherContainerRecord)r;
diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSheet.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSheet.java
index 72042769d1..5bfd3bbc9c 100644
--- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSheet.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSheet.java
@@ -151,8 +151,7 @@ public abstract class HSLFSheet implements HSLFShapeContainer, Sheet<HSLFShape,H
         EscherContainerRecord dg = ppdrawing.getDgContainer();
         EscherContainerRecord spgr = null;
 
-        for (Iterator<EscherRecord> it = dg.getChildIterator(); it.hasNext();) {
-            EscherRecord rec = it.next();
+        for (EscherRecord rec : dg) {
             if (rec.getRecordId() == EscherContainerRecord.SPGR_CONTAINER) {
                 spgr = (EscherContainerRecord) rec;
                 break;
@@ -163,13 +162,15 @@ public abstract class HSLFSheet implements HSLFShapeContainer, Sheet<HSLFShape,H
         }
 
         List<HSLFShape> shapeList = new ArrayList<HSLFShape>();
-        Iterator<EscherRecord> it = spgr.getChildIterator();
-        if (it.hasNext()) {
-            // skip first item
-            it.next();
-        }
-        for (; it.hasNext();) {
-            EscherContainerRecord sp = (EscherContainerRecord) it.next();
+        boolean isFirst = true;
+        for (EscherRecord r : spgr) {
+            if (isFirst) {
+                // skip first item
+                isFirst = false;
+                continue;
+            }
+
+            EscherContainerRecord sp = (EscherContainerRecord)r;
             HSLFShape sh = HSLFShapeFactory.createShape(sp, null);
             sh.setSheet(this);
             
diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java
index b20af62ec9..1251904b21 100644
--- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java
@@ -376,14 +376,8 @@ public class HSLFSlideShowEncrypted implements Closeable {
         } catch (Exception e) {
             throw new EncryptedPowerPointFileException(e);
         } finally {
-            try {
-                if (ccos != null) {
-                    ccos.close();
-                }
-                los.close();
-            } catch (IOException e) {
-                throw new EncryptedPowerPointFileException(e);
-            }
+            IOUtils.closeQuietly(ccos);
+            IOUtils.closeQuietly(los);
         }
     }
 
diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java
index 4cb3a310c3..60232d50a7 100644
--- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java
@@ -980,7 +980,7 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
             }

         }

 

-        if (lastPTPC == null || lastRTPC == null || ptpc == null || rtpc == null) {

+        if (lastPTPC == null || lastRTPC == null || ptpc == null || rtpc == null) { // NOSONAR

             throw new HSLFException("Not all TextPropCollection could be determined.");

         }

         

diff --git a/src/scratchpad/src/org/apache/poi/hssf/usermodel/HSSFChart.java b/src/scratchpad/src/org/apache/poi/hssf/usermodel/HSSFChart.java
index a919b047f2..1b67e52ff9 100644
--- a/src/scratchpad/src/org/apache/poi/hssf/usermodel/HSSFChart.java
+++ b/src/scratchpad/src/org/apache/poi/hssf/usermodel/HSSFChart.java
@@ -227,49 +227,47 @@ public final class HSSFChart {
 
 			if(r instanceof ChartRecord) {
 				lastSeries = null;
-				
 				lastChart = new HSSFChart(sheet,(ChartRecord)r);
 				charts.add(lastChart);
-			} else if(r instanceof LegendRecord) {
+            } else if (r instanceof LinkedDataRecord) {
+                LinkedDataRecord linkedDataRecord = (LinkedDataRecord) r;
+                if (lastSeries != null) {
+                    lastSeries.insertData(linkedDataRecord);
+                }
+			}
+            
+            if (lastChart == null) {
+                continue;
+            }
+            
+            if (r instanceof LegendRecord) {
 				lastChart.legendRecord = (LegendRecord)r;
 			} else if(r instanceof SeriesRecord) {
 				HSSFSeries series = new HSSFSeries( (SeriesRecord)r );
 				lastChart.series.add(series);
 				lastSeries = series;
 			} else if(r instanceof ChartTitleFormatRecord) {
-				lastChart.chartTitleFormat =
-					(ChartTitleFormatRecord)r;
+				lastChart.chartTitleFormat = (ChartTitleFormatRecord)r;
 			} else if(r instanceof SeriesTextRecord) {
-				// Applies to a series, unless we've seen
-				//  a legend already
+				// Applies to a series, unless we've seen a legend already
 				SeriesTextRecord str = (SeriesTextRecord)r;
-				if(lastChart.legendRecord == null &&
-						lastChart.series.size() > 0) {
+				if(lastChart.legendRecord == null && lastChart.series.size() > 0) {
 					HSSFSeries series = lastChart.series.get(lastChart.series.size()-1);
 					series.seriesTitleText = str;
 				} else {
 					lastChart.chartTitleText = str;
 				}
-			} else if (r instanceof LinkedDataRecord) {
-				LinkedDataRecord linkedDataRecord = (LinkedDataRecord) r;
-				if (lastSeries != null) {
-					lastSeries.insertData(linkedDataRecord);
-				}
 			} else if(r instanceof ValueRangeRecord){
 				lastChart.valueRanges.add((ValueRangeRecord)r);
 			} else if (r instanceof Record) {
-				if (lastChart != null)
-				{
-					Record record = (Record) r;
-					for (HSSFChartType type : HSSFChartType.values()) {
-						if (type == HSSFChartType.Unknown)
-						{
-							continue;
-						}
-						if (record.getSid() == type.getSid()) {
-							lastChart.type = type ;
-							break;
-						}
+				Record record = (Record) r;
+				for (HSSFChartType type : HSSFChartType.values()) {
+					if (type == HSSFChartType.Unknown) {
+						continue;
+					}
+					if (record.getSid() == type.getSid()) {
+						lastChart.type = type;
+						break;
 					}
 				}
 			}