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