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.




Reply via email to