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;
}
PropertiesDefaultHandler.java L115-116: can be replaced with
Properties.stringPropertyNames() and foreach.
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.
Perhaps s/SAXParserImp/SAXParserImpl be consistent with the other
classname e.g. XMLStreamWriterImpl.
As you mentioned, there is still a good bit of clean-up required and I
skip the style and coding convention comments. One question though -
jdk.internal.org.xml.sax.** are copied from JAXP source. What should
the copyright years be? It currently retains the same value from the
original copy.
One issue that I'm still mulling over, as least for the profiles
effort, is whether to only include the fallback provider in the
smallest profile as it won't be used otherwise. If the eventual size
isn't too significant then it might not be worth it of course.
Do you plan to include jdk.internal.util.xml.BasicXmlPropertiesProvider
in META-INF/services/sun.util.spi.XmlPropertiesProvider?
I guess you are referring to what makefile change should be depending on
the decision?
Mandy
