Question

I have implemented an MVP triad using the passive view pattern - i.e. the view contains only simple getters and setters. However I am having trouble seperating the view data and model data. In particular when handling a change in the view state.

The triad is used to enable the user to select a part from a list. The list of parts is supplied by the model with each part uniquely identified by a unique id.

Lets say the parts look like this:

class Part
{
    int ID; // this code uniquely identifies the part within the model
    String partCode;
    String description;
    double voltage;
}

The view displays the list to the user and allows them to select a part

The list is displayed in a DataGridView and a part is selected by clicking on a row in the dataGridView.

The ID is not to be displayed to the user and neither is the voltage, therefore the model creates a DataTable that contains just the partCode and description. This DataTable is assigned by the presenter to a property on the view that maps to the DataSource property of the DataGridView.

class Presenter
{
    IView _view;
    IModel _model;

    //...///

    _view.Data = _model.GetFilteredData();
}

class Model
{
    public DataTable GetFilteredData()
    {
        // create a DataTable with the partCode and Description columns only
        // return DataTable
    } 
}

class View  //winform
{
      public DataTable Data
      {
          set 
          {
              this.dataGridView.Source = value;
          }
      }
}

So far so good. The View dislays the filtered data in the DataGridView.

The problem I have is returning the part selected by the user.

The view has no knowledge of the unique ID since it is not displayed and the other information cannot be guaranteed to be unique - therfore it is not possible to uniquely identify the part selected.

Essentially i'm trying to convert view data (the selected row) to model data (the selected part) without one component using the others data.

So far I have the following solutions:

1) The view is passed a DataTable that contains the ID and then filters the display so that it is not displayed to the user. It is then trivial to return an ID for the selected row. The problem here is that i have now polluted the view with logic that is untested (the filtering of the display).

2) The view returns the row index and the model matches this index to a row in the original data. This would mean ensuring that the order in the view never changes, which while possible, restricts how the view can show (and manipulate) the data. This also pollutes the model with view data (the row index).

    public int RowIndexSelected { get; private set; }

    //...//

    private void gridParts_CellEnter(object sender, DataGridViewCellEventArgs e)
    {
        if (SelectedPartChangedEvent != null)
        {
            RowIndexSelected = e.RowIndex;

            SelectedPartChangedEvent();            
        }
    }

3) A variation on (2). Create an adapter object to sit between the presenter and view. Move the row to ID conversion code from the model to the adapter. The presenter then handles the dataGridAdapters part changed event.

    public PartSelectDataGridAdapter(IPartSelectView view, PartCollection data)
    {
        _view = view;
        _data = data;

        _view.SelectedPanelChangedEvent += HandleSelectedPartChanged;
    }

    void HandleSelectedPartChanged()
    {
        int id = _data[_view.RowIndexSelected].ID;

        if (SelectedPartChanged != null)
        {
            SelectedPartChanged(id);
        }
    }

At present im learning towards 3 since it is testable, keeps logic out of the view and view data out of the model and presenter.

How would you tackle this - is there a better solution?

Was it helpful?

Solution

The ID is not to be displayed to the user and neither is the voltage, therefore the model creates a DataTable that contains just the partCode and description.

Simple solution: do create an ID column in the datatable and hide it in in the datagrid view.

OTHER TIPS

I posted a simple solution earlier; this is a more detailed reply to the question

Is there a reason you don't want to pass a List<Part> to the view?

You could configure the grid to hide the id and voltage column. You can simply get the selected object from the binding source in the view. The presenter could query the view for this selection, or the view can call a SelectionChanged(Part selected) on the presenter.

It would mean you are no longer strictly following the passive-view pattern, but a supervising controller, because now your view knows about the model.

If you don't like this, you can introduce a view model, which you already do implicitly with your DataTable. (This is not necessarily bad, btw.)

In your example, the model classes knows about the view models, because you have methods on the model that generate them. I'd advice you to inverse this relationship: create methods on your view model that depend on your model objects. This way, you'll keep your model classes nice and clean and independent of all the UI data needed in the presentation layer.

When using the view model/supervising controller way, consider dropping the DataTable concept in favor of simple classes.

EDIT: alternative to make the view completely ignorant of the model:

Construct an instance of this class in the presenter, where you know of the both model and view model:

public class PartViewModel
{
  object PartModel { get; set; }
  string Name { get; set; }
  string Description { get; set; }
}

Pass a List<PartViewModel> as a datasource to the DataGridView. You can return the selected PartViewModel object to the presenter (either using an event or using a method). The presenter knows it can cast the PartModel property back to a Part instance. The view doesn't need to know anything about the model, as you say you are preferring. But you can still use simple object identity in the presenter, avoiding "complicated" lookup using id's.

With a presenter callback:

interface IPartListPresenter
{
  // other methods
  void SelectedPartChanged(PartViewModel nowSelected);
}

Assuming partBindingSource is the bindingsource the gridview is connected to, you can handle the CurrentChanged event of partBindingSource like this:

private void partBindingSource_CurrentChanged(object sender, EventArgs e)
{
  _presenter.SelectedPartChanged(partBindingSource.Current as PartViewModel);
}

In the presenter:

public void SelectedPartChanged(PartViewModel nowSelected)
{
  if(nowSelected == null)
  {
    return;
  }
  part myPart = (Part) nowSelected.Part;
  // dos stuff
}

Hope this helps.

I think you have misunderstood the whole concept a bit here!

It is the Presenter that should handle this, not the Model. The Model should only concentrate on the its sole responsibility, if not, you keep the View and Model too close!

My suggestion is to keep a hidden column in your table an pass the event of the selected row to your Presenter, and then let the Presenter handle to the work!

This will be the correct usage of MVP.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top