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