[Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]
alexandre.boulgakov at oracle.com
Mon Aug 8 19:16:55 PDT 2011
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.
----- 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:
> * 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 to everyone who reviewed the last version!
> 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
>>>>>> 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.
More information about the core-libs-dev