review request for 7022624, convert java.io test to use try-with-resources
Alan Bateman
Alan.Bateman at oracle.com
Tue Mar 1 09:46:50 UTC 2011
Stuart Marks wrote:
> Here's a small webrev with changes to a handful of java.io tests to
> use TWR.
>
> http://cr.openjdk.java.net/~smarks/reviews/7022624/webrev.0/
>
> I have a few minor questions:
>
> * test/java/io/File/SetLastModified.java
>
> Should the channel corresponding to a stream be considered a separate
> resource, or is it more like another object that represents the "same"
> resource? Since closing one closes the other, I treated them as peers
> and I didn't unroll the FileOutputStream and its channel into separate
> resource variables. However, I'm flexible on this.
The associated channel is connected to the same file as the input
stream. In theory it might be possible for getChannel to fail with a
virtual machine error (stack overflow or OOME for example) but for this
test I think what you have is fine.
>
> * test/java/io/OutputStreamWriter/Encode.java
>
> Pretty clearly a ServerSocket is a distinct resource from a Socket
> returned from the accept() call. However, does Socket.getInputStream()
> represent a distinct resource from the Socket? In this case it seemed
> most sensible to unroll them into separate resource variables, but
> again I could go either way on this.
I wouldn't bother but would instead reduce this down to three resources,
maybe:
try (ServerSocket listener = ss;
Socket s = listener.accept();
BufferedReader reader = new BufferedReader(new
InputStreamReader(s.getInputStream()))
{
...
}
While you are there, I assume ss should be final. Also might be good to
put a try/finally around the encode at L50-52 to ensure that disconnect
is called.
>
> * test/java/io/PrintStream/FailingConstructors.java
>
> Please check over my NIO usage here. Hm, I did more conversion here
> than in the other FailingConstructors tests (see webrev for 7021209).
> Maybe I should go back and fix the others....
Looks fine to me.
>
> *
> test/java/io/Serializable/evolution/RenamePackage/install/SerialDriver.java
>
> *
> test/java/io/Serializable/evolution/RenamePackage/test/SerialDriver.java
>
> Odd, seems like mostly duplicate code....
I don't know the history of that test but it does appear that it could
have been written to generate the class to both packages rather than
duplicating.
I've looked at the other tests in the webrev and all looks okay to me.
-Alan.
More information about the core-libs-dev
mailing list