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

[Chris Hegarty]

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:
>> 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.

Thanks for the comments. Please take a look at the latest changes.
   http://cr.openjdk.java.net/~chegar/7000511/webrev.02/webrev/

-Chris.

>
> -Alan.