CloseableStream exceptions [was: Re: experience trying out lambda-8-b74]

[Henry Jen]

My preference is mainly build on top of that factory methods throws IOException. I still believe factory methods should throw IOException instead of wrap it up into UncheckedIOException so to enforce a try thus better reminds using try-with-resource to ensure close. So that is not going to change for those APIs in nio.

However, that should probably be a separate concern from what is in CloseableStream, not all factory method need to throw IOException although it is more likely.

The wrapping cost and suppress of exceptions(as in JDK 7[1]) is not really a big deal, it's great if we don't need them, but if we have to, we have to.

On change the definition of CloseableStream, which would be preferred?

CloseableStream<T> close();

or

CloseableStream<T> close() throws Exception;

Also should we change it into AutoCloseableStream() as it is really just an AutoCloseable? Personally I like CloseableStream as I assume AutoCloseable is just a remedy for Closeable.

Cheers,
Henry 

[1] http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html#suppressed-exceptions


On Feb 1, 2013, at 9:38 PM, Peter Levart <peter.levart at gmail.com> wrote:

> 
> On 02/02/2013 12:44 AM, Henry Jen wrote:
>> On 01/28/2013 12:16 AM, Peter Levart wrote:
>> 
>>> On 01/28/2013 12:06 AM, Henry Jen wrote:
>>> 
>>>> http://cr.openjdk.java.net/~henryjen/lambda/nio.5/webrev/
>>>> 
>>>> 
>>>> 
>>> Hi Henry,
>>> 
>>> I can imagine that many times a single block of code would be
>>> responsible for constructing a Path stream (possibly enclosed into a
>>> try-with-resources construct) and consuming it's results, so having to
>>> deal with the duality of IOException/UncheckedIOException is a
>>> complication for a user in my opinion. Wouldn't it be better also for
>>> the stream-factory methods to throw UncheckedIOException and for
>>> CloseableStream.close() to also throw UncheckedIOException (that means,
>>> only inheriting from AutoCloseable, not Closeable and co-variant-ly
>>> redefining the throws declaration):
>>> 
>>> public interface CloseableStream<T> extends Stream<T>, AutoCloseable {
>>>     @Override
>>>     void close() throws UncheckedIOException;
>>> }
>>> 
>>> 
>> Hi Peter,
> 
> Hi Henry,
> 
>> Sorry for the late reply, I want to let the idea sink a little bit.
>> 
>> After couple days, I am slightly prefer not to change it because,
>> 
>> 1) The CloseableStream-factory method throws IOException reminds use of
>> try-with-resource better than an unchecked exception.
>> 
> 
> The CloseableStream method return type name also reminds of that ;-)
> 
>> 2) As factory methods throwing IOException, developer is dealing with
>> duality already.
>> 
> 
> He is dealing with duality already *if* the factory methods are throwing IOException. If they are throwing UncheckedIOException, he is not dealing with duality.
> 
>> 3) If the close method throws UncheckedIOException as the stream
>> handling, the suppressing of exceptions will be more confusing. Should
>> developer look into cause IOException or the UncheckedIOException?
>> 
> 
> Do you think a programmer might want to handle different subtypes of IOException differently? If that is the case, the javadoc should document all the possible situations. And what about different     subtypes of IOException wrapped by UncheckedIOException while consuming the stream? If the programmer already bothers to unwrap the unchecked exception to do the cause analisys, then this same handler would be good also for dealing with exceptions thrown in factory methods and CloseableStream.close(). The streams API is a higher-lever wrapper over the java.nio.file.DirectoryStream API and it is already wrapping the lower-level IOException with UncheckedIOException when consuming the CloseableStream. I think it should do it consistently. By doing it consistently, it simplifies correct exception handling logic in *all* situations.
> 
>> 4) When the implementation is a Closeable, the wrapping of IOException
>> into an UncheckedIOException doesn't do any good except overhead in case
>> developer want to deal with it. On the other hand, a IOException handler
>> is probably in place as the factory methods throws IOException.
>> 
> 
> It is probably in place *if* the factory methods are throwing IOException. If they are throwing UncheckedIOException, then such handler is not there. The question is whether the UncheckedIOException handler is in place too. If I look in one of your tests:
> 
>  148         public void testWalk() {
>  149                 try(CloseableStream<Path> s = Files.walk(testFolder)) {
>  150                         Object[] actual = s.sorted(Comparators.naturalOrder()).toArray();
>  151                         assertEquals(actual, all);
>  152                 } catch (IOException ioe) {
>  153                         fail("Unexpected IOException");
>  154                 }
>  155         }
> 
> 
> You haven't bothered to handle the UncheckedIOException (because the test would fail anyway if it was thrown). But I'm afraid that average programmer will walk away from similar codes with false sense of confidence that he handled all exceptional situations when he put the checked exception handler in place. I think that being consistent and throwing UncheckedIOException everywhere would     actually have greater probability for average programmer to not miss the handling of exceptional situations while consuming the stream - at least all exceptional situations would be handled or not handled equally.
> 
> Regards, Peter
> 
>> Does it make sense?
>> 
>> I updated the webrev to have some test coverage for exception handling,
>> it's painful as always, but the duality is not what bothers me.
>> 
>> 
>> http://cr.openjdk.java.net/~henryjen/lambda/nio.6/webrev/
>> 
>> 
>> Cheers,
>> Henry
>> 
>> 
>