Refactoring my Todo List was on my Todo List. How's my MVC?

This is a follow up to Rolling my own Configuration with UI. I got a lot of great advice about the Configuration system I created, but no one touched on how all of my logic for the UI was in the code behind of the control. I decided that it was a bad idea, and took a stab at the Model-View-Controller pattern. Mostly, because I didn't like that none of that code was testable.

The code I would like to have reviewed is responsible for displaying a list of tokens to the user and allow the user to add/remove the tokens, as well as adjust their priority. (If you're curious, the tokens are used by another process as markers to be found in text searches.)

Did I get the pattern correct? Is this MVC? Are there any other code smells here?

My Form looks like this.

(The treeview is out of scope and stored in a different user control.)
Red text gives the control names.

ITodoSettingsView:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Rubberduck.Config;
using System.ComponentModel;
using System.Windows.Forms;

namespace Rubberduck.UI.Settings
{
    public interface ITodoSettingsView
    {
        TodoPriority ActiveMarkerPriority { get; set; }
        string ActiveMarkerText { get; set; }
        BindingList TodoMarkers { get; set; }
        int SelectedIndex { get; set; }
        bool SaveEnabled { get; set; }

        event EventHandler RemoveMarker;
        event EventHandler AddMarker;
        event EventHandler SaveMarker;
        event EventHandler SelectionChanged;
        event EventHandler TextChanged;
        event EventHandler PriorityChanged;

    }
}

TodoListSettingsUserControl (the view):

```
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Drawing;
using System.Data;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
using Rubberduck.Config;

namespace Rubberduck.UI.Settings
{
public p

MVC

Quoting from the provided wikipedia link


Interactions
In addition to dividing the application into three kinds of components, the model–view–controller design defines the interactions
between them.



-
A controller can send commands to the model to update the model's state (e.g., editing a document). It can also send commands to its
associated view to change the view's presentation of the model (e.g.,
by scrolling through a document).

-
A model notifies its associated views and controllers when there has been a change in its state. This notification allows the views to
produce updated output, and the controllers to change the available
set of commands. In some cases an MVC implementation might instead be
"passive," so that other components must poll the model for updates
rather than being notified.

-
A view requests information from the model that it uses to generate an output representation to the user.

• basically you don't have MVC because you don't have a model.

• your view has events to which the controller subscribes. This could be the other way around but the view should subscribe to the model events.

Your view should react on state changes triggered by the controller and/or the model.

How to do MVC

The image on wikipedia next to "Interactions" shows how it should be done.

The user interacts by clicking on a button or selecting some item in a drop down etc. with the controller. The controller then changes the state of the model which can trigger an update of the view usually by raising an event to which the view had subscribed.

So let us start with the model from the controller perspective

public interface IModel
{
   bool Update(IToDoMarker marker);
   bool Add(string text, TodoPriority priority);
   bool Remove(IToDoMarker marker);
   bool RequestMarkerList();
   IToDoMarker GetMarker(int index);
}

the controller will also need methods to be called by the view and e.g an event stating an error.

public interface IController
{
   void Update(int index, TodoPriority priority);
   void Update(int index, string text);
   void Update(int index, string text, TodoPriority priority);
   void Add(string text, TodoPriority priority);
   void Remove(int index);
   void RequestMarkerList();
   event EventHandler FailedCall;
}

and the model from the view perspective

public interface IViewModel
{
    event EventHandler MarkerListUpdated;
    event EventHandler MarkerUpdated;
    event EventHandler MarkerAdded;
    event EventHandler MarkerRemoved;
}

and the combined IToDoModel

public interface IToDoModel : IModel, IViewModel 
{ }

Partial implemented IController

public class ToDoController : IController
{
    private IModel _model;
    public ToDoController(IModel model)
    {
        _model = model;
    }

    public void Update(int index, TodoPriority priority)
    {
        IToDoMarker marker = GetMarker(index);
        if (marker == null) { OnFailedCall("Marker with given index not found"); }

        marker.Priority = priority;

        if (_model.Update(marker)) { return; }

        OnFailedCall("Update failed");
    }

    private IToDoMarker GetMarker(int index)
    {
        return _model.GetMarker(index);
    }

    private void OnFailedCall(String reason)
    {
        // create a FailedCallEventArgs object
        // and call the overloaded OnFailCall(FailedCallEventArgs e)
    }

    protected void OnFailCall(FailedCallEventArgs e)
    {
        // raise the FailedCall event
    }
    public event EventHandler FailedCall;
}

Partial implemented IToDoModel

public class ToDoModel : IToDoModel
{
    public bool Update(IToDoMarker marker)
    {
        bool updateSuccess = UpdateTheMarker(marker);
        if (updateSuccess)
        {
            // raise the MarkerUpdated event
        }
        return updateSuccess;
    }

    private bool UpdateTheMarker(IToDoMarker marker)
    {
        return true;
    }

    public event EventHandler MarkerUpdated;

}

You would then add a constructor to your usercontrol which takes an IController and an IViewModel as parameter.

public TodoListSettingsUserControl(IController controller, IViewModel model)
    : this()
{
    // subscribe to the model events and store in a variable
    // subscribe to the controller events and store in a variable
}

and call the whole thing like

IToDoModell model = new ToDoModel();
IController controller = new ToDoController((IModel)model);  
TodoListSettingsUserControl ctrl= new TodoListSettingsUserControl(controller, (IViewModel)model);

StackExchange Code Review Q#77639, answer score: 4