Understanding Adapters.java about xiliary HOT 4 CLOSED

Indeed the "else if" assumption was buggy. Thanks for pointing that out. I have added an appropriate test and changed the implementation accordingly.

Regarding the unloaded adapter I usually would agree with your proposal. But there is something else to respect here. The behavior of the workbench's DeferredTreeContentManager relies on org.eclipse.ui.internal.util.Util#getAdapter, which the Adapters class mimics (admittedly not very well as it turned out...).

I prefered not to introduce a dependency to workbench internals, but to implement an own utility that provides the same behavior (in a typesafe manner) instead. This was a choice between two bad options. Which is also why the Assert#isNotNull parameter check for example is done instead of ignoring null silently like your implementation does.

But the important point here is that Util#getAdapter omits loading of an adapter, which might be to avoid starting bundles unintendedly[1]. And this is why I keep the original behavior in place, which is of course arguable from a general point of view.

After completing the fix the Util's PlatformObject check in case adaptable returns null suddenly looked reasonable too. This avoids redundant registry lookups, which the current Adapters version does also now. So hopefully the current version is safe to use as a replacement for the workbench's Util behavior.

Having said this I will close this issue as resolved. But feel free to reopen if you disagree ;-)

[1]: JavaDoc excerpt of IAdaperManager#loadAdapter(Object,String):
Note that unlike the getAdapter methods, this method
will cause the plug-in that contributes the adapter factory to be loaded
if necessary. As such, this method should be used judiciously, in order
to avoid unnecessary plug-in activations. Most clients should avoid
activation by using getAdapter instead.

from xiliary.

if( !( adaptable instanceof PlatformObject ) ) { return getAdapterFromRegistry( adaptable, adapterType ); } return null;

I see, you assume the registry lookup is already done because it is a platform object? Clever but a little tricky because implementations might not go to the registry?

But the important point here is that Util#getAdapter omits loading of an adapter, which might be to avoid starting bundles unintendedly[1]. And this is why I keep the original behavior in place, which is of course arguable from a general point of view.

Yes that is lovely discussion :D. I am favor of loading because:

• The plug-in developer did not make the adapter factory by accident.
• The user did not install the plug-in by accident.
• The developer is not in charge of activating the plug-in
• The user is not aware of this technical detail
• The user is not capable of starting the plug-in manually
• Starting a bundle is a few millisecs in 99% of the cases

This is why I do the checking and the subsequent loading but I agree, it is a matter of perspective.

from xiliary.

I tend to go with your argumentation regarding the loading topic. But as I said, I wanted to keep the behavior as it is given by the workbench's utility class. This is because I thought it might be even less understandable if similar UI components sometimes trigger plug-in activation and sometimes they don't.

Regarding the PlatformObject: I do not like this check very much, but this is also as the Util implementation behaves. I guess as the only purpose of the PlatformObject is to ask the registry for the adapter, it is assumed very unlikely that someone overrides the PlatfromObject#getAdapter(Class) implementation, or at least is clever enough to add the super call if doing so. In case the super call is omitted and no adapter is returned, this implementation prevents of course the registry lookup completely.

from xiliary.

Cool! Thanks again.

from xiliary.