RFR: 8157474: clean up/simplify/rename ModuleWrappers class

[Jonathan Gibbons]

On 05/20/2016 04:25 PM, Jonathan Gibbons wrote:
>
>
> On 05/20/2016 04:20 PM, Mandy Chung wrote:
>>> On May 20, 2016, at 1:54 PM, Jonathan Gibbons 
>>> <jonathan.gibbons at oracle.com> wrote:
>>>
>>> Please review this change to simplify and rename the javac 
>>> ModuleWrappers class.
>>> There is no deliberate change in functionality.
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8157474
>>> Webrev: http://cr.openjdk.java.net/~jjg/8157474/webrev.00/
>> Looks fine.  Nits:
>>
>>    58     public static final class ServiceLoaderHelper {
>>
>> You have a good convention to name the helper class same as the JDK 9 
>> class name.  Should this ServiceLoaderHelper one be renamed to 
>> “ServiceLoader” too?
>
> Good point.  It was deliberate to use Helper, but since we only use 
> this for a static method, and never need to create a wrapped instance 
> of ServiceLoader, we can probably simplify the name.
... but the return type is a real ServiceLoader, so it would cause 
confusion to force the call site to deal with the same class name in two 
different packages.

>
>>
>> On this Class::forName call:
>>   123                     moduleFinderClass = 
>> Class.forName("java.lang.module.ModuleFinder", false, 
>> ClassLoader.getSystemClassLoader());
>>
>> It’s okay to use ClassLoader.getSystemClassLoader() for delegation.  
>> Since the classes are in java.base, it’s safe to always pass null 
>> loader.  It’s nit.
>
> null is good.  Thanks for the tip.
>
>>
>> Mandy
>