シンプルなMVP Winformsアプリを批判する[非公開]

Question

私は、C＃/ Winformsアプリで使用されるMVPパターンを意識しようとしています。そこで、簡単な「メモ帳」を作成しました。すべての詳細を解決しようとするアプリケーションのような。私の目標は、開いている、保存する、新しいという古典的なウィンドウの動作を行うだけでなく、保存されたファイルの名前をタイトルバーに反映するものを作成することです。また、未保存の変更がある場合は、タイトルバーに*を含める必要があります。

だから私はビューを作成しました＆amp;アプリケーションの永続状態を管理するプレゼンター。私が検討した改善点の1つは、ビュー/プレゼンターが本当に単一目的のエンティティになるようにテキスト処理コードを分割することです。

参照用のスクリーンショットは次のとおりです...

以下の関連ファイルをすべて含めています。正しい方法で行ったかどうか、改善する方法があるかどうかについてのフィードバックに興味があります。

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();
        }
    }
}

Answer 1

完全なMVPパッシブビューパターンに近づける唯一の方法は、WinFormsダイアログを使用する代わりに、ダイアログ用に独自のMVPトライアドを作成することです。次に、ダイアログ作成ロジックをビューからプレゼンターに移動できます。

これは、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トライアド通信に使用されるモデルを見たこともあります。これの利点は、プレゼンターがお互いの存在を知る必要がないことです。通常は、モデルに状態を設定し、イベントをトリガーして別のプレゼンターがリッスンすることで実現します。面白いアイデア。個人的に使用したことのないもの。

トライアドコミュニケーションに対処するために他の人が使用したテクニックのいくつかとのリンクを次に示します。

• http://www.rickardnilsson.net/post/The -Humble-dialog-v2.aspx
• http：// spin。 atomicobject.com/2008/01/30/presenter-first-get-your-triads-talking/

Answer 2

私がさらに進める唯一の可能なレベルですべてが良さそうに見えるのは、ファイルを保存するロジックを抽象化し、それをプロバイダーが処理するようにすることです。そうすることで、後でデータベース、メール、クラウドストレージなどの別の保存方法を簡単に変更できます

ファイルシステムに触れるときはいつでも、IMOはレベルを抽象化する方が常に良いです。また、モックやテストをより簡単にします。

Answer 3

私がしたいことの1つは、View to Presenterの直接的なコミュニケーションを取り除くことです。これは、ビューがUIレベルであり、プレゼンターがビジネスレイヤーにあるためです。私は自分のレイヤーがお互いの固有の知識を持つことを好まないので、直接的なコミュニケーションを可能な限り制限しようとします。通常、私のモデルはレイヤーを超越する唯一のものです。したがって、プレゼンターはインターフェイスを介してビューを操作しますが、ビューはプレゼンターに対してあまり直接的なアクションを実行しません。プレゼンターが反応に基づいて自分の意見を聞いたり操作したりできるのが好きですが、自分の意見がプレゼンターについて持っている知識を制限したいのです。

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();
}

次に、ボタンのクリックでイベントが発生するようにビューの実装を変更します。

これにより、プレゼンターは人形遣いのように振る舞い、ビューに反応して弦を引っ張ります。その中で、プレゼンターのメソッドの直接呼び出しを削除します。ビューでプレゼンターをインスタンス化する必要がありますが、それはあなたがそれに対して行う唯一の直接的な作業についてです。