That makes sense. I'll do that with automatic refactoring in Netbeans. Do you want me to send another webrev afterwards?
Thanks, Sasha ----- Original Message ----- From: xuelei....@oracle.com To: alexandre.boulga...@oracle.com Cc: joe.da...@oracle.com, core-libs-dev@openjdk.java.net Sent: Monday, August 8, 2011 7:30:09 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 Aug 9, 2011, at 10:16 AM, Alexandre Boulgakov <alexandre.boulga...@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....@oracle.com > To: alexandre.boulga...@oracle.com > Cc: joe.da...@oracle.com, core-libs-dev@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 >>>> >
