On 12/7/2012 3:37 PM, Mandy Chung wrote:
On 12/7/12 3:24 PM, Joe Wang wrote:
Thanks for reviewing the changes. Please see comments inline.
On 12/7/2012 11:38 AM, Mandy Chung wrote:
On 12/5/12 10:58 AM, Alan Bateman wrote:
http://cr.openjdk.java.net/~alanb/8004371/webrev/
Yay! Properties no longer requires JAXP to be present in order to
load/store properties in XML format. This looks okay to me. Some
minor comments:
XMLStreamWriterImpl.isEncodingSupported - it returns true if any
string != "UTF-32" that assumes the given encoding is one of the few
known values. Would it be better to check the explicit list of
supported encoding?
private boolean isEncodingSupported(String encoding) {
if (encoding.equalsIgnoreCase("UTF-32")) {
return false;
}
return true;
}
The spec only requires UTF-8 and 16. But the writer can actually
handle most of the encodings. An explicit list would be quite long
and require comprehensive tests. Since the ukit parser explicitly
rejects UTF-32, I chose to do the same. Other than that, the
XMLWriter leaves it to java.nio.charset.Charset to determine if a
specified encoding is supported through this call:
try {
cs = Charset.forName(encoding);
} catch (IllegalCharsetNameException |
UnsupportedCharsetException ex) {
throw new XMLStreamException(new
UnsupportedEncodingException(encoding));
}
OK - you leave it for XMLWriter to check if it's a valid encoding. It
may be better to include Charset.forName check in the
isEncodingSupporting method that would avoid confusion.
Sure. Will move this code into isEncodingSupported and change the method
name to getCharset so that it'd return the charset if the encoding is
not UTF32 and supported by the jdk.
PropertiesDefaultHandler.java L115-116: can be replaced with
Properties.stringPropertyNames() and foreach.
Done. Will send update to Alan.
XMLStreamWriterImpl.java L156 - since the caller needs to unwrap
XMLStreamException to determine if the encoding is supported or not,
maybe better to throw this constructor to throw
UnsupportedEncodingException.
The writer implements partially the original StAX API. So I tried
not to change the writer API. But java.util.Properties expect
UnsupportedEncodingException in case of invalid encoding, thus the
compromise.
OK - PropertiesDefaultHandler uses the XMLStreamWriter API. That's
fine with me.
Thanks,
Joe
Thanks
Mandy
