Design/Architecture for MVC WinForms Dialog Control/Use

Question

All I have built an MVC WinForms application where Views are blind and deaf of their controllers. To facilitate this I have used Ninject as my Dependency Injection (DI) Inversion of Control (IoC) container. I have a question regarding my design/architecture of the way I handle showing dialogs (remembering I want full testability for all components).

I have two "marker" interfaces for the controllers and views of dialogs. The controller interface is:

IDialogController.cs:

public interface IDialogController : IDisposable { } 

Made disposable to enusure the view is cleaned up when we use using(saveDialogController){ ... }. The view interface is IDialogView.cs:

public interface IDialogView { }

I then have my ISaveDialogView.cs as:

public interface ISaveDialogView
{
    /// <summary>
    /// Load the items to be displayed in the ListView.
    /// </summary>
    /// <param name="items">The items to display.</param>
    void LoadItemList(IEnumerable<string> items);
}

and the actual view that uses this interface is SaveDialogView.cs:

public partial class SaveDialogView : Form, ISaveDialogView, IDialogView
{
    public SaveDialogView()
    {
        InitializeComponent();
    }

    private void buttonYes_Click(object sender, EventArgs e)
    {
        DialogResult = DialogResult.Yes;
        Close();
    }

    private void buttonNo_Click(object sender, EventArgs e)
    {
        DialogResult = DialogResult.No;
        Close();
    }

    private void buttonCancel_Click(object sender, EventArgs e)
    {
        DialogResult = DialogResult.Cancel;
        Close();
    }

    public void LoadItemList(IEnumerable<string> items)
    {
        listBoxItems.DataSource = items;
    }
}

and the SaveDialogController.cs is:

public class SaveDialogController : IDialogController
{
    private ISaveDialogView view = null;

    public SaveDialogController(ISaveDialogView view)
    {
        if (view == null)
            throw new ArgumentNullException("view");

        this.view = view;
        Log.Trace("SaveDialogController.Ctor(): Initialized successfully");
    }

    public DialogResult ShowDialog(IWin32Window owner, IEnumerable<string> items)
    {
        view.LoadItemList(items);
        using ((Form)view)
        {
            var result = ((Form)view).ShowDialog();
            Log.Trace($"SaveDialogController.ShowDialog(): " + 
                "DialogResult.{result.ToString()} returned from view");
            return result;
        }
    }

    #region IDisposable Members.
    private bool disposed = false;

    protected virtual void Dispose(bool disposing)
    {
        if (!disposed)
        {
            if (disposing)
            {
                if (view != null)
                {
                    ((Form)view).Dispose();
                    view = null;
                    Log.Trace("SaveDialogController.Dispose(): View disposed");
                }
            }
            disposed = true;
        }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
    #endregion // IDisposable Members.
}

I can then call the dialog from another controller as follows:

using (SaveDialogController c = DialogControllerFactory.GetController<SaveDialogController>())
{
    DialogResult result = c.ShowDialog((Form)View, new List<string>() { "A", "B", "C" });
    // Do stuff...
}

My questions on this are as follows:

1. Is this design/architecture okay, could it be made better, if so, how?
2. Are there any problems with the way I am disposing the view/Form here?
3. Any other comments?

Thanks very much for your time.

Answer 1

I am not sure what exactly do you archive with that architecture. In the example code above, the controller doesn't have any logic - so you can just drop it and use the dialog directly.

However, if that is just an example and you add more logic to the controller lets take a short look to the following method:

public DialogResult ShowDialog(IWin32Window owner, IEnumerable<string> items)
{
    view.LoadItemList(items);
    using ((Form)view)
    {
        var result = ((Form)view).ShowDialog();
        Log.Trace($"SaveDialogController.ShowDialog(): " + 
            "DialogResult.{result.ToString()} returned from view");
        return result;
    }
}

• If the mehtod ShowDialog is called, the view (instance variable) will be disposed (because: using ((Form)view)) . If the method is called again, it works on a disposed view. If the controller will be disposed later, it tries to dispose the view again. So you dont need the using here.

• If you cast your abstraction against the concrete class (((Form)view)), the abstraction is valueless because you can not mock it in unit tests. To avoid casting, add all needed methods (in that case ShowDialog) to your abstraction.

• The instance variable view should be readonly because it is initialized once in constructor.