Reputation: 357
So in some method, I will be opening a new IO stream, doing some processing with it, and then using that stream as the input to open up another IO stream. I don't believe I can use a single try-with-resources block because of the processing with the first IO stream being in between opening the first and second streams. So with that said, would it be better (in a coding-design sense) to use a single try-catch-finally block to open and close these streams or use nested try-with-resources blocks to open and close the streams? I know that if there is no processing between the first and second IO streams, it would be best to open all three streams in a single try-with-resources block.
A simplistic example follows:
Try-Catch-Finally
void someMethod(InputStream is) throws SomeException {
SomeIOStream io1 = null;
SomeIOStream io2 = null;
SomeIOStream io3 = null;
try{
io1 = new SomeIOStream( someSortOfProcessing() );
io1.moreStreamProcessing();
io2 = new SomeIOStream( someSortOfProcessing(io1) );
io3 = new SomeIOStream (is);
//do stuff with io2 and io3
} catch (Throwable t) {
//Exception Handling
} finally {
//closing streams io3, io2, io1, is
}
}
Try-with-resources
void someMethod(InputStream is) throws SomeException {
try ( SomeIOStream io1 = new SomeIOStream( someSortOfProcessing() ) ){
io1.moreStreamProcessing();
try ( SomeIOStream io2 = new SomeIOStream( someSortOfProcessing(io1) );
SomeIOStreeam io3 = new SomeIOStream (is); ){
//do stuff with io2 and io3
}
} catch (Throwable t) {
//Exception Handling
} finally {
//closing stream is
}
}
To me, it looks as though the first is cleaner, but the second has the benefits of a try-with-resources block. Of course, another alternative is to open the initial io1 with try-with-resources, but open io2 and io3 within that try-block. So would this third mixed approach be better than the above two?
Mixed Approach
void someMethod(InputStream is) throws SomeException {
SomeIOStream io1 = null;
SomeIOStream io2 = null;
SomeIOStream io3 = null;
try (SomeIOStream io1 = new SomeIOStream( someSortOfProcessing() ) ){
io1.moreStreamProcessing();
io2 = new SomeIOStream( someSortOfProcessing(io1) );
io3 = new SomeIOStream (is);
//do stuff with io2 and io3
} catch (Throwable t) {
//Exception Handling
} finally {
//closing streams io3, io2, is
}
}
Also as an additional question, am I right to assume that the only way to close the InputStream is
would be to put it in the finally-block?
Upvotes: 2
Views: 74
Reputation: 298469
It seems, you are not aware, what the try with resources does for you. It does not only ensure that close()
gets called, it also ensures that in the case that close()
fails with an exception, it won’t shadow the initial exception, if there is one, but instead, record the secondary exception using addSuppressed
.
So the equivalent of
try(Resource r = allocation ) {
…
}
is
{
Resource r = allocation;
Throwable primary = null;
try {
…
}
catch(Throwable t) { primary = t; }
finally {
if(r != null) try {
r.close();
}
catch(Throwable t) {
if(primary!=null) primary.addSuppressed(t); else primary=t;
}
}
if(primary!=null) throw primary;
}
Now think again whether rewriting any of the try with resources statements, to do this manually, creates cleaner code. Even without handling the close correctly, your alternatives to the nested try with resource statements are already bigger in code size, more complicated and expanding the scope of variables beyond their actual use.
In contrast, the nested try with resource statements reflect exactly what you are doing, using resources in a nested scope. It will become even better, if you remove the questionable catch-all part and the closing of the incoming resource.
Note that in rare circumstances, closing the incoming resource might be acceptable, e.g. if it is a private
method and the behavior is well-documented. But even then, you should not resort to finally
:
void someMethod(InputStream incomingIs) throws SomeException {
try(InputStream is=incomingIs;// it must be documented that we will close incomingIs
SomeIOStream io1 = new SomeIOStream(someSortOfProcessing()) ) {
io1.moreStreamProcessing();
try(SomeIOStream io2 = new SomeIOStream(someSortOfProcessing(io1));
SomeIOStreeam io3 = new SomeIOStream (is) ) {
//do stuff with io2 and io3
}
}
}
Upvotes: 3
Reputation: 100279
It's probably opinion-based, but I would certainly prefer using try-with-resources as much as possible. This clearly shows the scope of the closeable resource usage making the program logic easier to understand. If you worry about nested try-with-resources blocks, consider extracting the inner block into the separate method.
Also if you have the stream passed as parameter (is
in your example), it's usually not a good idea to close it. If caller created this stream, it's usually caller's responsibility to close it (preferably with try-with-resource statement in caller method). Finally it's rarely good idea to catch the Throwable
.
Upvotes: 3