try-with-resources and null resource

Stephen Colebourne scolebourne at joda.org
Thu Jan 27 01:10:31 PST 2011


On 26 January 2011 18:05, Tim Peierls <tim at peierls.net> wrote:
> No, I am writing about what I think the semantics of the construct should
> be. I'm drawing an analogy between two constructs, one old, one new, that
> many users will see as similar, rightly or wrongly. Their expectations for
> the new construct's semantics will be shaped by this perception. I claim
> that most users won't expect NPE to be thrown immediately if the initializer
> is null, and that they will expect NPE to be thrown on the first attempt to
> dereference a null. They'll expect this because that's what happens in the
> analogous case.
>
> You can say the analogy is false, you can say that people should approach
> this construct as being radically different from try-finally, you can even
> say that most people are sloppy thinkers. But you can't change how they
> think by wishing, and it would be wrong to design the feature without taking
> such people into account.

Funnily enough, I have proposed similar analogies in the past. But
they are seductively dangerous. How many devs looking at a foreach
loop know it uses an iterator? Or an index if its an array? The same
applies here.

A similar case is closures/lambdas. There, the analogy is to inner
classes, yet after much thought, Oracle has come to the same
conclusion I did - that "this" within a lambda should not act as
"this" does in an inner class. Sometimes, we have to break with the
analogy to get the right result.

Anyway, a real use case is far more helpful:

Consider getResourceAsStream() which returns null if the resource does
not exist:

try (InputStream stream =
getClass().getResourceAsStream("/com/foo/mightOrMightNotExist")) {
  if (stream != null) {
    readStream(stream);
  }
}

This will fail ATM, because of that null, yet the code is perfectly
clear and readable. What's the alternative?

InputStream stream =
getClass().getResourceAsStream("/com/foo/mightOrMightNotExist");
if (stream != null) {
  try (InputStream stream2 = stream) {
    readStream(stream2);
  }
}

That now works, but is significantly less clear to read, more verbose
and more prone to accidental refactoring.

In fact, there was a bug when I first typed this email, as I wrote
readStream(stream) rather than readStream(stream2). In this case, it
would have no bad effects, but other similar cases might.

In fact, I'd probably still write:
InputStream stream =
getClass().getResourceAsStream("/com/foo/mightOrMightNotExist");
try {
  if (stream != null) {
    readStream(stream);
  } finally {
    org.apache.commons.lang.IOUtils.closeQuietly(stream);
  }
}

as it is clearer than the t-w-r version.


Executive summary: C# has got it right. Null resources should be
silently be accepted.

Priority. Initially I said this didn't matter much to me. With this
new use case (straight out of my day job yesterday), I really care
about my conclusion now. Please reconsider the semantics based on this
use case.

Stephen



More information about the coin-dev mailing list