I made some more adjustments to your checkin. In particular I put the
example code back in. I am choosing not to mention garbage collection,
as I don't trust our users not to complain. (If a particular user knows
enough to leave it to gc then they don't need to docs anyway)
I also put closeQuietly back into the tests. Without the try-finally and
closeQuietly, a test failure was hidden by other errors. This emphasises
the value of the usage pattern to me.
I have also added a filter method to LineIterator and made it non-final.
Let me know if you still have issues, as I'd like to release.
Stephen
Stephen Colebourne wrote:
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
