try-with-resources and null resource

[David Holmes]

Reinier Zwitserloot said the following on 01/28/11 16:54:
> I don't understand why one couldn't explain t-w-r by showing "if 
> (resource != null) resource.close();" in the finally block.

One could of course do this. But most usages will involve accessing 
resource within the try block, which would result in NPE anyway.

> It is then your opinion that this code is abusing the t-w-r construct, 
> because Class.getResourceAsStream can return null?
> 
> try (InputStream introTextStream = 
> getClass().getResourceAsStream("intro.txt")) {
>     ...
> }

getResourceAsStream is a problematic example as it should not have been 
defined to return null. But that aside ... yes, that is my opinion. If 
the stream here may not exist I have to deal with that fact in my code. 
But you need to expand your example to show what the try block actually 
attempts to do with the stream that might be null. Your expectations in 
the null case will naturally influence what you expect the construct to do.

> If so, we've clearly progressed into the "Everyone has their own 
> 'feeling', none of them agree with each other, and making language 
> decisions based on these feelings is a silly thing to do" phase. Let's 
> check how often the above occurs. I'm betting it'll be "loads of times". 

The question to answer is: what do those "loads of times" do if they 
encounter a null?

> Also contrast what you'd have to write if the above is not okay:
> 
> InputStream introTextStream1 = getClass().getResourceAsStream();
> if (introTextStream1 != null) {
>     try (InputStream introTextStream2 = introTextStream1) {
>         ...
>     }
> }
> 
> I assume I don't have to resort to 'feeling' to claim that the above is 
> just bad.

What's the bad part? Having to declare a secondary variable? Certainly 
not ideal, but hardly fatal - we have to do that to capture locals as 
finals for inner class access. Also the secondary variable is a 
consequence of a different decision to limit the expression to be a 
local variable declaration. I wasn't involved in any discussions of that 
and I think it unfortunate that you can't use:

    final InputStream istream = getClass().getResourceAsStream();
    if (istream != null) {
      try (istream) {
        ...
      }
    }

provided istream is a reference to a final, or effectively final, value.

To me a null reference is a bad input for try-with-resources and bad 
input should be rejected.

David Holmes

>  --Reinier Zwitserloot
> 
> 
> 
> On Fri, Jan 28, 2011 at 7:38 AM, David Holmes <David.Holmes at oracle.com 
> <mailto:David.Holmes at oracle.com>> wrote:
> 
>     Tim Peierls said the following on 01/28/11 00:47:
>      > I don't know about C#, but I like the formulation that can be
>     sloppily
>      > summarized as "t-w-r calls close on non-null resources".
> 
>     So to summarize there are three potential candidates for:
> 
>     try (Resource r = getResource() {
>       // stuff here
>     }
> 
>     I'm ignoring the suppressed exception aspects for simplicity:
> 
>     a) Simple transformation into try-finally (current implemented
>     semantics)
> 
>       Resource r = getResource();
>       try {
>         // stuff here
>       }
>       finally {
>         r.close();
>       }
> 
>     b) Transformation into try-finally with explicit null-check on entry
> 
>       Resource r = getResource();
>       Objects.requireNonNull(r); // heh heh - couldn't resist!
>       try {
>         // stuff here
>       }
>       finally {
>         r.close();
>       }
> 
>     c) Simple transformation into try-finally but only close a non-null
>     resource
> 
>       Resource r = getResource();
>       try {
>         // stuff here
>       }
>       finally {
>         if (r != null)
>           r.close();
>       }
> 
> 
>     Option (a) is, perhaps, the most consistent with preserving the
>     behaviour of current code that would be modified to use the new
>     construct. This construct will be taught by analogy as it is the only
>     effective way to explain things "try-with-resources behaves as-if you
>     had written ....". That said I think existing code would have an
>     explicit null-check to avoid the try-finally in the null case. And I
>     think it is potentially risky to execute the try block only to have NPE
>     thrown later.
> 
>     Option (c) seems appealing in some sense because there's no NPE's at all
>     if you don't use "r", but I think that appeal is superficial. To me the
>     fundamental question to ask is: what is try-with-resources for? And I
>     believe the answer to that is to initialize, use and then clean-up an
>     AutoCloseable object. So to me using try-with-resource on a resource
>     that might be null is simply a mis-use of try-with-resources: if it can
>     be null you can't autoclose it, so trying to is a programming error and
>     programming errors should be detected and reported as early as possible.
>     So to me option (b) is the preferred semantics for this new construct.
> 
>     David Holmes
> 
>