uDIG
  1. uDIG
  2. UDIG-1635

Box should implement IAdaptable

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: UDIG 1.2.0
    • Fix Version/s: UDIG 1.2.0
    • Component/s: framework, printing
    • Labels:
      None

      Description

      It seems the definition of the Box class is a bit inconsistent; the class is primarily used to define a rectangle of page real estate - and delegates all interesting functionality to an internal BoxPrinter. It is however an Element in the EMF sense and persists its state as part of the project file.

      Stephen writes on email:

      Hi List,
      This is Stephen here and have been studying the printing plug-in source for sometime.
      While going through the source code of Box and BoxImpl in the 'net.refractions.udig.printing.model' project, I noticed that the BoxImpl class provides an implementation for the 'getAdapter()' method, even though the IAdaptable interface is nowhere present in its nor in the heirarchy of the Box interface.

      Also the BoxPart (package: net.refractions.udig.printing.ui.internal.editor.parts) class in its implementation of getAdapter() checks if the Box object it references is an instance of type 'IAdaptable', (before it forwards the request to getAdaptable() of the Box object) but as this interface does not exist in the type hierarchy of the Box interface and the BoxImpl class, the check is always going to fail, so the request is never forwarded to the 'BoxPrinter' object as it seems to be the intent.

      So what I would like to know is, if I want the getAdapter() request to be forwarded from the BoxElement to my Custom BoxPrinter objects, what exactly should I do, add IAdaptable to type heirarchy of Box or directly to BoxImpl or just remove the 'instanceof' IAdaptable check in the getAdapter() method of BoxPart ???

      After a bit of discussion:

      Well Jody, from what I can gather it seems the main intention in the getAdapter() implementation of BoxImpl class is to just forward the call to the getAdapter() method of the BoxPrinter object so that when we implement our own custom components using the boxPrinter extension point, any call on getAdapter() on the Box object or BoxPart object would be directly transferred to our custom BoxPrinter extension which we have implemented.
      That is:-
      BoxPart.getAdapter() -> Box.getAdapter() -> CustomBoxPrinter.getAdapter()

      So that the implementer of the extension can decide to what object his UI container BoxPart can adapt to, by just specifying that behaviour in the getAdapter() of the BoxPrinter object.
      Now if thats what the developers had intended, then just adding 'IAdaptable' to the list of interfaces extended by 'Box' (as you mentioned below) should be the only fix that is needed

      End of the day:

      • add IAdaptable to the Box hierarchy that seems the most consistent

        Activity

        Hide
        Jody Garnett added a comment -
        Okay as anticipated this was a one line fix; however implementing IAdatable is not without its tricks ...

        The final fix calls AdapterManager correctly...

            public Object getAdapter( Class adapter ) {
                Object obj = Platform.getAdapterManager().getAdapter(this, adapter);
                if (obj != null) {
                    return obj; // an explicit adapter factory for box was defined
                }
                BoxPrinter printer = getBoxPrinter();
                if (printer != null) {
                    return printer.getAdapter(adapter);
                }
                return null;
            }

        And I had to go into all the BoxPrinter implementations and ensure they do the same. Note that the AdapterManager is called for "this" prior to delegating to the internal BoxPrinter.
        Show
        Jody Garnett added a comment - Okay as anticipated this was a one line fix; however implementing IAdatable is not without its tricks ... The final fix calls AdapterManager correctly...     public Object getAdapter( Class adapter ) {         Object obj = Platform.getAdapterManager().getAdapter(this, adapter);         if (obj != null) {             return obj; // an explicit adapter factory for box was defined         }         BoxPrinter printer = getBoxPrinter();         if (printer != null) {             return printer.getAdapter(adapter);         }         return null;     } And I had to go into all the BoxPrinter implementations and ensure they do the same. Note that the AdapterManager is called for "this" prior to delegating to the internal BoxPrinter.
        Hide
        Jody Garnett added a comment -
        Fixed as of -r31637
        Show
        Jody Garnett added a comment - Fixed as of -r31637

          People

          • Assignee:
            Jody Garnett
            Reporter:
            Jody Garnett
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: