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
// 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 ?

Emmanuel L├ęcharny

Reply via email to