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

[Xuelei Fan]

On Aug 9, 2011, at 10:16 AM, Alexandre Boulgakov <alexandre.boulgakov at oracle.com> wrote:

> 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 would prefer to change it back to LNE, and keep the new ALNE class, since the most confusing place is the code using the LNE, I think.

Xuelei



> 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
>>>> 
>