Simple code structure (meaning small number of classes) does not necessarily mean easy maintenance (see Ayende's recent post on this). My suggestion is to keep to the Single Responsibility Principle (SRP). A Model's responsibility in MVP is to encapsulate data to the view, but in your case it gets another responsibility: fetching the data from the service. Apart from this overloading of responsibilities, I see two other problems with this:
- What happens if the call to the service fails? Then you'll be forced to handle this in view's code.
- What responsibilities does the presenter have in your case?
It seems to me your architecture isn't really MVP (but without seeing the code, I might be wrong).
Also, the actual implementation of MVP pattern depends on the GUI technology you are using. My general suggestions:
- Stick to the Passive View pattern.
- Keep the model dumb. I generally use simple POCO/POJO classes, no callbacks. I use C# view events in WinForms to notify the presenter about any UI events.
- Make the model view-friendly: models should generally be implemented for view's needs, so that the view code is kept to the minimum.
- Presenter is the king. Keep all the business logic inside presenters (and other services/repositories).
UPDATE: answer to your comment:
- Yes, you should store the data in your model.
If I understand correctly, you expose events/callbacks in the model for handling exceptions. Then you let the view fetch data, and in case a service call fails, the presenter handles this by (again) calling the view to let the user know the service failed. I see several problems with this approach:
- It can create convoluted execution paths when a service exception occurs: presenter -> view -> model -> presenter -> view. This can be tricky to debug and unit test.
- How do you know where the view was (in terms of filling the view with the model data) when the exception occurred?
My usual approach is for the presenter to do all the fetching and exception handling before the view gets its hands on the model data. That way I don't need any callbacks in the model, since I can handle exceptions with simple try-catch blocks in the presenter. The model becomes just a holder of static data and not a gateway to backend services.