Your patch got lost, but perhaps you could commit it and then I'll review.
I think I agree with most of your points, but I still want to be able to
manually close the iterator, and to have a closeQuietly to help with
that (closeQuietly is [io] style)
Stephen
Sandy McArthur wrote:
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 <[EMAIL PROTECTED]> wrote:
On 3/5/06, Stephen Colebourne <[EMAIL PROTECTED]> 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
------------------------------------------------------------------------
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
