Критика моего простого приложения MVP Winforms

Я пытаюсь обратить внимание на шаблон MVP, используемый в приложении С#/Winforms. Поэтому я создал простой "блокнот", такой как приложение, чтобы попытаться разобрать все детали. Моя цель - создать что-то, что делает классические поведения в окнах open, save, new, а также отражает имя сохраненного файла в строке заголовка. Кроме того, когда есть несохраненные изменения, строка заголовка должна включать *.

Итак, я создал представление и ведущий, которые управляют состоянием сохранения приложения. Одно усовершенствование, которое я рассмотрел, - это разбор кода обработки текста, поэтому view/presenter - это действительно одноцелевая сущность.

Вот скриншот для справки...

alt text

Я включаю все соответствующие файлы ниже. Меня интересует обратная связь о том, правильно ли я это сделал или есть способы улучшить.

NoteModel.cs:

public class NoteModel : INotifyPropertyChanged 
{
    public string Filename { get; set; }
    public bool IsDirty { get; set; }
    string _sText;
    public readonly string DefaultName = "Untitled.txt";

    public string TheText
    {
        get { return _sText; }
        set
        {
            _sText = value;
            PropertyHasChanged("TheText");
        }
    }

    public NoteModel()
    {
        Filename = DefaultName;
    }

    public void Save(string sFilename)
    {
        FileInfo fi = new FileInfo(sFilename);

        TextWriter tw = new StreamWriter(fi.FullName);
        tw.Write(TheText);
        tw.Close();

        Filename = fi.FullName;
        IsDirty = false;
    }

    public void Open(string sFilename)
    {
        FileInfo fi = new FileInfo(sFilename);

        TextReader tr = new StreamReader(fi.FullName);
        TheText = tr.ReadToEnd();
        tr.Close();

        Filename = fi.FullName;
        IsDirty = false;
    }

    private void PropertyHasChanged(string sPropName)
    {
        IsDirty = true;
        PropertyChanged.Invoke(this, new PropertyChangedEventArgs(sPropName));
    }


    #region INotifyPropertyChanged Members

    public event PropertyChangedEventHandler PropertyChanged;

    #endregion
}

Form2.cs:

public partial class Form2 : Form, IPersistenceStateView
{
    PersistenceStatePresenter _peristencePresenter;

    public Form2()
    {
        InitializeComponent();
    }

    #region IPersistenceStateView Members

    public string TheText
    {
        get { return this.textBox1.Text; }
        set { textBox1.Text = value; }
    }

    public void UpdateFormTitle(string sTitle)
    {
        this.Text = sTitle;
    }

    public string AskUserForSaveFilename()
    {
        SaveFileDialog dlg = new SaveFileDialog();
        DialogResult result = dlg.ShowDialog();
        if (result == DialogResult.Cancel)
            return null;
        else 
            return dlg.FileName;
    }

    public string AskUserForOpenFilename()
    {
        OpenFileDialog dlg = new OpenFileDialog();
        DialogResult result = dlg.ShowDialog();
        if (result == DialogResult.Cancel)
            return null;
        else
            return dlg.FileName;
    }

    public bool AskUserOkDiscardChanges()
    {
        DialogResult result = MessageBox.Show("You have unsaved changes. Do you want to continue without saving your changes?", "Disregard changes?", MessageBoxButtons.YesNo);

        if (result == DialogResult.Yes)
            return true;
        else
            return false;
    }

    public void NotifyUser(string sMessage)
    {
        MessageBox.Show(sMessage);
    }

    public void CloseView()
    {
        this.Dispose();
    }

    public void ClearView()
    {
        this.textBox1.Text = String.Empty;
    }

    #endregion

    private void btnSave_Click(object sender, EventArgs e)
    {
        _peristencePresenter.Save();
    }

    private void btnOpen_Click(object sender, EventArgs e)
    {
        _peristencePresenter.Open();
    }

    private void btnNew_Click(object sender, EventArgs e)
    {
        _peristencePresenter.CleanSlate();
    }

    private void Form2_Load(object sender, EventArgs e)
    {
        _peristencePresenter = new PersistenceStatePresenter(this);
    }

    private void Form2_FormClosing(object sender, FormClosingEventArgs e)
    {
        _peristencePresenter.Close();
        e.Cancel = true; // let the presenter handle the decision
    }

    private void textBox1_TextChanged(object sender, EventArgs e)
    {
        _peristencePresenter.TextModified();
    }
}

IPersistenceStateView.cs

public interface IPersistenceStateView
{
    string TheText { get; set; }

    void UpdateFormTitle(string sTitle);
    string AskUserForSaveFilename();
    string AskUserForOpenFilename();
    bool AskUserOkDiscardChanges();
    void NotifyUser(string sMessage);
    void CloseView();
    void ClearView();
}

PersistenceStatePresenter.cs

public class PersistenceStatePresenter
{
    IPersistenceStateView _view;
    NoteModel _model;

    public PersistenceStatePresenter(IPersistenceStateView view)
    {
        _view = view;

        InitializeModel();
        InitializeView();
    }

    private void InitializeModel()
    {
        _model = new NoteModel(); // could also be passed in as an argument.
        _model.PropertyChanged += new PropertyChangedEventHandler(_model_PropertyChanged);
    }

    private void InitializeView()
    {
        UpdateFormTitle();
    }

    private void _model_PropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
    {
        if (e.PropertyName == "TheText")
            _view.TheText = _model.TheText;

        UpdateFormTitle();
    }

    private void UpdateFormTitle()
    {
        string sTitle = _model.Filename;
        if (_model.IsDirty)
            sTitle += "*";

        _view.UpdateFormTitle(sTitle);
    }

    public void Save()
    {
        string sFilename;

        if (_model.Filename == _model.DefaultName || _model.Filename == null)
        {
            sFilename = _view.AskUserForSaveFilename();
            if (sFilename == null)
                return; // user canceled the save request.
        }
        else
            sFilename = _model.Filename;

        try
        {
            _model.Save(sFilename);
        }
        catch (Exception ex)
        {
            _view.NotifyUser("Could not save your file.");
        }

        UpdateFormTitle();
    }

    public void TextModified()
    {
        _model.TheText = _view.TheText;
    }

    public void Open()
    {
        CleanSlate();

        string sFilename = _view.AskUserForOpenFilename();

        if (sFilename == null)
            return;

        _model.Open(sFilename);
        _model.IsDirty = false;
        UpdateFormTitle();
    }

    public void Close()
    {
        bool bCanClose = true;

        if (_model.IsDirty)
            bCanClose = _view.AskUserOkDiscardChanges();

        if (bCanClose)
        {
            _view.CloseView();
        }
    }

    public void CleanSlate()
    {
        bool bCanClear = true;

        if (_model.IsDirty)
            bCanClear = _view.AskUserOkDiscardChanges();

        if (bCanClear)
        {
            _view.ClearView();
            InitializeModel();
            InitializeView();
        }
    }
}

Ответы

Ответ 1

Единственный способ приблизиться к идеальному шаблону пассивного представления MVP - это написать собственные триады MVP для диалогов, а не использовать диалоги WinForms. Затем вы можете переместить логику создания диалога из представления в презентатора.

Это попадает в тему связи между triads mvp, тема, которая обычно затушевывается при изучении этого шаблона. То, что я нашел для меня, - это связать триады с их ведущими.

public class PersistenceStatePresenter
{
    ...
    public Save
    {
        string sFilename;

        if (_model.Filename == _model.DefaultName || _model.Filename == null)
        {
            var openDialogPresenter = new OpenDialogPresenter();
            openDialogPresenter.Show();
            if(!openDialogPresenter.Cancel)
            {
                return; // user canceled the save request.
            }
            else
                sFilename = openDialogPresenter.FileName;

        ...

Конечно, метод Show() несет ответственность за показ не упомянутого OpenDialogView, который будет принимать вход пользователя и передавать его вместе с OpenDialogPresenter. В любом случае, должно стать ясно, что ведущий является продуманным посредником. В разных обстоятельствах у вас может возникнуть соблазн реорганизовать посредника, но здесь он намерен:

  • Извлеките логику из представления, где сложнее протестировать
  • Избегайте прямых зависимостей между представлением и моделью.

Иногда я также видел модель, используемую для связи триады MVP. Преимущество этого заключается в том, что ведущий не должен знать друг друга. Обычно это достигается путем установки состояния в модели, которое запускает событие, которое затем прослушивает другой ведущий. Интересная идея. Один из них я не использовал лично.

Здесь несколько ссылок на некоторые из тех методов, которые другие использовали для связи с триадой:

Ответ 2

Все выглядит хорошо, единственный возможный уровень, на который я хотел бы пойти дальше, - отвлечь логику сохранения файла и обработать его провайдерами, чтобы потом вы могли легко сгибаться в альтернативных методах сохранения, таких как база данных, электронная почта, облачное хранилище.

IMO в любое время, когда вы касаетесь файловой системы, всегда лучше абстрагировать ее от уровня, а также упрощает насмешку и тестирование.

Ответ 3

Одна вещь, которую я люблю делать, - это избавиться от прямой связи View to Presenter. Причиной этого является представление на уровне пользовательского интерфейса, а ведущий - на бизнес-уровне. Мне не нравятся мои слои, чтобы иметь неотъемлемые знания друг о друге, и я стараюсь максимально ограничить прямую связь. Как правило, моя модель - это единственное, что выходит за пределы слоев. Таким образом, ведущий манипулирует представлением через интерфейс, но представление не требует особого прямого действия против ведущего. Мне нравится, чтобы Ведущий мог слушать и манипулировать моим взглядом, основанным на реакции, но я также хотел бы ограничить знание, которое имеет мое мнение у его ведущего.

Я добавлю некоторые события в свой IPersistenceStateView:

event EventHandler Save;
event EventHandler Open;
// etc.

Затем мой Ведущий прослушивает эти события:

public PersistenceStatePresenter(IPersistenceStateView view)
{
    _view = view;

    _view.Save += (sender, e) => this.Save();
    _view.Open += (sender, e) => this.Open();
   // etc.

   InitializeModel();
   InitializeView();
}

Затем измените реализацию представления, чтобы кнопки нажимали на события.

Это делает актёра более похожим на кукловодителя, реагируя на представление и вытягивая его струны; устраняя прямые вызовы методов презентатора. Вам все равно придется создавать экземпляр презентатора в представлении, но это касается единственной прямой работы, которую вы будете делать на нем.