[Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

[Alexandre Boulgakov]

I can change it back to LdapNamingEnumeration. I just thought it would be more consistent with LdapBindingEnumeration and LdapSearchEnumeration. I did not think of the back-porting issue. Would it be better to change AbstractLdapNamingEnumeration to LdapNamingEnumeration, as far as back-porting, since they share the most code? Or do you mean for back-porting code that uses these enumerations?

I don't think you need to review the other changes; they are specific change that were already discussed and agreed on.

Thanks,
Sasha

----- Original Message -----
From: xuelei.fan at oracle.com
To: alexandre.boulgakov at oracle.com
Cc: joe.darcy at oracle.com, core-libs-dev at openjdk.java.net
Sent: Monday, August 8, 2011 6:58:41 PM GMT -08:00 US/Canada Pacific
Subject: Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

On 8/6/2011 2:11 AM, Alexandre Boulgakov wrote:
> Here's the second version:
> http://cr.openjdk.java.net/~mduigou/7072353/2/webrev/
> <http://cr.openjdk.java.net/%7Emduigou/7072353/2/webrev/>
> 
>   * Changed LdapResult.referrals to be a Vector<Vector<String>>;
>   * Refactored
>       o com/sun/jndi/dns/DnsContext.java: BaseNameClassPairEnumeration<T>;
>       o com/sun/jndi/toolkit/dir/HierMemDirCtx.java: BaseFlatNames<T>;
>       o com/sun/jndi/ldap/*.java: refactored LdapNamingEnumeration into
>         AbstractLdapNamingEnumeration, LdapNameClassPairEnumeration;
>         changed LdapBindingEnumeration and LdapSearchEnumeration
>         accordingly, as well as a few of their consumers;
Is it necessary to rename LdapNamingEnumeration to
LdapNameClassPairEnumeration? I think it might not good for sustaining
work, as when backport a fix from JDK 8 to previous releases, we will
have to recognize and rename the class accordingly.

Otherwise, looks fine to me.

>   * Made a few additional small changes that were discussed.
> 
I did not review other updates. There are too many files, if you want me
review all of the changes again, please let me know.

Thanks,
Xuelei

> Thanks to everyone who reviewed the last version!
> 
> Thanks,
> Sasha
> 
> On 8/4/2011 2:00 PM, Alexandre Boulgakov wrote:
>>
>> On 8/3/2011 10:44 PM, Joe Darcy wrote:
>>> David Holmes wrote:
>>>> Joe Darcy said the following on 08/04/11 12:33:
>>>>> David Holmes wrote:
>>>>>>> Using wildcards makes the subtyping work along the type argument
>>>>>>> axis.
>>>>>>
>>>>>> So what is the right fix here? To declare the underlying Vector as
>>>>>> a Vector<?> and cast it to something concrete when needed? It
>>>>>> seems very wrong to me to be inserting raw type casts all through
>>>>>> this code.
>>>>>
>>>>> It isn't entirely clear to be from a quick inspection of the code
>>>>> what the actual type usage is.  A writable general Vector should be
>>>>> a Vector<Object> and Vector just meant for reading should be a
>>>>> Vector<?> (or the equivalent Vector<? extends Object>).
>>>>>
>>>>> If the type usage is "a sequence of X's and Y's" where X and Y
>>>>> don't have some useful supertype, I would recommend using a
>>>>> somewhat different set of data structures, like a list of type-safe
>>>>> heterogeneous containers or a list of a new package-level XorY class.
>>>>
>>>> Buried in one of Sasha's emails it says:
>>>>
>>>> "The current code uses it to store Strings and Vector<String>s."
>>>>
>>>> Hence it is declared Vector<Object>.
>>>>
>>>> This is looking to me like code that should not have been generified.
>>>>
>>>
>>> Hmm.  If String or Vector<String> are the two kinds of stored values,
>>> I would recommend Vector<Vector<String>> where a lone string was
>>> wrapped in a vector.  (Actually, I would recommend a
>>> List<List<String>>, but that may be beyond the scope of this cleanup.
>>
>> I can do Vector<Vector<String>>.
>>
>> List<List<String>> is probably beyond the scope of removing compiler
>> warnings; getting rid of Vectors and Hashtables in general could take
>> a whole summer by itself, and would probably be best to do as a whole,
>> since it's not always clear at first glance if other classes depend on
>> a particular object being a Vector.
>>
>> -Sasha
>>>
>>> -Joe
>>>