On 7/19/10 7:02 PM, Matthew Swift wrote:
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.
Absolutly. If only you use the API for a moment, it becomes obvious that
it's the right way to go.
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.
We have done the same thing a while back :
http://mail-archives.apache.org/mod_mbox/directory-api/201003.mbox/ajax/%[email protected]%3e
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.
+1
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
