Moving from MVC to MVP

Inspired by a talk by Ray Ryan on Google IO conference (http://code.google.com/events/io/sessions/GoogleWebToolkitBestPractices.html), I decided to try out some of his ideas. One of the main reasons to switch architecture was a promise of improved unit testing. Those, who are familiar with testing of GWT code, know that in order to test code that calls GWT.create(), you have to extend GWTTestCase in unit tests. However, there are couple of reasons why nobody likes such tests:

  • They are slow. Behind the scenes GWTTestCase launches Hosted Browser. It takes about 20 second to launch a test.
  • Firing events is difficult and takes much time to write.
  • Hard to set up

For these reasons, developers try do isolate code that calls GWT.create(). Mostly it includes creation of widgets, like buttons and text boxes.

MVC version of small widget from test application (https://intranet.aqris.com/svn/repos/sandbox/aleksz/potormozim)

DollarController.java
class DollarController {

  private final Dollar view;
  private final DollarModel model;

  protected DollarController(Dollar view, DollarModel model) {
    this.view = view;
    this.model = model;
    registerHandlers();
  }

  private void registerHandlers() {
    view.getImg().addClickHandler(new ClickHandler() {
      @Override
      public void onClick(ClickEvent event) {
        model.toggleState();
      }
    });
  }
}
DollarModel.java
class DollarModel extends DynamicModel {

  private Participant participant;
  private final PartyServiceAsync partyService;

  protected DollarModel(Participant participant) {
    this(participant, ServiceLocator.partyService);
  }

  protected DollarModel(Participant participant, PartyServiceAsync partyService) {
    this.participant = participant;
    this.partyService = partyService;
  }

  public void toggleState() {
    partyService.togglePayedStatus(participant.getEncodedKey(), new ErrorHandlingAsyncCallback<Boolean>() {
      @Override
      public void onSuccess(Boolean result) {
        participant.setPayed(result);
        SystemMessaging.success("Status changed");
        fireModelChangeEvent();
      }
    });
  }

  public Participant getParticipant() {
    return participant;
  }
}
DollarView.java
public class Dollar extends Composite {

  public static final String DOLLAR_GREY_IMG = "/images/dollar_grey.png";
  public static final String DOLLAR_IMG = "/images/dollar.png";
  private Image img;
  private DollarModel model;

  public Dollar(Participant participant) {
    initWidget(getImg());
    model = new DollarModel(participant);
    new DollarController(this, model);
    ModelListener listener = new ModelListener();
    model.addListener(listener);
    listener.onModelUpdate();
  }

  protected Image getImg() {
    if (img != null) {
      return img;
    }
    img = new Image();
    img.setStyleName("dollarImg");
    return img;
  }

  private class ModelListener implements DynamicModelListener {
    @Override
    public void onModelUpdate() {
      getImg().setUrl(model.getParticipant().isPayed() ? DOLLAR_IMG : DOLLAR_GREY_IMG);
      getImg().setTitle(model.getParticipant().isPayed() ? "payed" : "not payed");
    }
  }
}

At least for me, this architecture worked quite well. It was easy and fun to write and maintain. However, unit testing of this code was quite weak. There was only one layer to test without GWTTestCase, which is the model. After writing a couple of such tests, I had a strong feeling that they are useless.

MVP version of same class

Dollar is a presenter and Participant is a model.

Dollar.java
public class Dollar {

  public static final String STATUS_CHANGED = "Status changed";
  public static final String DOLLAR_GREY_IMG = "/images/dollar_grey.png";
  public static final String DOLLAR_IMG = "/images/dollar.png";
  public static final String POSITIVE_TITLE = "Payed";
  public static final String NEGATIVE_TITLE = "Not payed";

  public interface View {
    HasClickHandlers getImg();
    void setImgUrl(String url);
    void setImgTitle(String title);
    Widget asWidget();
  }

  private final PartyServiceAsync partyService;
  private View view;
  private final Participant model;
  private SystemMessaging systemMessaging;

  public Dollar(Participant participant, View view, PartyServiceAsync partyService,
      SystemMessaging systemMessaging) {
    this.model = participant;
    this.partyService = partyService;
    this.systemMessaging = systemMessaging;
    bindDisplay(view);
    updateDisplay();
  }

  private void bindDisplay(View view) {
    this.view = view;
    this.view.getImg().addClickHandler(new ClickHandler() {
      @Override
      public void onClick(ClickEvent event) {
        toggleState();
      }
    });
  }

  public void toggleState() {
    partyService.togglePayedStatus(model.getEncodedKey(), new ErrorHandlingAsyncCallback<Boolean>(
        systemMessaging) {
      @Override
      public void onSuccess(Boolean result) {
        model.setPayed(result);
        updateDisplay();
        systemMessaging.success(STATUS_CHANGED);
      }
    });
  }

  private void updateDisplay() {
    view.setImgUrl(model.isPayed() ? DOLLAR_IMG : DOLLAR_GREY_IMG);
    view.setImgTitle(model.isPayed() ? POSITIVE_TITLE : NEGATIVE_TITLE);
  }
}
DollarView.java
public class DollarView extends Composite implements Dollar.View {

  private Image img;

  public DollarView() {
    initWidget(getImg());
  }

  @Override
  public Image getImg() {
    if (img != null) {
      return img;
    }
    img = new Image();
    img.setStyleName("dollarImg");
    return img;
  }

  @Override
  public void setImgTitle(String title) {
    getImg().setTitle(title);
  }

  @Override
  public void setImgUrl(String url) {
    getImg().setUrl(url);
  }

  @Override
  public Widget asWidget() {
    return this;
  }
}
DollarTest.java
public class DollarTest {

  private MockDollarView view;
  private PartyServiceMock partyService;
  private Participant participant;
  private Dollar dollar;
  private MockSystemMessaging systemMessaging;

  @Before
  public void init() {
    view = new MockDollarView();
    partyService = new PartyServiceMock();
    participant = new Participant("testParticipant", new Party("testParty"));
    systemMessaging = new MockSystemMessaging();
    dollar = new Dollar(participant, view, partyService, systemMessaging);
  }

  @Test
  public void toggleReturnsTrue() {
    partyService.expectTogglePayedStatus(participant.getEncodedKey()).andReturn(true);
    view.img.lastClickHandler.onClick(new MockClickEvent());
    assertEquals(Dollar.POSITIVE_TITLE, view.title);
    assertEquals(Dollar.DOLLAR_IMG, view.url);
    assertTrue(systemMessaging.success);
    assertEquals(STATUS_CHANGED, systemMessaging.text);
  }

  @Test
  public void toggleReturnsFalse() {
    partyService.expectTogglePayedStatus(participant.getEncodedKey()).andReturn(false);
    view.img.lastClickHandler.onClick(new MockClickEvent());
    assertEquals(Dollar.NEGATIVE_TITLE, view.title);
    assertEquals(Dollar.DOLLAR_GREY_IMG, view.url);
    assertTrue(systemMessaging.success);
    assertEquals(STATUS_CHANGED, systemMessaging.text);
  }

  @Test
  public void toggleStateByOtherWidget() {
    partyService.expectTogglePayedStatus(participant.getEncodedKey()).andReturn(true);
    dollar.toggleState();
    assertEquals(Dollar.POSITIVE_TITLE, view.title);
    assertEquals(Dollar.DOLLAR_IMG, view.url);
    assertTrue(systemMessaging.success);
    assertEquals(STATUS_CHANGED, systemMessaging.text);
  }

  @Test
  public void viewInitializedOnConstructorCall() {
    assertEquals(Dollar.NEGATIVE_TITLE, view.title);
    assertEquals(Dollar.DOLLAR_GREY_IMG, view.url);
  }
}

View is the only one untested part. It is possible to cover it with tests using GWTTestCase, however, it seemed to me that there is no need in doing this. Code in view is usually extremely primitive and I would prefer to test it with Selenium test.

MockDollarView.java
public class MockDollarView implements Dollar.View {

  MockHasClickHandlers img = new MockHasClickHandlers();
  String title;
  String url;

  @Override
  public HasClickHandlers getImg() {
    return img;
  }

  @Override
  public void setImgTitle(String title) {
    this.title = title;
  }

  @Override
  public void setImgUrl(String url) {
    this.url = url;
  }

  @Override
  public Widget asWidget() {
    return null;
  }
}

Results

For me, difference between two implementation was quite significant. I was able to cover about 90% of GWT code with fast unit tests. Including browser navigation and history. Writing tests was easy and has proved to be very useful. In more complex widgets, MVP was easier than MVC. I was able to delete almost all custom events.

Labels

mvc mvc Delete
mvp mvp Delete
gwt gwt Delete
google google Delete
architecture architecture Delete
practice practice Delete
best best Delete
Enter labels to add to this page:
Please wait 
Looking for a label? Just start typing.

Comments (4)

 

  1. Sep 18, 2009

    Anonymous says:

    Noticed one issue regarding View (or Display) interfaces. Ray Ryan advices to us...

    Noticed one issue regarding View (or Display) interfaces. Ray Ryan advices to use interfaces like HasValue and HasText to interact with your view. However, when I had to make different implementations of View (edit mode, read only mode), I noticed that using these interfaces is not very convenient.

    For example, you have a DatePicker in edit mode. View interface declares link as HasValue<Date>. Now you start implementing read only view and you want to use a Label instead of DatePicker. You will be quite disappointed about the fact that Label does not have HasValue interface.

    Solution is very simple - use getters and setters in View interface (void setDate(Date date); Date getDate(). It will also make your mock implementation a bit more simpler.

    P.S. This comment applies only to retrieving/setting values. Handler interfaces still do their job

  2. Feb 25

    Anonymous says:

    I wonder if one should use Model View Presenter (or actually any other GUI archi...

    I wonder if one should use Model View Presenter (or actually any other GUI architecture pattern) for widgets? I mean, in every single MVP example (like Ray's talk one) MVP is used for views (or screens) that is compositions of some widgets. So basically view interface shouldn't talk in terms of domain model, but rather use simple types like String, Integer or Date. There was no example about architecting complex widgets that use complex types. I agree that simple widgets (like built-in Button) are best implemented within just single class, but what about really complex ones? For example, I'm now implementing quite complex widget (some kind of spreadsheet) and I wonder which pattern to use. I feel MVP would be great, but I would be able to use it just like any other widget, so just creating it like this: SimpleSpreadsheet mySimpleSpreadsheet = new SimpleSpreadsheet(someDataSource). Using classic MVP approach I have to create Presenter class with its dependencies, get view from presenter and add this view to widget container.

    1. Feb 25

      Ürgo Ringo says:

      I prefer to implement complex widgets also using their own MVP. However, the cod...

      I prefer to implement complex widgets also using their own MVP. However, the code that uses these widgets should know as little as possible about the existence of Model and Presenter. All access should be done via the View. This means also that View needs to initialize the Presenter and Model. I suppose that there are cases where Model may need to be exposes and provided to the View in constructor.

      So In my opinion for client code it shouldn't be any different if SimpleSpreadsheet internally uses MVP or something else.

      1. Feb 25

        Anonymous says:

        Urgo, thanks for your answer. It's very helpful. I totally agree with you. So, p...

        Urgo, thanks for your answer. It's very helpful. I totally agree with you. So, probably the best solution is to create package-private presenter and instantiate it within the View's constructor, passing reference to itself, since a presenter needs to call view. Moreover, model-related dependencies (like DataSource in my example) should be essentially passed to the presenter through the View's constructor (I can't see other solution here). This implementation effectively hides any GUI architecture internals. For now I think that above strategy would be good for widgets development, but I still prefer presenter-in-front approach for screens/views. However, distinction here is probably quite subtle due to fact that view/screen is essentially a widget . It would be really cool to have one unified, application-wide approach here.