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
