directory-api mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lecharny <elecha...@gmail.com>
Subject Re: About operation result
Date Mon, 19 Jul 2010 17:18:25 GMT
  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/%3C4B9A1FE4.4040909@gmail.com%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


Mime
View raw message