On 6/25/2012 5:14 AM, Alan Bateman wrote:
On 25/06/2012 08:38, Joe Wang wrote:
Hi all,
Thanks for all the comments!
I've updated the patch for the following recommended changes:
1. ServiceConfigurationError instead of ConfigurationError
2. Use factory class, class.forName section removed
3. Use load method without specifying classloader
4. To be clear on how the process handles exceptions, I've updated
JavaDoc with the following:
* Uses the service-provider loading facilities, defined by the
{@link java.util.ServiceLoader} class, to attempt
* to locate and load an implementation of the service. In case
multiple providers exist, the first
* valid 3rd party provider should be returned; If there is no
valid 3rd party provider,
* the default implementation is returned if it is on the
classpath or installed as a module.
I think this is better, and I'm okay with your suggestion to deal with
the code duplication issue separately.
Ok.
On the above wording then I think term "3rd party provider" may
provoke questions. How about "If there are providers other than the
implementation specific default located, then the first provider that
is not the default is instantiated and returned".
That looks to be a better wording.
I'm not sure about catching and ignoring the ServiceConfigurationError
(or "keep on trucking" as Paul termed it in one of the replies). The
existing specification reads " Any Exception thrown during the
instantiation process is wrapped as a
<something>ConfigurationException" so this seems a significant change
to me.
Indeed, that would be a significant change. I think I said I'm not going
to fix 6975142, or make it work for jdk7 and older in that matter, but
then I still allowed that to dictate the JavaDoc. I guess it's in my
mind for too long, or it's because the whole ServiceLoader thing was
actually started when I looked at old bugs and found 6975142 :)
Would it be cleaner if the FactoryFinder.find methods were changed to:
static <T> T find(Class<T> c, String ...)
rather than using a raw type and returning Object? That would
eliminate some of the casts in this case. I guess if the short term is
to have a package-private copy of FactoryFinder in each package then
it could be changed to return the specific type. Related to this is
the changes increase the number of warnings by using ServiceLoader
without a type parameter.
JAXP code supported -source 1.4 -target 1.4, and changed to 1.5 not long
ago. There are lot of old code in JAXP for sure.
-Joe
-Alan.