Code Review 7000511: PrintStream, PrintWriter, Formatter leave files open when exception thrown
Alan Bateman
Alan.Bateman at oracle.com
Thu Dec 16 11:46:57 UTC 2010
Chris Hegarty wrote:
>
> :
> Thanks for your comments Alan. I reworked the changes to include them.
>
> Updated webrev:
> http://cr.openjdk.java.net/~chegar/7000511/webrev.01/webrev/
This looks better. A few comments:
In PrintStream it looks like the charset will now be checked twice, once
by using isSupported and again when creating the OutputStreamWriter. As
OutputStreamWriter has a constructor that takes a Charset (and so
doesn't throw UnsupportedEncodingException) then maybe it would be
simpler to just replace verifyCharsetName with a toCharset(String)
method that does the Charset.forName and returns the Charset. If you did
that, and introduced a private constructor that takes the Charset as its
first parameter then it might be cleaner.
Same thing in PrinterWriter and Formatter and you would avoid the Void
parameter trick that might not be clear to future maintainers.
Minor nit is that the new private constructor in PrinterWriter might be
better placed before the public constructors that use it. Also minor
comment on Formatter is that the private static getZero method is
declared final :-)
On Scanner there is one other constructor that has the same issue (line
726).
On the tests, it's nice to see multi-catch being used in
test/java/util/Formatter/Constructors.java. It might be good to add the
bugID for future archaeologists trying to understand the behavior change
where UnsupportedEncodingException is thrown instead of FNF.
-Alan.
More information about the core-libs-dev
mailing list