Use of a separate class to connect to a service when using MVP for a form

Consider a case of a form that is getting its data from a service and is implemented using the MVP pattern. Do I need to isolate the service access in a separate class or it can be kept in the Model class itself. In my particular开发者_运维技巧 case, the model class only acts as a pass through to the data access class that eventually calls the service. The data access class has logic that is used for service callbacks and querying the service in specific time intervals for any updates.

I am asking this, because I would like to keep my code structure as simple as possible and only use additional classes if needed. But I also do not want to simplify it so much that future maintenance/extension can become very difficult.

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:

1. What happens if the call to the service fails? Then you'll be forced to handle this in view's code.
2. 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:

  1. 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.
  2. 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.

The Presenter could use a Repository which will talk to the web service.

public class SomePresenter
{
    private readonly ISomeView _view;
    private readonly ISomeRepository _repository;

    public class SomePresenter(ISomeView view, ISomeRepository repository)
    {
        _view = view;
        _repository = repository;
    }

    public void Foo()
    {
        var model = _repository.GetModel();
        // TODO: work with the model and update the view
    }
}

When instantiating the presenter you would pass a real implementation of the repository which will talk to the web service.

Such class is usually called Service Agent. Service Agent is used for loosely coupling between service and client and for supporting some intermediate logic. Based on your description your data access class already is a Service Agent.