We could solve that by defining a specific exception for adaptation-not-possible and then catch only that.
Of course that would leak tons of exceptions from code written before that exception became available. Maybe do the catching based on some sort of version clue? Cheers, Jeff. On 01/07/2014 09:40, "Konrad Windszus" <konra...@gmx.de> wrote: >It is not (only) about throwing exceptions in case no suitable adapter is >available. It rather is about the fact, that today the adaptTo is a >barrier for all kinds of exceptions. In some cases the adaptation fails >for a specific reason (one example is Sling Models where injection fails, >another one is the issue mentioned in >https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not supporting >primitives)). Both would be valid use cases where the client would be >interested to catch the exception here. > >On 01 Jul 2014, at 10:34, Carsten Ziegeler <cziege...@apache.org> wrote: > >> Adding a new interface would require us to implement it all over the >>place >> and as Felix points out, client code would always need to check whether >>the >> new interface is implemented or not Having to methods, like hasAdapter >>and >> adaptOrThrow does not work very well as between the two calls things >>might >> have changed already: while hasAdapter returns true, the underlying >>factory >> gets unregistered before adaptOrThrow is called. >> In many use cases, the current pattern works fine - the client does not >> care whether an exception is thrown within the adaption - it just cares >> whether an object is returned or not. And there are valid use cases, >>where >> client code does different things whether the adaption works or not >>(e.g. >> the post servlet checks for adaptTo(Node) and then does additional >>things >> if the resource is backed up by a node.) >> >> I see the point that there are also use cases where it would be fine to >> simpy throw an exception if adaptTo is not successful. This would make >>the >> client code easier. However as this most properly is a runtime >>exception, >> client code can today just call a method on the object and end up with a >> NPE - having the same result :) >> >> This leaves us with use cases where the client code explicitely wants to >> catch the exception and then do something depending on the exception. >>Maybe >> we should just add something for this explicit use case instead of >>bloating >> the general adaptTo mechanism? >> >> Regards >> Carsten >> >> >> >> >> 2014-07-01 9:44 GMT+02:00 Konrad Windszus <konra...@gmx.de>: >> >>> Regarding 1) Having such a Result class would mean that all consumer >>>would >>> need to unwrap the exception first. So instead of being forced of >>> implementing a null-check (as with the old solution) one would need to >>> implement another check. I want to prevent such a burden to the >>>consumers. >>> Regarding 2) Since the client code knows on which object the >>> adaptToOrThrow is called I don¹t get why an instanceof check is >>>necessary. >>> Whether this object implements Adaptable2 is known at compile-time, so >>>you >>> do have the full IDE-support here. >>> Regarding 3) In that case I would no longer allow a null value to be >>> returned. One drawback is, that all the null checks are no longer >>>effective. >>> >>> IMHO solution 2) is the best. At the same time I would deprecate the >>>old >>> Adaptable, because I cannot come up with a real use-case where >>>returning >>> has an advantage. I would rather implement another method boolean >>> hasAdapter(Class<AdapterType> type) on the Adaptable2 interface. >>> Regards, >>> Konrad >>> >>> On 01 Jul 2014, at 09:07, Felix Meschberger <fmesc...@adobe.com> wrote: >>> >>>> Hi >>>> >>>> There currently are two issues floating around dealing with the >>>>question >>> of returning more information than just null from the >>> Adaptable.adaptTo(Class) method: >>> https://issues.apache.org/jira/browse/SLING-3714 and >>> https://issues.apache.org/jira/browse/SLING-3709. I think these >>>requests >>> warrant some discussion on the list. >>>> >>>> Background: adaptTo can be implemented by Adaptable implementations >>> themselves or, by extending from SlingAdaptable, they may defer to an >>> AdapterMananager. AdapterManager itself is extended by AdapterFactory >>> services. All these interfaces define an adaptTo method. All these >>>methods >>> return null if adaption is not possible and don't declare or document >>>to >>> throw an exception. >>>> >>>> While not explicitly documented as such, the intention is and was that >>> adaptTo never throws on the grounds that adaption may fail which is >>> considered a valid result and thus exceptions are not to be expected >>>and >>> handled. >>>> >>>> Hence all implementations of the methods generally >>> catch-and-log-but-don't-throw. Interestingly SlingAdaptable.adaptTo and >>> AdapterManagerImpl.getAdapter don't catch ‹ so any RuntimeException >>>thrown >>> from an AdapterFactory would be forwarded. >>>> >>>> Having said this there are options available: >>>> >>>> (1) Add support for a new Result<?> class. We would probably implement >>> this in the AdapterManager.getAdapter implementation explicitly >>>handling >>> this case because it would entail catching the adaptTo/getAdapter >>>calls to >>> get the exception (the Result.getError should return Throwable >>>probably not >>> Error) >>>> >>>> Use would be limited to new AdapterFactory implementations throwing >>> RuntimeExcpetion. For Sling Models this would be the case. >>>> >>>> (2) Add a new adaptToOrThrow method, which is declared to throw a >>> RuntimeException and never return null: Either it can adapt or it >>>throws. >>> This would require a new interface Adaptable2 (probably) to not break >>> existing Adaptable implementations. The SlingAdaptable base class would >>> implement the new method of course, probably something like this: >>>> >>>>> SlingAdaptable implements Adaptable2 { >>>>> Š >>>>> public <AdapterType> AdapterType adaptToOrThrow(Class<AdapterType> >>> type) { >>>>> AdapterType result = this.adaptTo(type); >>>>> if (result != null) { >>>>> return result; >>>>> } >>>>> throw new CannotAdaptException(Š); >>>>> } >>>>> } >>>>> >>>> >>>> Use is problematic because you would have to know whether you can call >>> the new method: So instead of an null check you now have an instanceof >>> check Š Except for the Resource interface which would be extended to >>>extend >>> from Adaptable2 as well. >>>> >>>> (3) Document, that Adaptable.adaptTo may throw a RuntimeException. >>>> >>>> The problem here is, that this may conceptually break existing callers >>> of Adaptable.adaptTo which don't expect an exception at all ‹ >>>presumably >>> this is a minor nuisance because technically a RuntimeException may >>>always >>> be thrown. >>>> >>>> Regards >>>> Felix >>> >>> >> >> >> -- >> Carsten Ziegeler >> Adobe Research Switzerland >> cziege...@apache.org >
