Hi ! Stefan has added some TODO in the LdapConnection interface :
// TODO: all the SASL bind methods are not declared in this interface, but implemented in LdapNetworkConnection. Is that intended? // TODO: why does connect() return a boolean? What is the difference between false and an Exception? // TODO: think about usage of abbrevisions (Dn/Rdn) vs. spelled out (relative distinguished name) in javadoc // TODO: describe better which type of LdapException are thrown in which case? // TODO: remove the "we" language in javadoc // TODO: does method getCodecService() belong into the interface? It returns a LdapApiService, should it be renamed? // TODO: does method doesFutureExistFor() belong into the interface? Move to LdapAsyncConnection? I have fixed some of them : - Replaced teh Dn/Rdn abbreviations by the full name - Removed the 'we' all over the javadoc - Replaced the doesFutureExistFor() method by a more explicit isRequestCompleted() method. It's not a pure async method, it's also useful when one want to abandon a request that takes too long. - The connect() method returns a boolean to be able to distinguish between a problem while establishing a connection, and a simple timeout because teh remote host does not answer. We do have a retry mechanism in this case, and when we reach the number of possible retries, we simply return 'false'. Of course, when we have some more problematic problem, an exception is thrown. Regarding the few oher TODOs : - We have to add those SASL methods - LDAPException : yes, it would be useful to list specific excpetions instead of generic ones - getCodecService() : this is a problem. the LdapApiService means nothing for those who haven't wrote the API? CodecService is slightly more significant. This method returns the container that exposes the supported codec and extended operation. Renaming it to getServerFeatures() would be better, but we should also rename the LdapApiService (something like ServerFeatures would probably be clearer ?) Thoughts ? -- Regards, Cordialement, Emmanuel Lécharny www.iktek.com
