try-with-resources and null resource (original) (raw)

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