commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sandy McArthur" <sandy...@apache.org>
Subject Re: [io] LineIterator suggestions [was: LineIterator finalize]
Date Mon, 06 Mar 2006 02:45:48 GMT
Attached is the changed I'd make. If no one objects to those changes I
can commit it myself.

On 3/5/06, Sandy McArthur <sandymac@apache.org> wrote:
> On 3/5/06, Stephen Colebourne <scolebourne@btopenworld.com> wrote:
> > Sandy McArthur wrote:
> >  >>>I don't think LineIterator should have a finalizer method and I
> >  >>>believe the JavaDocs in that class about resource leaks are wrong and
> >  >>>unnecessarily alarming.
> > How is the javadoc over the top? I'll happily make changes, or go ahead
> > yourself.
>
> I checked out the IO trunk and here is what I'd change relating to the
> current LineIterator:
>
> * I think IOIterator should be removed. It's based on the premise that
> an Iterator needs special action else it will leak resources. Also
> there is only one implementation of this interface, which doesn't
> actually leak anything. I don't believe it's utility justify it's
> existence. Maybe in a later release if you find yourself adding a
> number of Iterators with a close() method you can add it back in and
> retro fit LineIterator to implement it.
>
> * Don't automatically closing the Reader when the last line is read.
> The LineIterator potentially breaks being used with the
> java.io.PushbackReader. PushbackReader lets the Reader backup so to
> speak but it cannot do that if the Reader is closed.
>
> * Either make LineIterator final or change the way hasNext works to
> allow meaningful subclassing. As hasNext() is currently implemented
> there is no way for a sub-class that filters lines that only match a
> regex without reimplementing hasNext() completely.
>
> * Remove the static method closeQuietly(LineIterator). I don't think
> it's useful enough to justify itself.
>
> * Change the constructor to throw an IllegalArguementException not a
> NullPointerException. I personally view an NPE as the result of trying
> to dereference a field or method of null. The constructor doesn't
> actually dereference reader, we are testing that the argument reader
> is a legal and meaningful value to preemptively prevent a future NPE.
>
> --
> Sandy McArthur
>
> "He who dares not offend cannot be honest."
> - Thomas Paine
>


--
Sandy McArthur

"He who dares not offend cannot be honest."
- Thomas Paine


Mime
View raw message