MVP injection best practices

26 views
Skip to first unread message

Arthur Kalmenson

unread,
Aug 17, 2009, 12:26:04 PM8/17/09
to google-guice
I'm not sure if this is the best place to ask, but the question is
about dependency injection in general. We're currently using GWT for
our UI and GIN for the dependency injection. We're also trying to
follow the MVP pattern as suggested by Ray Ryan at his Google I/O
presentation. We're running into issues when one UI component contains
another, smaller, component.

For example, if we have a Contact model, ContactView UI element and
ContactPresenter. The Contact model contains an Email model. Assume
the UI represents the model object graph, so ContactView has EmailView
that it arranges alone side its other elements. If ContactView has
EmailView injected, we get into a dilemma where we don't have the
Presenter instantiated. While you could make EmailView a singleton, it
doesn't work if you have a List of EmailViews. There would be no way
to tie it to the appropriate Presenter.

The solution we've come up with is to tie the Presenters together and
have ContactView have a setter for EmailView. However, the
disadvantage is ContactView is no longer having the EmailView injected
with GIN/Guice, but by hand with a setter. Does anyone have any
suggested best practices for handling this situation? Thank you for
your time.

Best regards,
--
Arthur Kalmenson

Alen Vrecko

unread,
Aug 17, 2009, 4:23:50 PM8/17/09
to google-guice
I am in a similar situation. Have some displays containing other
displays. I am also using setters. But this is what I am experimenting
with:

I have a Document editing UI that takes a file browser ui (each
document can have corresponding files) and a single file upload ui.
This is how it looks like:

// other presenters are injected
DocumentEditor(Presenter) is ctor injected with
+FileBrowserPresenter
+SingleFileUploadPresenter
+RpcService
+EventBus

// the presenter is injected with the presenter
DocumentEditorWidget is ctor injected with
+DocumentEditorPresenter
+SingleFileUploadWidget // created via new
+FileBrowserWidget // created via new

that is right. I made the widget have a strong dependency on the
presenter. I see the Presenter/View a wholesome unit. Where logic from
the UI has been factored out in the presenter.

This is different from what is presented at the slides

PhoneEditWidget phoneEditWidget = new PhoneEditWidget();
PhoneEditor phoneEditor = new PhoneEditor(phoneEditWidget,
rpcService);

I am doing

PhoneEditor phoneEditor = new PhoneEditor(phoneEditWidget,
rpcService);
PhoneEditWidget phoneEditWidget = new PhoneEditWidget(phoneEditor);

I can then do

@Inject Provider<DocumentEditorWidget> documentEditors;
panel.add(documentEditors.get());

with no problems.

the bindDisplay and display of DocumentEditor look like

public static interface Display{
...
FileBrowser.Display getFileBrowser();
SingleFileUpload.Display getUpload();
}

public void bindDisplay(Display display) {
...
fileBrowserPresenter.bindDisplay(display.getFileBrowser());
uploadPresenter.bindDisplay(display.getUpload());
}

I can't really call this a best practice. Will see what the code will
tell me after some more time. Am also interested in what other people
are doing.

Cheers
Alen

Eduardo Nunes

unread,
Aug 18, 2009, 12:06:08 AM8/18/09
to google...@googlegroups.com
I would recommend both of you take a look in
http://gwt-mvp-sample.googlecode.com
It's an implementation and sample application using the MVP pattern.
I'm working on a new version and I will publish it as soon as
possible.

Best regards,
Eduardo S. Nunes
--
Eduardo S. Nunes
http://e-nunes.com.br

Alen Vrecko

unread,
Aug 18, 2009, 8:03:49 AM8/18/09
to google-guice
I've looked at it some weeks ago. It would be nice if you added some
tests for your example. I am wondering why do you have an interface
for the presenter and not just a concrete class?

Cheers,
Alen

On Aug 18, 6:06 am, Eduardo Nunes <esnu...@gmail.com> wrote:
> I would recommend both of you take a look inhttp://gwt-mvp-sample.googlecode.com

Tercio Filho

unread,
Aug 18, 2009, 8:13:12 AM8/18/09
to google-guice
Hi!

I've been using a variation of Eduardo's implemantation for a while,
and it works great.

In my application I'm using setters and a initialize method.
Unfortunately GIN doesn't support Assisted Injection, as Guice do, so
no go for immutable objects.

Regards,

Tercio

Eduardo Nunes

unread,
Aug 18, 2009, 8:43:02 AM8/18/09
to google...@googlegroups.com
Because when you have a presenter that uses another presenter, you
should wire than by interfaces. Consider that you have to test a
presenter A that has references to presenters B and C, using
interfaces you can test presenter A providing mocks of presenters B
and C.

Best regards,
Eduardo S. Nunes

Etienne Neveu

unread,
Aug 22, 2009, 8:22:59 AM8/22/09
to google-guice
Hi,

*WALL OF TEXT BEGINS ;) *

We also encountered this problem in our app.

In our case, our root presenter did not need to hold references to our
child presenter(s). In this case, you only need to solve the problem
you mentioned: "If ContactView has EmailView injected, we get into a
dilemma where we don't have the Presenter instantiated". So I did
something like this:

-------------------------

We are using gwt-presenter. If you extend "WidgetPresenter" (
http://code.google.com/p/gwt-presenter/source/browse/trunk/src/main/java/net/customware/gwt/presenter/client/widget/WidgetPresenter.java
), you can do the following in your view:

public class ContactView extends VerticalPanel implements
ContactPresenter.View {

private final Provider<EmailPresenter> emailPresenterProvider;

@Inject
public ContactView(Provider<EmailPresenter> emailPresenterProvider) {
emailPresenterProvider = emailPresenterProvider;
}

private Widget createEmailView() {
return emailPresenterProvider.get().getDisplay().asWidget();
}

private void buildWidget() {
add(new HtmlPanel("Hi there");
// Create 3 different email views, with 3 different
presenters instantiated
add(createEmailView());
add(createEmailView());
add(createEmailView());
}
}

The problem is that you make your views aware of the presenters. But
since you unit-test the presenters, it won't pose a problem while unit-
testing.

Another solution if you want to abstract the presenter from the view,
would be to use a provider method in your GIN module:

@Provides
EmailWidget createView(EmailPresenter presenter) {
return presenter.getDisplay().asWidget();
}

and then inject a Provider<EmailWidget> in your view.

Each call to emailWidgetProvider.get() will then create a new
presenter (which gets injected with a new view), give it to the
provider method, and return the associated widget to be used by the
view. This solves the problem of "not having the presenter
instantiated".

-------------------------

Now, this "simple case" works because we supposed our ContactPresenter
does not need to access the EmailPresenters. You can avoid these kind
of dependencies by communicating between the presenters through the
eventbus. But you won't always be able to do so, and in the real world
you will often want to be able to call methods on your child
presenters.

I think I would do something like this:


public class ContactPresenter extends
WidgetPresenter<ContactPresenter.View> {
public static interface View extends WidgetDisplay {
void add(WidgetDisplay display);

void remove(WidgetDisplay display);
}

private final Provider<EmailPresenter> emailPresenterProvider;

private Contact contact;

private Map<Email, EmailPresenter> emailMap = new HashMap<Email,
EmailPresenter>();

public ContactPresenter(ContactView display, EventBus eventBus,
Provider<EmailPresenter> emailPresenterProvider,
ContactModel contactModel) {
super(display, eventBus);
this.emailPresenterProvider = emailPresenterProvider;

}

void onBind() {
registerHandler(eventBus.addHandler(EmailAddedEvent.getType(),
new EmailAddedHandler() {
void onEmailAdded(EmailAddedEvent event) {
// should this ContactPresenter handle this email?
if (contact.equals(event.getContact())) {
addEmail(event.getEmail());
}
}
}));
registerHandler(eventBus.addHandler(EmailRemovedEvent.getType(),
new EmailRemovedHandler() {
void onEmailRemoved(EmailRemovedEvent event) {
// should this ContactPresenter handle this email?
if (contact.equals(event.getContact())) {
removeEmail(event.getEmail());
}
}
}));

}

void addEmail(Email email) {
EmailPresenter emailPresenter = emailPresenterProvider.get();
emailPresenter.setEmail(email);
emailMap.put(email, emailPresenter);
display.add(emailPresenter.getDisplay());
}

void removeEmail(Email email) {
EmailPresenter emailPresenter = emailMap.get(email);
display.remove(emailPresenter.getDisplay());
emailPresenter.destroy();
emailMap.remove(email);
}

void onUnbind() {
// handlers are de-registered by BasicPresenter superclass
// since we called registerHandler() in the onBind() method
for (Email email : emailMap.keySet()) {
removeEmail(email);
}
}

// Should be injected in the constructor with assisted inject or with
a
// factory?
void setContact(Contact contact) {
this.contact = contact;
for (Email email : contact.getEmails()) {
addEmail(email);
}

}

}

class ContactView extends VerticalPanel implements
ContactPresenter.View {

// Create the view and so on...

void add(WidgetDisplay display) {
add(display.asWidget());

}

void remove(WidgetDisplay display) {
remove(display.asWidget());
}

}

(some things might be missing, since I wrote that without any dev
environment... but the main ideas are there)

-------------------------


All of this works because the Presenter has a getDisplay() method, and
the WidgetDisplay has a asWidget() method returning the underlying
widget. One would think that the asWidget() method is bad when unit-
testing presenters, but you can simply return null in your
WidgetDisplay mock.

Actually, in our application, since we use SmartGwt, where the high-
level component is a "Layout" instead of a "Widget" we created a
LayoutPresenter and a LayoutDisplay (with a
com.smartgwt.client.widgets.layout.Layout asLayout() method). We also
have a WindowPresenter / WindowDisplay, where the WindowPresenter
superclass handles some of the window logic (with a openWindow()
method that calls bind() and then shows the window... and a closeWindow
() that calls unbind() and then hides the window...).



What do you think of this kind of architecture? I'm interested in
other ideas and best practices as well.

Regards,
-Etienne

Alen Vrecko

unread,
Sep 8, 2009, 11:14:58 AM9/8/09
to google-guice
Hi,

maybe useful maybe not. This is what I ended up doing (similar to
Etienne's 2nd suggestion but different):

// a document has an associated collection of files
// document editor contains the file editor

class DocumentEditorPresenter{

interface Display{
....
// the Document editor view takes file editor view
void adoptDisplay(FileEditorPresenter.Display display);
}

DocumentEditorPresenter(Display display, ..., FileEditorPresenter
fileEditorPresenter, ....){
// I ask the parent display to adopt the child display
// get the display from the child presenter
display.adoptDisplay(fileEditorPresenter.getDisplay());
....
}

void onBind(){
// also call the child presenters onBind
// what the file presenter is doing with event bus
// and services is not the concern of doc presenter
fileEditorPresenter.onBind();
....
}

void edit(DmsDocument document){
// delegate file handling to child presenter
fileEditorPresenter.edit(document.getFiles());
....
}

}

There are quite a lot of options with MVP.

Cheers
Alen

On Aug 22, 2:22 pm, Etienne Neveu <nev...@gmail.com> wrote:
> Hi,
>
> *WALL OF TEXT BEGINS ;) *
>
> We also encountered this problem in our app.
>
> In our case, our root presenter did not need to hold references to our
> child presenter(s). In this case, you only need to solve the problem
> you mentioned: "If ContactView has EmailView injected, we get into a
> dilemma where we don't have the Presenter instantiated". So I did
> something like this:
>
> -------------------------
>
> We are using gwt-presenter. If you extend "WidgetPresenter" (http://code.google.com/p/gwt-presenter/source/browse/trunk/src/main/j...
Reply all
Reply to author
Forward
0 new messages