svn commit: r1804328 - /ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityListIterator.java

Author: jleroux
Date: Mon Aug  7 12:43:54 2017
New Revision: 1804328

URL: http://svn.apache.org/viewvc?rev=1804328&view=rev
Log:
Improved: Refactor EntityListIterator
(OFBIZ-9549)

The EntityListIterator contains duplicate code and has need for refactoring in
general.

Thanks: Tobias Laufkötter

Modified:
    ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityListIterator.java

Modified: ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityListIterator.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityListIterator.java?rev=1804328&r1=1804327&r2=1804328&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityListIterator.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityListIterator.java Mon Aug  7 12:43:54 2017
@@ -19,10 +19,9 @@
 
 package org.apache.ofbiz.entity.util;
 
-
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.ListIterator;
 
@@ -40,7 +39,6 @@ import org.apache.ofbiz.entity.model.Mod
 import org.apache.ofbiz.entity.model.ModelField;
 import org.apache.ofbiz.entity.model.ModelFieldTypeReader;
 
-
 /**
  * Generic Entity Cursor List Iterator for Handling Cursored DB Results
  */
@@ -69,12 +67,10 @@ public class EntityListIterator implemen
         this(sqlp, modelEntity, selectFields, modelFieldTypeReader, null, null, null, false);
     }
 
-    public EntityListIterator(SQLProcessor sqlp, ModelEntity modelEntity, List<ModelField> selectFields, ModelFieldTypeReader modelFieldTypeReader, GenericDAO genericDAO, EntityCondition whereCondition, EntityCondition havingCondition, boolean distinctQuery) {
+    public EntityListIterator(SQLProcessor sqlp, ModelEntity modelEntity, List<ModelField> selectFields, ModelFieldTypeReader modelFieldTypeReader, GenericDAO genericDAO,
+            EntityCondition whereCondition, EntityCondition havingCondition, boolean distinctQuery) {
+        this(sqlp.getResultSet(), modelEntity, selectFields, modelFieldTypeReader);
         this.sqlp = sqlp;
-        this.resultSet = sqlp.getResultSet();
-        this.modelEntity = modelEntity;
-        this.selectFields = selectFields;
-        this.modelFieldTypeReader = modelFieldTypeReader;
         this.genericDAO = genericDAO;
         this.whereCondition = whereCondition;
         this.havingCondition = havingCondition;
@@ -93,82 +89,101 @@ public class EntityListIterator implemen
         this.delegator = delegator;
     }
 
-    /** Sets the cursor position to just after the last result so that previous() will return the last result */
+    /**
+     * Sets the cursor position to just after the last result so that previous() will return the last result.
+     *
+     * @throws GenericEntityException
+     *             if a database error occurs.
+     */
     public void afterLast() throws GenericEntityException {
         try {
             resultSet.afterLast();
         } catch (SQLException e) {
-            if (!closed) {
-                this.close();
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            closeWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GenericEntityException("Error setting the cursor to afterLast", e);
         }
     }
 
-    /** Sets the cursor position to just before the first result so that next() will return the first result */
+    /**
+     * Sets the cursor position to just before the first result so that next() will return the first result.
+     *
+     * @throws GenericEntityException
+     *             if a database error occurs.
+     */
     public void beforeFirst() throws GenericEntityException {
         try {
             resultSet.beforeFirst();
         } catch (SQLException e) {
-            if (!closed) {
-                this.close();
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            closeWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GenericEntityException("Error setting the cursor to beforeFirst", e);
         }
     }
 
-    /** Sets the cursor position to last result; if result set is empty returns false */
+    /**
+     * Sets the cursor position to last result.
+     *
+     * @return true if the result set is not empty.
+     * @throws GenericEntityException
+     *             if a database error occurs.
+     */
     public boolean last() throws GenericEntityException {
         try {
             return resultSet.last();
         } catch (SQLException e) {
-            if (!closed) {
-                this.close();
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            closeWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GenericEntityException("Error setting the cursor to last", e);
         }
     }
 
-    /** Sets the cursor position to first result; if result set is empty returns false */
+    /**
+     * Sets the cursor position to first result.
+     *
+     * @return true if the result set is not empty.
+     * @throws GenericEntityException
+     *             in case of a database error.
+     */
     public boolean first() throws GenericEntityException {
         try {
             return resultSet.first();
         } catch (SQLException e) {
-            if (!closed) {
-                this.close();
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            closeWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GenericEntityException("Error setting the cursor to first", e);
         }
     }
 
+    /**
+     * Closes the iterator.
+     *
+     * @throws GenericEntityException
+     *             if the {@link EntityListIterator} cannot be closed.
+     */
     public void close() throws GenericEntityException {
         if (closed) {
-            //maybe not the best way: throw new GenericResultSetClosedException("This EntityListIterator has been closed, this operation cannot be performed");
-            Debug.logWarning("This EntityListIterator for Entity [" +
-                    modelEntity == null ? "" : modelEntity.getEntityName() +
-                            "] has already been closed, not closing again.", module);
-        } else {
-            if (sqlp != null) {
-                sqlp.close();
-                closed = true;
-            } else if (resultSet != null) {
-                try {
-                    resultSet.close();
-                } catch (SQLException e) {
-                    throw new GenericEntityException("Cannot close EntityListIterator with ResultSet", e);
-                }
-                closed = true;
-            } else {
-                throw new GenericEntityException("Cannot close an EntityListIterator without a SQLProcessor or a ResultSet");
-            }
+            String modelEntityName = modelEntity != null ? modelEntity.getEntityName() : "";
+            Debug.logWarning("This EntityListIterator for Entity [" + modelEntityName + "] has already been closed, not closing again.", module);
+            return;
+        }
+        if (sqlp != null) {
+            sqlp.close();
+            closed = true;
+            return;
+        }
+        if (resultSet == null) {
+            throw new GenericEntityException(
+                    "Cannot close an EntityListIterator without a SQLProcessor or a ResultSet");
+        }
+        try {
+            resultSet.close();
+        } catch (SQLException e) {
+            throw new GenericEntityException("Cannot close EntityListIterator with ResultSet", e);
         }
+        closed = true;
     }
 
-    /** NOTE: Calling this method does return the current value, but so does calling next() or previous(), so calling one of those AND this method will cause the value to be created twice */
+    /**
+     * NOTE: Calling this method does return the current value, but so does calling next() or previous()
+     *  So calling one of those AND this method will cause the value to be created twice.
+     */
     public GenericValue currentGenericValue() throws GenericEntityException {
         if (closed) throw new GenericResultSetClosedException("This EntityListIterator has been closed, this operation cannot be performed");
 
@@ -186,24 +201,37 @@ public class EntityListIterator implemen
         return value;
     }
 
+    /**
+     * Determines the current index of the cursor position.
+     *
+     * @return the current index, 0 if there is none.
+     * @throws GenericEntityException
+     *             if an error with the database access occurs.
+     */
     public int currentIndex() throws GenericEntityException {
         if (closed) throw new GenericResultSetClosedException("This EntityListIterator has been closed, this operation cannot be performed");
 
         try {
             return resultSet.getRow();
         } catch (SQLException e) {
-            if (!closed) {
-                this.close();
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            closeWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GenericEntityException("Error getting the current index", e);
         }
     }
 
-    /** performs the same function as the ResultSet.absolute method;
+    /**
+     * performs the same function as the {@link ResultSet#absolute(int)} method.
      * if rowNum is positive, goes to that position relative to the beginning of the list;
      * if rowNum is negative, goes to that position relative to the end of the list;
-     * a rowNum of 1 is the same as first(); a rowNum of -1 is the same as last()
+     * a rowNum of 1 is the same as first();
+     * a rowNum of -1 is the same as last()
+     *
+     * @param rowNum
+     *            the index of the row to set the cursor to.
+     * @return true if the cursor moved to a row within the result set, false if it
+     *         is on the row before or after.
+     * @throws GenericEntityException
+     *             if an error with the database access occurs.
      */
     public boolean absolute(int rowNum) throws GenericEntityException {
         if (closed) throw new GenericResultSetClosedException("This EntityListIterator has been closed, this operation cannot be performed");
@@ -212,21 +240,24 @@ public class EntityListIterator implemen
             if (rowNum == 0) {
                 resultSet.beforeFirst();
                 return true;
-            } else {
-                return resultSet.absolute(rowNum);
             }
+            return resultSet.absolute(rowNum);
         } catch (SQLException e) {
-            if (!closed) {
-                this.close();
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            closeWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GenericEntityException("Error setting the absolute index to " + rowNum, e);
         }
     }
 
-    /** performs the same function as the ResultSet.relative method;
+    /**
+     * performs the same function as the {@link ResultSet#relative(int)} method.
      * if rows is positive, goes forward relative to the current position;
      * if rows is negative, goes backward relative to the current position;
+     *
+     * @param rows
+     *            the amount of rows to move.
+     * @return true if the cursor is on a row.
+     * @throws GenericEntityException
+     *             in case of an error related to the database.
      */
     public boolean relative(int rows) throws GenericEntityException {
         if (closed) throw new GenericResultSetClosedException("This EntityListIterator has been closed, this operation cannot be performed");
@@ -234,27 +265,28 @@ public class EntityListIterator implemen
         try {
             return resultSet.relative(rows);
         } catch (SQLException e) {
-            if (!closed) {
-                this.close();
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            closeWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GenericEntityException("Error going to the relative index " + rows, e);
         }
     }
 
     /**
-     * PLEASE NOTE: Because of the nature of the JDBC ResultSet interface this method can be very inefficient; it is much better to just use next() until it returns null
+     * PLEASE NOTE: Because of the nature of the JDBC ResultSet interface this method can be very inefficient.
+     * It is much better to just use next() until it returns null
      * For example, you could use the following to iterate through the results in an EntityListIterator:
      *
-     *      GenericValue nextValue = null;
-     *      while ((nextValue = (GenericValue) this.next()) != null) { ... }
+     * GenericValue nextValue = null;
+     * while ((nextValue = (GenericValue)
+     * this.next()) != null) { ... }
      *
      */
     public boolean hasNext() {
         if (!haveShowHasNextWarning) {
             // DEJ20050207 To further discourage use of this, and to find existing use, always log a big warning showing where it is used:
             Exception whereAreWe = new Exception();
-            Debug.logWarning(whereAreWe, "For performance reasons do not use the EntityListIterator.hasNext() method, just call next() until it returns null; see JavaDoc comments in the EntityListIterator class for details and an example", module);
+            Debug.logWarning(whereAreWe,
+                    "For performance reasons do not use the EntityListIterator.hasNext() method, just call next() until it returns null; see JavaDoc comments in the EntityListIterator class for details and an example",
+                    module);
 
             haveShowHasNextWarning = true;
         }
@@ -262,169 +294,119 @@ public class EntityListIterator implemen
         try {
             if (resultSet.isLast() || resultSet.isAfterLast()) {
                 return false;
-            } else {
-                // do a quick game to see if the resultSet is empty:
-                // if we are not in the first or beforeFirst positions and we haven't made any values yet, the result set is empty so return false
-                if (!haveMadeValue && !resultSet.isBeforeFirst() && !resultSet.isFirst()) {
-                    return false;
-                } else {
-                    return true;
-                }
             }
+            // do a quick game to see if the resultSet is empty:
+            // if we are not in the first or beforeFirst positions and we haven't made any values yet, the result set is empty so return false
+            return haveMadeValue || resultSet.isBeforeFirst() || resultSet.isFirst();
         } catch (SQLException e) {
-            if (!closed) {
-                try {
-                    this.close();
-                } catch (GenericEntityException e1) {
-                    Debug.logError(e1, "Error auto-closing EntityListIterator on error, so info below for more info on original error; close error: " + e1.toString(), module);
-                }
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            tryCloseWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GeneralRuntimeException("Error while checking to see if this is the last result", e);
         }
     }
 
-    /** PLEASE NOTE: Because of the nature of the JDBC ResultSet interface this method can be very inefficient; it is much better to just use previous() until it returns null */
+    /**
+     * PLEASE NOTE: Because of the nature of the JDBC ResultSet interface this method can be very inefficient.
+     * It is much better to just use previous() until it returns null.
+     */
     public boolean hasPrevious() {
         try {
             if (resultSet.isFirst() || resultSet.isBeforeFirst()) {
                 return false;
-            } else {
-                // do a quick game to see if the resultSet is empty:
-                // if we are not in the last or afterLast positions and we haven't made any values yet, the result set is empty so return false
-                if (!haveMadeValue && !resultSet.isAfterLast() && !resultSet.isLast()) {
-                    return false;
-                } else {
-                    return true;
-                }
             }
+            // do a quick game to see if the resultSet is empty:
+            // if we are not in the first or beforeFirst positions and we haven't made any values yet, the result set is
+            // empty so return false
+            return haveMadeValue || resultSet.isAfterLast() || resultSet.isLast();
         } catch (SQLException e) {
-            if (!closed) {
-                try {
-                    this.close();
-                } catch (GenericEntityException e1) {
-                    Debug.logError(e1, "Error auto-closing EntityListIterator on error, so info below for more info on original error; close error: " + e1.toString(), module);
-                }
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            tryCloseWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GeneralRuntimeException("Error while checking to see if this is the first result", e);
         }
     }
 
-    /** Moves the cursor to the next position and returns the GenericValue object for that position; if there is no next, returns null
-     * For example, you could use the following to iterate through the results in an EntityListIterator:
+    /**
+     * Moves the cursor to the next position and returns the value for that position; For example, you could use the following to iterate through the results in an
+     * EntityListIterator:
      *
-     *      GenericValue nextValue = null;
-     *      while ((nextValue = (GenericValue) this.next()) != null) { ... }
+     * GenericValue nextValue = null; while ((nextValue = (GenericValue) this.next()) != null) { ... }
      *
+     * @return the next element or null, if there is no next element.
      */
     public GenericValue next() {
         try {
-            if (resultSet.next()) {
-                return currentGenericValue();
-            } else {
-                return null;
-            }
+            return resultSet.next() ? currentGenericValue() : null;
         } catch (SQLException e) {
-            if (!closed) {
-                try {
-                    this.close();
-                } catch (GenericEntityException e1) {
-                    Debug.logError(e1, "Error auto-closing EntityListIterator on error, so info below for more info on original error; close error: " + e1.toString(), module);
-                }
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            tryCloseWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GeneralRuntimeException("Error getting the next result", e);
         } catch (GenericEntityException e) {
-            if (!closed) {
-                try {
-                    this.close();
-                } catch (GenericEntityException e1) {
-                    Debug.logError(e1, "Error auto-closing EntityListIterator on error, so info below for more info on original error; close error: " + e1.toString(), module);
-                }
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            tryCloseWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GeneralRuntimeException("Error creating GenericValue", e);
         }
     }
 
-    /** Returns the index of the next result, but does not guarantee that there will be a next result */
+    /**
+     * Returns the index of the next result, but does not guarantee that there will be a next result.
+     */
     public int nextIndex() {
         try {
             return currentIndex() + 1;
         } catch (GenericEntityException e) {
-            if (!closed) {
-                try {
-                    this.close();
-                } catch (GenericEntityException e1) {
-                    Debug.logError(e1, "Error auto-closing EntityListIterator on error, so info below for more info on original error; close error: " + e1.toString(), module);
-                }
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            tryCloseWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GeneralRuntimeException(e.getNonNestedMessage(), e.getNested());
         }
     }
 
-    /** Moves the cursor to the previous position and returns the GenericValue object for that position; if there is no previous, returns null */
+    /**
+     * Moves the cursor to the previous position and returns the GenericValue object for that position;
+     * if there is no previous, returns null.
+     */
     public GenericValue previous() {
         try {
-            if (resultSet.previous()) {
-                return currentGenericValue();
-            } else {
-                return null;
-            }
+            return resultSet.previous() ? currentGenericValue() : null;
         } catch (SQLException e) {
-            if (!closed) {
-                try {
-                    this.close();
-                } catch (GenericEntityException e1) {
-                    Debug.logError(e1, "Error auto-closing EntityListIterator on error, so info below for more info on original error; close error: " + e1.toString(), module);
-                }
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            tryCloseWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GeneralRuntimeException("Error getting the previous result", e);
         } catch (GenericEntityException e) {
-            if (!closed) {
-                try {
-                    this.close();
-                } catch (GenericEntityException e1) {
-                    Debug.logError(e1, "Error auto-closing EntityListIterator on error, so info below for more info on original error; close error: " + e1.toString(), module);
-                }
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            tryCloseWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GeneralRuntimeException("Error creating GenericValue", e);
         }
     }
 
-    /** Returns the index of the previous result, but does not guarantee that there will be a previous result */
+    /**
+     * Returns the index of the previous result, but does not guarantee that there will be a previous result.
+     */
     public int previousIndex() {
         try {
             return currentIndex() - 1;
         } catch (GenericEntityException e) {
-            if (!closed) {
-                try {
-                    this.close();
-                } catch (GenericEntityException e1) {
-                    Debug.logError(e1, "Error auto-closing EntityListIterator on error, so info below for more info on original error; close error: " + e1.toString(), module);
-                }
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            tryCloseWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GeneralRuntimeException("Error getting the current index", e);
         }
     }
 
+    /**
+     * Sets the fetch size of the result set to the given value.
+     *
+     * @param rows
+     *            the fetch size
+     * @throws GenericEntityException
+     *             if a database error occurs.
+     */
     public void setFetchSize(int rows) throws GenericEntityException {
         try {
             resultSet.setFetchSize(rows);
         } catch (SQLException e) {
-            if (!closed) {
-                this.close();
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            closeWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GenericEntityException("Error getting the next result", e);
         }
     }
 
+    /**
+     * Gets all the elements of the {@link EntityListIterator} as a list.
+     *
+     * @return a list of the elements.
+     * @throws GenericEntityException
+     *             if there is a problem with the database.
+     */
     public List<GenericValue> getCompleteList() throws GenericEntityException {
         try {
             // if the resultSet has been moved forward at all, move back to the beginning
@@ -432,35 +414,39 @@ public class EntityListIterator implemen
                 // do a quick check to see if the ResultSet is empty
                 resultSet.beforeFirst();
             }
-            List<GenericValue> list = new LinkedList<GenericValue>();
-            GenericValue nextValue = null;
+            List<GenericValue> list = new ArrayList<>((int) this.getResultSize());
+            GenericValue nextValue;
 
             while ((nextValue = this.next()) != null) {
                 list.add(nextValue);
             }
             return list;
         } catch (SQLException e) {
-            if (!closed) {
-                this.close();
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            closeWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GeneralRuntimeException("Error getting results", e);
         } catch (GeneralRuntimeException e) {
-            if (!closed) {
-                this.close();
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            closeWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GenericEntityException(e.getNonNestedMessage(), e.getNested());
         }
     }
 
-    /** Gets a partial list of results starting at start and containing at most number elements.
-     * Start is a one based value, ie 1 is the first element.
+    /**
+     * Gets a partial list of results starting at start and containing at most number elements.
+     * Start is a one based value, i.e. 1 is the first element.
+     *
+     * @param start
+     *            the index from which on the elements should be retrieved. Is one based.
+     * @param number
+     *            the maximum number of elements to get after the start index.
+     * @return A list with the retrieved elements, with the size of number or less if the result set does not contain enough values.
+     *            Empty list in case of no values or an invalid start index.
+     * @throws GenericEntityException
+     *            if there is an issue with the database.
      */
     public List<GenericValue> getPartialList(int start, int number) throws GenericEntityException {
         try {
-            if (number == 0) return new LinkedList<GenericValue>();
-            List<GenericValue> list = new LinkedList<GenericValue>();
+            if (number == 0)
+                return new ArrayList<>(0);
 
             // just in case the caller missed the 1 based thingy
             if (start == 0) start = 1;
@@ -468,69 +454,136 @@ public class EntityListIterator implemen
             // if can't reposition to desired index, throw exception
             if (!this.absolute(start - 1)) {
                 // maybe better to just return an empty list here...
-                return list;
-                //throw new GenericEntityException("Could not move to the start position of " + start + ", there are probably not that many results for this find.");
+                return new ArrayList<>(0);
             }
 
+            List<GenericValue> list = new ArrayList<>(this.getResultsSizeAfterPartialList());
             GenericValue nextValue = null;
-
-            //number > 0 comparison goes first to avoid the unwanted call to next
+            // number > 0 comparison goes first to avoid the unwanted call to next
             while (number > 0 && (nextValue = this.next()) != null) {
                 list.add(nextValue);
                 number--;
             }
             return list;
         } catch (GeneralRuntimeException e) {
-            if (!closed) {
-                this.close();
-                Debug.logWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString(), module);
-            }
+            closeWithWarning("Warning: auto-closed EntityListIterator because of exception: " + e.toString());
             throw new GenericEntityException(e.getNonNestedMessage(), e.getNested());
         }
     }
 
+    /**
+     * Determines the possible result size.
+     *
+     * If a {@link GenericDAO} is known, the result size will be counted by the
+     * database. Otherwise the result size is the last index of the
+     * {@link EntityListIterator}.
+     *
+     * @return the result size or 0 if the result set is empty.
+     * @throws GenericEntityException
+     *             if there is an issue with the call to the database.
+     */
     public int getResultsSizeAfterPartialList() throws GenericEntityException {
         if (genericDAO != null) {
             if (resultSize == null) {
-                EntityFindOptions efo = null;
-                if (distinctQuery) {
-                    efo = new EntityFindOptions();
-                    efo.setDistinct(distinctQuery);
-                }
-                resultSize = (int) genericDAO.selectCountByCondition(sqlp.getDelegator(), modelEntity, whereCondition, havingCondition, selectFields, efo);
+                resultSize = (int) getResultSize();
             }
-            return resultSize;
-        } else if (this.last()) {
-            return this.currentIndex();
-        } else {
-            // evidently no valid rows in the ResultSet, so return 0
-            return 0;
+            return resultSize.intValue();
         }
+        return this.last() ? this.currentIndex() : 0;
     }
 
+    /**
+     * Finds the size of the result.
+     *
+     * @return count of elements returned by a query.
+     * @throws GenericEntityException
+     *             if there is an issue with the call to the database.
+     */
+    private long getResultSize() throws GenericEntityException {
+        EntityFindOptions efo = null;
+        if (distinctQuery) {
+            efo = new EntityFindOptions();
+            efo.setDistinct(distinctQuery);
+        }
+        return genericDAO.selectCountByCondition(sqlp.getDelegator(), modelEntity, whereCondition,
+                havingCondition, selectFields, efo);
+    }
+
+    /**
+     * Unsupported {@link ListIterator#add(Object)} method.
+     */
     public void add(GenericValue obj) {
         throw new GeneralRuntimeException("CursorListIterator currently only supports read-only access");
     }
 
+    /**
+     * Unsupported {@link ListIterator#remove()} method.
+     */
     public void remove() {
         throw new GeneralRuntimeException("CursorListIterator currently only supports read-only access");
     }
 
+    /**
+     * Unsupported {@link ListIterator#set(Object)} method.
+     */
     public void set(GenericValue obj) {
         throw new GeneralRuntimeException("CursorListIterator currently only supports read-only access");
     }
 
+    /**
+     * Extends {@link Object#finalize()} to make sure that the {@link EntityListIterator} is closed when it is garbage collected.
+     *
+     * {@inheritDoc}
+     */
     @Override
     protected void finalize() throws Throwable {
         try {
             if (!closed) {
                 this.close();
-                Debug.logError("\n==============================================================================\n EntityListIterator Not Closed for Entity [" + (modelEntity==null ? "" : modelEntity.getEntityName())
-                        + "], caught in Finalize\n Please report by creating an OFBIZ-9297 subtask, thanks\n==============================================================================\n", module);
+                Debug.logError("\n==============================================================================\n"
+                        + "EntityListIterator Not Closed for Entity [%s], caught in Finalize\n"
+                        + "\n==============================================================================\n",
+                        module, modelEntity == null ? "" : modelEntity.getEntityName());
             }
         } catch (Exception e) {
             Debug.logError(e, "Error closing the SQLProcessor in finalize EntityListIterator", module);
         }
         super.finalize();
     }
+
+    /**
+     * Closes the {@link EntityListIterator} and logs a warning if it isn't already closed.
+     *
+     * If you don't want to handle the {@link GenericEntityException} thrown by {@link #close()}, use {@link #tryCloseWithWarning()}.
+     *
+     * @param warningMessage
+     *            the warning to be logged.
+     * @throws GenericEntityException
+     *             when the closing of the {@link EntityListIterator} fails. Thrown by {@link #close()}.
+     */
+    private void closeWithWarning(String warningMessage) throws GenericEntityException {
+        if (!closed) {
+            this.close();
+            Debug.logWarning(warningMessage, module);
+        }
+    }
+
+    /**
+     * Tries to close the {@link EntityListIterator} and logs a warning if it isn't already closed.
+     *
+     * Catches the {@link GenericEntityException} thrown by {@link #close()} and logs it. If you want to handle the exception yourself, use {@link #closeWithWarning()}.
+     *
+     * @param warningMessage
+     *            the warning to be logged.
+     */
+    private void tryCloseWithWarning(String warningMessage) {
+        if (!closed) {
+            try {
+                this.close();
+            } catch (GenericEntityException e) {
+                Debug.logError(e, "Error auto-closing EntityListIterator on error, so info below for more info on original error; close error: %s", module, e.toString());
+            }
+            Debug.logWarning(warningMessage, module);
+        }
+    }
 }