Hi Emmanuel,
Long time no hear!
I think this is a good idea. It's what we chose to do in the OpenDS SDK
and I think that it makes the API much more usable in practice.
Application code is leaner and less error prone since there is no need
to check (or forget to check!) the result code after each operation.
Instead all error handling can be performed in a single catch block at
the end.
Something else we did was to also create several subclasses of our
ErrorResultException class in order to make it easier to isolate common
failure reasons, e.g. connection failure, authn/authz failure, referral,
timeout, etc:
http://www.opends.org/daily-builds/sdk/latest/OpenDS_SDK/doc/org/opends/sdk/ErrorResultException.html
I think that this is similar to JNDI. I didn't shoot for a 1:1 mapping
between result codes and exception types since this would lead to very
many exception classes which I thought would be a bit excessive. It's a
trade-off, and I don't know if I set the bar too low or too high. The
good thing is that it is possible to add more sub-classes later without
breaking compatibility, so I erred on the low side probably. Since all
of these exceptions expose the underlying result, it still possible to
do a catch-all on ErrorResultException and still have logic based on the
ResultCode.
Also note that you will need to make LdapException a sub-class of
j.u.c.ExecutionException for it to be thrown by Future.get (or make it a
runtime exception but I think that this is a bad idea). This is a bit
annoying, but in practice not a big deal (it just looks surprising
seeing java.util.concurrent in the class hierarchy for a result exception).
Another API problem I ran into was what to do with the "checked"
InterruptedException which can be thrown from blocking operations such
as Future.get. I could have chosen to catch it and rethrow it as a
cancelled result exception (or a new exception like
InterruptedErrorResultException). This would avoid having to catch/throw
it every time, as this example illustrates:
Connection connection = ...;
Entry entry = ...;
try
{
connection.add(entry);
}
catch (ErrorResultException e)
{
// Handle operation failure.
}
catch (InterruptedException e)
{
// Grrr... Handle thread interrupt
// This would not be needed if I caught and re-threw
// the exception as a sub-type of ErrorResultException.
}
I played it safe and kept it separate since InterruptedException has a
very specific contract, so hiding it inside an ErrorResultException (or
LdapException in your case) might cause it to get overlooked. Brian
Goetz talks about it here:
http://www.ibm.com/developerworks/java/library/j-jtp05236.html
Having said that, every time I look at our APIs and see the "throws
InterruptedException" I have to restrain myself as I am still very
tempted to catch and rethrow as a sub-type of ErrorResultException! :-)
Matt
On 19/07/10 16:49, Emmanuel Lecharny wrote:
Hi,
long time, no see :)
I have a proposal about the way we handle the operation results.
Currently, every operation is returning a Response, and if we want to
know if the operation has been successful, we have to check the
LdapResult field. This is not very convenient.
Assuming that when we have issued a synchronous operation, we are
waiting until we get a response, why can't we have those operations
throwing a LdapException ?
For async operations, we can also make that the XXXFuture.get() method
throw the same exception.
thoughts ?
