[Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]
Alexandre Boulgakov
alexandre.boulgakov at oracle.com
Fri Aug 5 18:11:34 UTC 2011
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;
* Made a few additional small changes that were discussed.
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
>>
More information about the core-libs-dev
mailing list