Sheet[1] and Row[2] interfaces expose an iterator interface that has a remove method. This allows sheets to be removed from the underlying data structure (TreeMap.values() -> Collection view of TreeMap's values), but doesn't do the rest of the cleanup that is done in removeRow(Row)[3] or removeCell(Cell)[4]. If I understand the behavior of a Collection *view* correctly, this would remove the row/cell from the TreeMap without cleaning up the rest of the data structures. The rowIterator and cellIterator should explicitly disallow the remove method unless the method correctly handles the rest of the cleanup without causing a ConcurrentModificationException. Perhaps this would be a good usage for Collections.unmodifiableMap or Collections.unmodifiableList[5]. Otherwise, a custom iterator could be written like was done for XSSFWorkbook.iterator[6] [1] XSSFSheet.iterator https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java?revision=1702802&view=markup#l1780 [2] XSSFRow.iterator https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java?revision=1649107&view=markup#l95 [3] XSSFSheet.removeRow https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java?revision=1702802&view=markup#l1689 [4] XSSFRow.removeCell https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java?revision=1649107&view=markup#l406 [5] Collections.unmodifiableMap https://docs.oracle.com/javase/7/docs/api/java/util/Collections.html [6] XSSFWorkbook.SheetIterator https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java?revision=1703573&view=markup#l1058
Created attachment 33121 [details] failing unit tests I've written unit tests for Workbook, Sheet, and Row which should test the sheetIterator, rowIterator, and cellIterator, respectively for the following scenarios: 1) iterator.remove() either is unsupported or behaves the same as the removeSheetAt, removeRow, or removeCell method 2) if removeSheetAt, removeRow, or removeCell are called after an iterator has been created, the iterator should throw a ConcurrentModificationException the next time a method is called on it. The Workbook sheetIterator passes, but the rowIterator and cellIterator fail for HSSF, XSSF, and SXSSF variants. Summary of test results HSSF XSSF SXSSF BaseTestWorkbook pass pass pass BaseTestSheet 1fail 1fail 1fail BaseTestRow 2fails 1fail 1fail I needed to modify HSSFRow.hashCode in order to have assertNull(HSSFRow) give a more meaningful error.
Created attachment 33122 [details] unit test results
Additional unit tests are necessary to verify that the side-effects of iterator.remove are identical to removeSheetAt, removeRow, and removeCell. I propose making iterator.remove() an UnsupportedOperationException for now until someone can find a solution that doesn't duplicate code and works correctly.