Search code examples
javadesign-patternsgwtmvp

Best way to implement MVP


In the MVP pattern, there is no dependency between the view and the model. Everything is done by the presenter.

In our GWT-project, we implement all our MVP classes like shown on the GWT .page. To access the view, our presenter uses an interface like this:

public interface Display {
    HasClickHandlers getAddButton();
    HasClickHandlers getDeleteButton();
    ...
}

So, the presenter gets a button from the view, and applies its click handlers to it.

But this feels so wrong to me. This violates the single responsibility principle, as well as the information hiding principle. the presenter should not know anything of the internals of the view (even when we get an abstract HasClickHandlers).

In my opinion, we should better use this kind of pattern:

public interface Display {
    void setPresenter(Presenter p);
}

public interface Presenter {
    void onAdd();
    void onDelete();
}

So, the view tells the presenter actively, that some interaction occured. But not on which view-element. My team partners argue, that with the first solution, we avoid circular dependencies. That´s right. But anyway, I would prefer the second way because the modules are better separated and can be maintained independently.

What are the advantages / disadvantages?


Solution

  • Strongly agree - and the article you link describes both sides, if you continue to the second section.

    I don't build my Views exposing widgets or their HasSomethingHandlers - I find it generally better to build the Presenter API to expose those possible interactions for several reasons:

    • Unit tests - It is simpler to make a mock view which invokes methods rather than firing events. This is not to say that it isn't possible (especially if your Presenter can run on the JVM and you can create a Proxy type for each one.

    • Cleaning up unused handlers - Presenters are generally cheap and get re-created each time, but sometimes Views are heavy and are saved, reused. Any time you reuse a view, you have to be sure that all of those externally added handlers were removed. This can be done correctly with some "lifecycle" methods like onStop() or something on the presenter, so that whatever code replaces presenters can shut it down, but would need to be factored in.

    • Multiple view implementations - like unit tests, different view implementations (mobile vs desktop, readonly vs readwrite, etc) might have different ways of causing the same change. Now, you could have multiple methods exposing HasClickHandlers onAddClicked() and HasTouchHandlers onAddTapped(), or, take the approach you describe. (You also can end up leaking some UX details into the presenter, like hitting the Enter key while in the last field, as opposed to clicking the button - that arguably belongs in the view, and not in the presenter).

    One downside of this approach: the view needs more brains, and part of the goal of MVP is to keep the view as skinny as possible.

    Finally, while this alternative approach isn't actually compared and contrasted on the page you linked, page two of the same article you linked does show your approach instead http://www.gwtproject.org/articles/mvp-architecture-2.html:

    Now that we have the UI constructed, we need to hook up the associated UI events, namely Add/Delete button clicks, and interaction with the contact list FlexTable. This is where we’ll start to notice significant changes in the overall layout of our application design. Mainly due to the fact that we want to link methods within the view to UI interactions via the UiHandler annotation. The first major change is that we want our ContactsPresenter to implement a Presenter interface that allows our ContactsView to callback into the presenter when it receives a click, select or other event. The Presenter interface defines the following:

    public interface Presenter<T> {
      void onAddButtonClicked();
      void onDeleteButtonClicked();
      void onItemClicked(T clickedItem);
      void onItemSelected(T selectedItem);
    }
    

    This is done in the context of describing the UiBinder way of building views, but it applies equally well without UiBinder.

    So when discussing the article in relation to organizing how your team is building their application, be sure to consider the entire set of articles - those articles are both referenced from http://www.gwtproject.org/doc/latest/DevGuideMvpActivitiesAndPlaces.html, which also assumes this non-eventhandler approach. And in that article there are other "wrong" ways of doing things, like manual dependency injection, instead of an automated reliable tool like Gin or Dagger2.

    The articles aren't meant to describe the One True Way, but to understand the ideas and patterns that can be used. Don't cargo cult this, applying patterns blindly, but make sure to understand the pros and cons - and work as a team, a pattern used inconsistently is arguably worse than no pattern at all!


    EDIT: I realized I didn't mention the circular dependency issue!

    This is as much or as little of an issue as you want it to be. If you create them at the same time, and pass references to the other in the constructors, obviously you have an issue - GWT can't proxy them (and arguably that's a bad idea anyway). But, down the road, you might get into a situation where views are expensive to build and could be pooled/cached and not re-created every time, in which case the view needs to be informed of the new presenter it now works with anyway.

    That then requires that the View interface supports a void setPresenter(P) method anyway, so our circle is broken. Keep in mind also that this approach will require either the presenter being sure to clear out data in the view, or the view knowing when a new presenter is set to clear its own data.

    My personal approach for this leads to the presenter owning a view field, injected (probably via constructor) when it is created, and have a start method in the presenter. When called, the presenter takes control of the view, and when ready, appends it where it belongs in the dom hierarchy.

    Yes, this requires one extra method in the view, but if you have any base class for your views, this will be trivial, and if you end up doing any view reuse, you need that method anyway.