> Please review these JNDI changes.
> Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7072353
> webrev: http://cr.openjdk.java.net/~mduigou/7072353/0/webrev/
Thanks for your effort to make JNDI free of compile-warning. The work is
hard, I appreciate it.
1. I noticed the copyright date of a few files are unchanged, please
update them before you push the changes.
2. src/share/classes/com/sun/jndi/cosnaming/CNCtx.java
In javax.naming.Context, Hashtable<?,?> is used for context
environment. In this changes, you use Hashtable<String, java.lang.Object>.
What do you think if we keep the type of K and V the same as
Hashtable<?,?>?
I also noticed the similar inconsistency at:
. com/sun/jndi/dns/DnsContext.java:
50 Hashtable<Object,Object> environment;
. com/sun/jndi/ldap/LdapCtx.java
226 Hashtable<String, java.lang.Object> envprops = null;
3. src/share/classes/com/sun/jndi/dns/DnsContext.java
What do you think if we have BaseNameClassPairEnumeration to
implement the NamingEnumeration, so that we can share the code of
nextElement()?
class BaseNameClassPairEnumeration implements NamingEnumeration<T>
*** com/sun/jndi/ldap/Connection.java
251 } catch (ClassNotFoundException |
252 InstantiationException |
253 InvocationTargetException |
254 IllegalAccessException e) {
I like this new try-catch feature!
4. com/sun/jndi/ldap/LdapCtx.java
1194 return (NamingEnumeration)
1195 new LdapBindingEnumeration(this, answer, name, cont);
LdapBindingEnumeration is of type NamingEnumeration, it may be not
necessary to convert to NamingEnumeration. Do you mean
NamingEnumeration<Binding>?
return (NamingEnumeration<Binding>)
new LdapBindingEnumeration(this, answer, name, cont);
2244 switch (propName) {
Do you want to indent this block? We usually indent a block even for
switch blocks.
3017 Vector<Object> temp = (Vector)extractURLs(res.errorMessage);
You may not need the conversion any more, the return value of
extractURLs() has been updated to
2564 private static Vector<String> extractURLs(String refString)
5. com/sun/jndi/ldap/LdapBindingEnumeration.java
Why it is necessary to convert the return type twice (line 92)?
92 return (LdapBindingEnumeration)(NamingEnumeration)
93 refCtx.listBindings(listArg);
It's great to use convariant return type. I would suggest add
override tag to make it easy understood.
I am only able to review a quarter of the update today, will continue
tomorrow.
Thanks,
Xuelei
