Cleaned up LineIterator changes include:

* Removed the IOIterator interface, it can be added back later when there is more than one
Iterator implementations with a close method.
* Doesn't automatically close the Reader at EOF.
* made LineIterator final because the hasNext method isn't implemented in a subclassable way.
* constructor throws an IllegaArgumentException, not a NPE when the argument is bogus.

git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/io/trunk@383730 13f79535-47bb-0310-9956-ffa450edef68

Author: Sandy McArthur Jr

src/java/org/apache/commons/io/FileUtils.java

@@ -24,6 +24,8 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.UnsupportedEncodingException;
+import java.io.FileReader;
+import java.io.Reader;
 import java.net.URL;
 import java.util.Collection;
 import java.util.Date;
@@ -923,22 +925,9 @@ public static List readLines(File file, String encoding) throws IOException {
 
     /**
      * Return an Iterator for the lines in a <code>File</code>.
-     * Please read the javadoc of {@link LineIterator} to understand
-     * whether you should close the iterator.
-     * The file is closed if an exception is thrown during this method.
-     * <p>
-     * The recommended usage patterm is:
-     * <pre>
-     * LineIterator it = FileUtils.lineIterator(file, "UTF-8");
-     * try {
-     *   while (it.hasNext()) {
-     *     String line = it.nextLine();
-     *     /// do something with line
-     *   }
-     * } finally {
-     *   LineIterator.closeQuietly(iterator);
-     * }
-     * </pre>
+     * This neccessitates creating an InputStream for the file. The only ways
+     * to close this stream are to call {@link LineIterator#close()} or let
+     * the <code>LineIterator</code> be garbage collected.
      * <p>
      * There is no lineIterator method without encoding parameter because
      * the default encoding can differ between platforms and will have

src/java/org/apache/commons/io/IOIterator.java

@@ -509,21 +509,9 @@ public static List readLines(Reader input) throws IOException {
     //-----------------------------------------------------------------------
     /**
      * Return an Iterator for the lines in a <code>Reader</code>.
-     * Please read the javadoc of {@link LineIterator} to understand
-     * whether you should close the iterator.
-     * <p>
-     * The recommended usage pattern is:
-     * <pre>
-     * LineIterator it = IOUtils.lineIterator(reader);
-     * try {
-     *   while (it.hasNext()) {
-     *     String line = it.nextLine();
-     *     /// do something with line
-     *   }
-     * } finally {
-     *   LineIterator.closeQuietly(iterator);
-     * }
-     * </pre>
+     * Unless you keep a reference to the <code>InputStream</code> the
+     * only way to close it is to call {@link LineIterator#close()} or
+     * wait for the garbage collector.
      *
      * @param reader  the <code>Reader</code> to read from, not null
      * @return an Iterator of the lines in the reader, never null
@@ -537,21 +525,9 @@ public static LineIterator lineIterator(Reader reader) {
     /**
      * Return an Iterator for the lines in an <code>InputStream</code>, using
      * the character encoding specified (or default encoding if null).
-     * Please read the javadoc of {@link LineIterator} to understand
-     * whether you should close the iterator.
-     * <p>
-     * The recommended usage pattern is:
-     * <pre>
-     * LineIterator it = IOUtils.lineIterator(stream, "UTF-8");
-     * try {
-     *   while (it.hasNext()) {
-     *     String line = it.nextLine();
-     *     /// do something with line
-     *   }
-     * } finally {
-     *   LineIterator.closeQuietly(iterator);
-     * }
-     * </pre>
+     * Unless you keep a reference to the <code>InputStream</code> the
+     * only way to close it is to call {@link LineIterator#close()} or
+     * wait for the garbage collector.
      *
      * @param input  the <code>InputStream</code> to read from, not null
      * @param encoding  the encoding to use, null means platform default
@@ -562,7 +538,7 @@ public static LineIterator lineIterator(Reader reader) {
      */
     public static LineIterator lineIterator(InputStream input, String encoding) 
                      throws IOException {
-        InputStreamReader reader = null;
+        Reader reader = null;
         if (encoding == null) {
             reader = new InputStreamReader(input);
         } else {

src/java/org/apache/commons/io/IOUtils.java

@@ -22,32 +22,23 @@
 
 /**
  * An Iterator over the lines in a <code>Reader</code>.
+ *
  * <p>
- * This iterator must be closed after use to avoid a resource leak.
- * If you read every line, then the final {@link #hasNext()} method
- * will close the iterator. If you do not fully read the iterator
- * then you must call the {@link #close()} method.
- * <p>
- * However, since the iterator methods can throw exception, we recommend
- * always calling close in a finally block:
- * <pre>
- * LineIterator it = FileUtils.lineIterator(file, "UTF-8");
- * try {
- *   while (it.hasNext()) {
- *     String line = it.nextLine();
- *     /// do something with line
- *   }
- * } finally {
- *   LineIterator.closeQuietly(iterator);
- * }
- * </pre>
+ * If you do not wish to maintain a reference to the <code>Reader</code>
+ * you can call {@link #close()} to close the backing <code>Reader</code>
+ * and free an interal resources.
  *
  * @author Niall Pemberton
  * @author Stephen Colebourne
+ * @author Sandy McArthur
  * @version $Id$
  * @since Commons IO 1.2
  */
-public class LineIterator implements IOIterator {
+/*
+ * XXX: hasNext() should be reworked so this class can be
+ * meaningfully subclassed before the final below is removed.
+ */
+public final class LineIterator {
 
     /** The reader that is being read. */
     private final BufferedReader bufferedReader;
@@ -60,22 +51,24 @@ public class LineIterator implements IOIterator {
      * Constructs an iterator of the lines for a <code>Reader</code>.
      *
      * @param reader the <code>Reader</code> to read from, not null
-     * @throws NullPointerException if the reader is null
+     * @throws IllegalArgumentException if the reader is null
      */
-    public LineIterator(Reader reader) {
+    public LineIterator(final Reader reader) throws IllegalArgumentException {
         if (reader == null) {
-            throw new NullPointerException("Reader must not be null");
+            throw new IllegalArgumentException("Reader must not be null.");
         }
         if (reader instanceof BufferedReader) {
-            this.bufferedReader = (BufferedReader) reader;
+            bufferedReader = (BufferedReader) reader;
         } else {
-            this.bufferedReader = new BufferedReader(reader);
+            bufferedReader = new BufferedReader(reader);
         }
     }
 
     //-----------------------------------------------------------------------
     /**
      * Indicates whether the <code>Reader</code> has more lines.
+     * If there is an <code>IOException</code> then {@link #close()} will
+     * be called on this instance.
      *
      * @return <code>true</code> if the Reader has more lines
      * @throws IllegalStateException if an IO exception occurs
@@ -89,7 +82,7 @@ public boolean hasNext() {
             try {
                 cachedLine = bufferedReader.readLine();
                 if (cachedLine == null) {
-                    close();
+                    finished = true;
                     return false;
                 } else {
                     return true;
@@ -130,15 +123,13 @@ public String nextLine() {
      * Closes the underlying <code>Reader</code> quietly.
      * This method is useful if you only want to process the first few
      * lines of a larger file. If you do not close the iterator
-     * then the <code>Reader</code> remains open and is a resource leak.
+     * then the <code>Reader</code> remains open.
      * This method can safely be called multiple times.
      */
     public void close() {
-        if (!finished) {
-            IOUtils.closeQuietly(bufferedReader);
-            finished = true;
-            cachedLine = null;
-        }
+        finished = true;
+        IOUtils.closeQuietly(bufferedReader);
+        cachedLine = null;
     }
 
     /**