RFR: javax.xml.transform: Using ServiceLoader to load JAXP stream factories (7169894: JAXP Plugability Layer: using service loader)

[Daniel Fuchs]

Hi,

Please find below an updated webrev for the javax.xml.transform
package, taking into account Mandy's & Joe's comments - namely:

1. Fixed call to creationMethod.invoke.

2. Got rid of the 4 args FactoryFinder.newInstance method.

3. Got rid of the useBSClsLoader which was always passed as 'false'.
    (thanks Mandy!)

<http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.transform/webrev.01/>


-- daniel

On 12/18/12 11:02 PM, Daniel Fuchs wrote:
> On 12/18/12 9:06 PM, Mandy Chung wrote:
>> In FactoryFinder.newInstanceNoServiceLoader method, L223, 226 -
>> NoSuchMethodException will be thrown if such method doesn't exist.
>> creationMethod will not be null.
> Thanks - yes - you're right of course - no need to check for null...
>> L236 - this change is not needed, right?  The method is a static
>> no-arg method. You passed an additional argument creationMethod as the
>> first parameter although it's harmless as it's ignored.
> Oops - my bad. That's a mistake - I did too many successive changes -
> should be creationMethod.invoke(null) of course.
> And my tests didn't even catch it!
>>
>> A minor comment:
>>   151     static<T>  T newInstance(Class<T>  type, String className,
>> ClassLoader cl, boolean doFallback)
>>   152         throws TransformerFactoryConfigurationError
>>   153     {
>>   154         return newInstance(type, className, cl, doFallback,
>> false, false);
>>   155     }
>>
>> The FactoryFinder.newInstance method 4-argument version is only called by
>> TransformerFactory.newInstance(String factoryClassName, ClassLoader
>> classLoader).
>> Perhaps you can clean this up TransformerFactory to call the
>> Factory.newInstance
>> method 6-argument version.
> 3 successive boolean parameters... I hate that ;-)  Yes I think I can do
> this cleanup...
>
> Thanks Mandy,
>
> -- daniel
>
>>
>> Thanks
>> Mandy
>