On Wed, 4 Oct 2023 22:56:28 GMT, Joe Wang <jo...@openjdk.org> wrote:
> Add a new factory method so that a CatalogResolver can be created with a
> resolve property on top of the Catalog object.
src/java.xml/share/classes/javax/xml/catalog/CatalogManager.java line 101:
> 99: * and {@code ignore}. {@code null} may be specified to indicate that
> the
> 100: * {@code catalog} object's current {@link
> CatalogFeatures.Feature#RESOLVE RESOLVE}
> 101: * value remains unchanged.
Supported values explanation and the behavior on `null` string may be moved
into the method description body, as it explains how this method is supposed to
work.
src/java.xml/share/classes/javax/xml/catalog/CatalogManager.java line 106:
> 104: * @throws IllegalArgumentException if the value of the {@code
> resolve} property is
> 105: * not a supported value or {@code null}.
> 106: *
`@throws NPE` for `catalog==null` is needed
src/java.xml/share/classes/javax/xml/catalog/Util.java line 280:
> 278: * Catalog implementation
> 279: * @param uri the specified uri
> 280: * @return Returns the absolute form of the specified uri
"Returns" may be redundant. Anyway, can this `Util` class be `final` and have
private constructor not to be instantiated?
src/java.xml/share/classes/javax/xml/catalog/Util.java line 298:
> 296: return temp;
> 297: } catch (MalformedURLException ex) {
> 298: // no action, shouldn't happen as the base has already been
> validated
If this "shouldn't happen", is that considered an Error, which should be
reported?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16045#discussion_r1347735825
PR Review Comment: https://git.openjdk.org/jdk/pull/16045#discussion_r1347766234
PR Review Comment: https://git.openjdk.org/jdk/pull/16045#discussion_r1347749031
PR Review Comment: https://git.openjdk.org/jdk/pull/16045#discussion_r1347756200
