Code Review 7000511: PrintStream, PrintWriter, Formatter leave files open when exception thrown

Chris Hegarty chris.hegarty at
Tue Jan 4 15:34:57 UTC 2011

On 16/12/2010 11:46, Alan Bateman wrote:
> Chris Hegarty wrote:
>> :
>> Thanks for your comments Alan. I reworked the changes to include them.
>> Updated 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/ It might be good to add the
> bugID for future archaeologists trying to understand the behavior change
> where UnsupportedEncodingException is thrown instead of FNF.

Thanks for the comments. Please take a look at the latest changes.


> -Alan.

More information about the core-libs-dev mailing list