Paper DX between Porlet-driven and Interface driven approaches

[ujoni]

Gather preference data using paper examples of Vaadin Portlet API. Provide two examples implemented with both Portlet-driven and interface-driven styles. The examples should highlight the portlet API with as little business logic as possible. But the operation context should still be clear. The potential user looking at the example should be able to understand what is the use-case being presented, even if the business logic is missing.

Examples:

• Build an example on window state driven portlet application. Normal mode displays an image, maximized displays a form.
• Build an example based on the address book demo. Both portlets (contact list and contact information) should be displayed. Highlight IPC and portlet mode (for contact information portlet, for editing).

Portlet-driven:

• Produce an Vaadin portlet API proposal from a draft #25

Interface-driven:

• Demonstrate window state handling in Vaadin portlets #27
• Address book demonstration version 2.0 #29

[mehdi-vaadin]

We asked the opinions of developers about different API designs of how to get informed of window state change. The three options are the following.

public class MyComponent extends Div implements WindowStateHandler {
    @override
    public void windowStateChange(WindowState newState) {
    }
}

public class MyComponent extends Div implements VaadinPortletSession.PortletListener {
    @override
    public void handleActionRequest(ActionRequest request, ActionResponse response, UI ui) {
    }

    @override
    public void handleRenderRequest(RenderRequest request, RenderResponse response, UI ui) {
    }

    @override
    public void handleEventRequest(EventRequest request, EventResponse response, UI ui) {
    }

    @override
    public void handleResourceRequest(ResourceRequest request, ResourceResponse response, UI ui) {
        if(request.getWindowState().equals(WindowState.NORMAL)) {
        }
    }
}

public class MyComponent extends Div implements VaadinPortletView {
    @override
    public void init(VaadinPortletContext context) {
        context.addWindowStateChangeHandler(newState -> {
            if(newState.equals(WindowState.NORMAL)) {
            }
        });
    }
}

We asked the opinions of 8 developers. Six people chose the first option and two chose the second. However, two people suggested having an interface including windowStateChange and other similar methods that might be needed e.g. portletModeChange with default empty implementations. Something like the following.

public interface PortletListener {
    default void windowStateChange(String newState) {
    }

    default void portletModeChange(String newMode) {
    }
}

[mehdi-vaadin]

Adding other required methods to the PortletListener interface from the previous comment to also support IPC, it will look like the following. I also renamed it to PortletHandler.

public interface PortletHandler {
    default void windowStateChange(WindowState newState) {
    }

    default void portletModeChange(PortletMode newMode) {
    }

    default void parametersChange(Map<String, String> newParameters) {
    }

    default void portletEventFired(EventObject event) {
    }

    default WindowState getWindowState() {
        return ((P) VaadinPortlet.getCurrent()).getWindowState();
    }

    default PortletMode getPortletMode() {
        return ((P) VaadinPortlet.getCurrent()).getPortletMode();
    }

    default Map<String, String> getParameters() {
        return ((P) VaadinPortlet.getCurrent()).getParameters();
    }
}

[tmattsso]

default void windowStateChange(String newState) {
}

default void portletModeChange(String newMode) {
}

Why string, when you have a WindowState and PortletMode enum? :)

[mehdi-vaadin]

@tmattsso thanks for your comment. I edited my comment. WindowState and PortletMode are not enums though. They are classes.

[caalador]

Only thing with the have all features under one class is that we would always need to enable things that all the things need even if not necessary which might cause unnecessary overhead.

[pleku]

all the things need even if not necessary which might cause unnecessary overhead.

What type of overhead ? Extra roundtrips ? Or ?

[caalador]

Creating un-needed portal urls (which will need some book keeping).
Running extra JS for registering things and probably some book keeping.

[pleku]

Ok. So if we are certain that those are an issue and not premature optimization, what should we do to remedy the situation ? Split into more interfaces ?

As long as the API stays the same, we can introduce more interfaces later on without breaking things.

[caalador]

Well if this suggestion is to remedy the findability then we should go with the context object where you use methods to add handlers as then they would be findable, but we wouldn't have to do anything if nothing is requested from the context.
It just has the problem of easily makin the code hart to read.

[ujoni]

In regards to those modes, I kinda have the fear that a user might write something like:

public class Something implements PortletListener {
    private WindowState currentState;
    private PortletMode currentMode;

   void windowStateChange(WindowState newState) {
        currentState = newState;
        refresh();
    }

   void portletModeChange(PortletMode newMode) {
        currentMode = newMode;
        refresh();
    }

    private void refresh() {
        if (/* repeat for some permutations of mode and state */) {

        }
    }
}

Those values are (currently) available through VaadinPortlet.getCurrent().getWindowState() and whatnot. But they might not realize that. So I am somewhat wondering whether we should just collapse that thing into a cleaned RequestContext from which the user has access to everything. But neatly

[ujoni]

Closing as the decision has been made and we have moved forward with the "Separate handler interfaces for each status change" approach.

A new DX should be conducted was prototype is working