try-with-resources and null resource

[Reinier Zwitserloot]

Using t-w-r with a null resource would do something other than throw an NPE
in the body if the body contains an if-based null check, for example.

 --Reinier Zwitserloot



On Fri, Jan 28, 2011 at 8:18 AM, David Holmes <David.Holmes at oracle.com>wrote:

> 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
>>
>>
>>