+1 for going the Closeable way.
Close operation can end up with an error (e.g. timeout). That cannot be
avoided. And for me it is much easier (and much less error-prone) to
handle a checked exception than to do a complex sequence of close() and
then check for isClosed() or isConnected() and similar and then trying
to figure out what really was the root cause because the original
exception got eaten somewhere deep in the code. Having close() that
throws IOException will be a nudge to all the clients to do at least
some error handling.
Of course, the problem is that it is IOException and not LdapException.
One of the solution would be to make LdapException subclass of
IOException? That obviously does not fit 100% well, as some subclasses
of LdapException and more related to parsing than to IO. But I think
that it is still somehow acceptable. Another (and perhaps cleaner) would
be to throw IO exceptions that happen during the close directly as IO
exceptions without wrapping them to LdapException. And that might be the
right thing to also for all other operations: if the operation dies on
network condition (e.g. TCP timeout), throw original IO exception. If it
dies on LDAP condition (e.g. LDAP error code 49) throw LdapException.
That would solve the connection being Closeable. Now for cursor there is
still this tricky part that the close() on cursor might want to abort an
operation and therefore it may produce LDAP errors. So maybe the best
thing to do would to do both: make LdapException subclass of IOException
and pass low-level IO exceptions without wrapping them to LdapException.
This is how I would do it. But maybe the API has some other philosophy
when dealing with errors? If there is one, I must have missed it so I
will be very grateful for anyone to point my nose in the right
direction. However, I really think one of the big drawbacks of the API
is error handling in general. Especially in the schema processing part,
but there are obviously also other parts that could use some
improvement. We obviously cannot change everything in a day. So
gradually improving the status quo seems to be a good way to take.
--
Radovan Semancik
Software Architect
evolveum.com
On 01/29/2016 10:53 AM, Emmanuel Lécharny wrote:
Le 29/01/16 10:06, Maxim Solodovnik a écrit :
sorry for cross-posting, but I have no response in user list
Hopefully this list will fit better :)
Sorry for having been mute :/
Implementing Closeable was suggest in december 2011 by Stefan.
This have a huge impact on the code, as the current close() method does
not throw any exception atm, and we will have to take care of the
IOException all over. OTOH, this is not such a problem.
guys, wdyt ?
