Hi,

On Thu, Jul 3, 2014 at 4:50 AM, Alexander Klimetschek
<aklim...@adobe.com> wrote:
> On 03.07.2014, at 09:12, Konrad Windszus <konra...@gmx.de> wrote:
>
>> - The client can decide how to expose that error (whether just logging is 
>> fine or something more obvious). This cannot be achieved by just setting up 
>> a utility method, because that one does only see the null return value and 
>> not the original reason for that.
>
> Yes, but my question is whether there is any need to pass through the 
> exception at all.
>
>> - Tracing problems during development is much easier (instead of looking at 
>> the log I can see the full exception)
>
> You can debug exceptions inside the adapterfactories as well (after seeing 
> them in the log).
>
>> - It allows to use it for something like validation in Sling Models
>
> How would that work? (I saw the reference earlier in the thread, but I don't 
> know how you'd use adaptTo for validation and can't really imagine it's a 
> good fit)

Validation means a lot of things; I would say that your example below
where a resource has to be a certain type to be adapted to a resource
collection is a form of validation. As you note, using exceptions for
validation use cases is wrong.

>
>> - It is less error-prone to the developers (you can easily forget to either 
>> use the utility method or check for null)
>
> The null-returning method is out there, it cannot be changed to throw a 
> checked exception (which is the only way to force handling for devs)
>
>> - In my regard in most of the cases if adaptation fails, there is something 
>> wrong with the deployment (required bundles are not installed, components 
>> are not running, ….) and I cannot reasonably work around that issue in the 
>> code -> that calls for an exception
>
> It's not only exception, it's also a way to check if something is of a 
> certain type (say adapting a resource to a resourcecollection). In this case 
> an exception is not the right thing.
>
> I guess it would make sense to have adapterfactories et. al. to work like 
> this:
> a) if it is not of the desired type, i.e. cannot semantically be adapted, 
> return null
> b) if it fails due to some actual exception, throw a runtimexception
>
> But not sure if that will work.

It won't work :) This is a hugely non-backwards compatible change. It
happens to be binary compatible, but it is not semantically compatible
(which is in some ways just as important). Callers of adaptTo() assume
(because we have told them to assume) that null will be returned if
the adaptation can't be done. We can't now start throwing exceptions.
Callers won't expect this.

The caller *must* indicate in some way that she wants behavior which
is different than the current. AFAICT, there are only two viable
solutions:

* My original suggestion of using a Result interface. This requires
more verbose code on the caller side -- the caller needs to check a
success flag -- but allows for fine-grained information (which would
be appropriate for a validation use case).
* Bertrand's suggestion of using some kind of ThreadLocal. Less
verbose code on the caller side. Would seem to violate Effective Java
#57 in that it is using exceptions for control flow.

Both cases can be implemented without impacting existing callers or
adapter factories. Adapter factories would need indicate that they
support the Result interface or exception throwing via a service
property. For factories without the property, the AdapterManagerImpl
would simulate the proper behavior.

Justin

>
> Cheers,
> Alex

Reply via email to