Code Review 7000511: PrintStream, PrintWriter, Formatter leave files open when exception thrown (original) (raw)

Chris Hegarty chris.hegarty at oracle.com
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: 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.

More information about the core-libs-dev mailing list