[JAXP] RFR: 8169778: Add new public methods to get new instances of the JAXP factories builtin system-default implementations (original) (raw)

Daniel Fuchs daniel.fuchs at oracle.com
Wed Nov 16 17:35:29 UTC 2016

Thanks Roger,

On 16/11/16 16:03, Roger Riggs wrote:

Hi Daniel,

DocumentBuilderFactory and classes: - The new methods uses "Creates" instead of "Obtain"; perhaps it should be consistent with existing newInstance methods?

I had some pre-review advice from Joe (Wang) about who suggested using the 3rd person (Obtains) rather than the imperative (Obtain).

Also I noticed that some factories use Get, some Obtain, some Create... Because this is a new method - that doesn't use any lookup mechanism, and I wanted that there be no ambiguity that a new instance will be returned with each invocation - I thought it might be better to use "Creates" in all new methods - regardless of what other verb the other method use - so for this feature, stay consistent across factories instead of being consistent within the factory.

I can revert that if there's a general feeling that staying consistent with the other methods in the same file is preferable.

- "Otherwise," and "Otherwise" (with and without ",") are not consistent in the webrev.

Right - I will add the coma where it's missing. "Otherwise, " seems to be more widely used.

XMLInputFactory.java: -line 212: the "system-default" should be inside the braces with the @link.

Good catch!

SchemaFactory.java: - line 201: (pre-existing typo) "in a implementation" -> "in an implementation"

OK.

XPathFactory.java - line 177: The {@linkplain platform...} doesn't look like a properly formed link.

Good catch again. How did I miss that?

XMLOutputFactoryNewInstanceTest - line 2, has a 1999 date but is a new file.

Right. It's surprising that this file did not exits. I copied XMLInputFactoryNewInstanceTest and subtituted Input for Output...

Many of the files have long lines (some new) that make side-by-side compares not fit on the screen.

I will crop the new and changed lines - but I don't want to fix more as it would dilute the meaningful changes in noise.

Here is a new webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8169778/webrev.01/index.html

-- daniel

Regards, Roger

On 11/16/2016 10:24 AM, Daniel Fuchs wrote: Hi, Please find below a patch for: 8169778: Add new public methods to get new instances of the JAXP factories builtin system-default implementations bug: https://bugs.openjdk.java.net/browse/JDK-8169778 webrev: http://cr.openjdk.java.net/~dfuchs/webrev8169778/webrev.00/ best regards, -- daniel

More information about the core-libs-dev mailing list