Revisiting MayHoldCloseableResource

[Brian Goetz]

I'm inclined to agree with Andrey here.  The flatMap use case calls for more than just "have a close".  

On Aug 3, 2013, at 3:10 AM, Andrey Breslav wrote:

> In case you are asking for opinions, I think 
> * we'd better fix the AC spec early than late;
> * have streams implement (fixed) AC (Option 1) looks better to me than just have close() (Option 2) or not having the Files.* methods (Option 3).
> 
> --
> Andrey Breslav
> http://jetbrains.com
> Develop with pleasure!
> 
> 
> On Aug 3, 2013, at 01:39 , Brian Goetz wrote:
> 
>> There seems to be consensus that the current strawman for MayHoldCloseableResoruce and @HoldsResource is too flawed to proceed.  
>> 
>> Flaw #1 relates to @HoldsResource.  While there seems to be consensus that annotations are reasonable for imparting information like this which is suited to static analysis tools, the current proposal (a) confusingly leaks through into subclass Javadoc and (b) seems too weak.  Conclusion: just dropping @HR from the current scheme (leaving only MHCR <: AC) is an improvement; we can revisit what the correct annotations for hinting at resourceful-ness later.  
>> 
>> Flaw #2 relates to MHCR.  The real problem is that AutoCloseable is overspecified; it says "a resource that must be closed."  This is an overreach; what j.l.AC should say is "has a close method which is called automatically on exit from a TWR block."  Because of this overspecify, subclassing doesn't help, because if an AC must be closed, so must a MHCR.  Conclusion: dropping MHCR and just having BaseStream <: AC is an improvement.  
>> 
>> Flaw #3 relates to the spec of AC.  The conclusion of the core library maintainers is that it is acceptable to correct the spec, and replace the "must" with "may".  This leaves us with a fixed AC, and BaseStream <: AC.  This seems strictly better than where we are now.  Let's call this "Option 1."  
>> 
>> But, it is possible we might still choose to punt further for now.  
>> 
>> Observation: the need for Stream to have a close() method comes about because of the desire to have static factory methods that will create streams that encapsulate a resource that requires closing, when there is no way to get access to that underlying resource.  Such as Files.walk(dir) or Files.lines(path).  
>> 
>> For other cases, like BufferedReader.lines(), the BR itself is AC, so users can handle resource management like:
>> 
>> try (BufferedReader b = ...) {
>>    b.lines()...
>> }
>> 
>> and there is no need to close the *stream* at all, since the user has access to the actual closeable resource.  It is only the static methods such as Files.list(dir), which don't provide the user access in this way.  The methods we provide are:
>> 
>> Files.list(Path)
>> Files.walk(Path, maxDepth, options)
>> Files.walk(Path, options)
>> Files.find(Path, maxDepth, BiPredicate<Path, BasicFileAttributes>, options)
>> Files.lines(Path)
>> 
>> Colin @ Google pointed out that if the spliterators for these streams were lazy (didn't open the resource until the terminal began), and all terminal ops called the close hook in a finally block after processing, then we wouldn't need to be AC, since there would be no way to leak a closeable resource.  (This would move resource-not-found exceptions from the factory method to the terminal op.)  
>> 
>> Still, having a close() method and *not* being AC may look weird to the users.  
>> 
>> Let's call this "Option 2": have a close() method, don't extend AC, make sure that all terminals call close, make sure that all static resource-holding stream factories produce late-binding spliterators.
>> 
>> Option 3 would be to simply get rid of these methods (note that Files.list and Files.lines are pretty simple wrappers around existing AC abstractions, DirectoryStream and BufferedReader.)  
>> 
>> Orthogonal to Options 2 and 3 would be fixing the spec of AC.
>> 
>> 
>