views:

277

answers:

3

I'm working on building up an MVP application (C# Winforms). My initial version is at http://stackoverflow.com/questions/1422343/ ... Now I'm increasing the complexity. I've broken out the code to handle two separate text fields into two view/presenter pairs. It's a trivial example, but it's to work out the details of multiple presenters sharing the same model.

My questions are about the model:

  1. I am basically using a property changed event raised by the model for notifying views that something has changed. Is that a good approach? What if it gets to the point where I have 100 or 1000 properties? Is it still practical at that point?

  2. Is instantiating the model in each presenter with   NoteModel _model = NoteModel.Instance   the correct approach? Note that I do want to make sure all of the presenters are sharing the same data.

  3. If there is a better approach, I'm open to suggestions ....

My code looks like this:

NoteModel.cs

public class NoteModel : INotifyPropertyChanged
{
    private static NoteModel _instance = null;

    public static NoteModel Instance
    {
        get { return _instance; }
    }

    static NoteModel()
    {
        _instance = new NoteModel();
    }

    private NoteModel()
    {
        Initialize();
    }

    public string Filename { get; set; }
    public bool IsDirty { get; set; }
    public readonly string DefaultName = "Untitled.txt";

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

    string _sMoreText;
    public string MoreText
    {
        get { return _sMoreText; }
        set
        {
            _sMoreText = value;
            PropertyHasChanged("MoreText");
        }
    }

    public void Initialize()
    {
        Filename = DefaultName;
        TheText = String.Empty;
        MoreText = String.Empty;
        IsDirty = false;
    }

    private void PropertyHasChanged(string sPropName)
    {
        IsDirty = true;

        if (PropertyChanged != null)
        {
            PropertyChanged(this, new PropertyChangedEventArgs(sPropName));
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;
}

TextEditorPresenter.cs

public class TextEditorPresenter
{
    ITextEditorView _view;
    NoteModel _model = NoteModel.Instance;

    public TextEditorPresenter(ITextEditorView view)//, NoteModel model)
    {
        //_model = model;
        _view = view;
        _model.PropertyChanged += new PropertyChangedEventHandler(model_PropertyChanged);
    }

    void model_PropertyChanged(object sender, PropertyChangedEventArgs e)
    {
        if (e.PropertyName == "TheText")
            _view.TheText = _model.TheText;
    }

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

    public void ClearView()
    {
        _view.TheText = String.Empty;
    }
}

TextEditor2Presenter.cs is essentially the same except it operates on _model.MoreText instead of _model.TheText.

ITextEditorView.cs

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

ITextEditor2View.cs

public interface ITextEditor2View
{
    string MoreText { get; set; }
}
+1  A: 
  1. This approach is good. However, if you are looking at having hundred (thousands even!) of Properties then I think you might have a God class (anti-pattern). There aren't many good classes with 100 properties. Instead consider breaking up your model into smaller classes. Furthermore, you don't need to have a separate event for each property. If the model is changed at all you can fire a single event (which might include information describing the change) and the views can handle it from there.
  2. I would avoid using the Singleton pattern unless you actually are sure you want it to apply. Instead, change the constructor for all your views to take in an instance of the model.
tster
1. I was exaggerating a little, I guess. I could foresee 100 things I want to track. My thought on the model would be then to encapsulate and aggregate so it would be something like Model.ComplexThing1 (with 10 properties) and Model.ComplexThing2 (with 4 properties), etc.2. I guess I'm not sure if I want it to apply. How do I know? My understanding of the model is that it is the definitive source of information stored during runtime. In the case, there should only ever be one instance of the model, right?
Keith G
@Keith: There's a difference in "there **should** only ever be one instance of the model" and "there **can** only ever be one instance of the model". You are free to use a singleton (small 's'!) instance by simply just creating a single instance in a factory etc., but only use a Singleton (capital 'S'!) when you **must**.
Johann Gerell
@Johan Gerell: As you "only use....when you must": Is there something bad about a Singleton in this context?
Marcel
@tster: Your point 2: What seems bad to you with the Singleton pattern in this context? And why should the View take the model in MVP as the pattern should separate the View from the Model?
Marcel
The reason I don't like it here is that there is really no reason other than "because I want to access the same object from multiple places" for it to be a singleton. Singleton is for places where it makes no sense to have more than 1 instance. For instance, database connections, resource pools, global factories or controllers. In this case, if you want later to support having multiple Notes, then you have to undo the Singleton.
tster
A: 

Remember, in any layered application, it's normal for the domain model to transcend all layers.

  1. Thus, I would have your presenter pass your Note instance to the view (which no doubt is a Control of some sort), and then let databinding through a BindingSource take over. Once you're using databinding, then the controls will automatically listen to the PropertyChanged event and update accordingly without the need for extra code on your part. Event-based notification is the appropriate use here no matter how many properties are being monitored as only the objects that care about the change will take action (vs. having many objects taking action unnecessarily).

  2. Typically, you get your entity instances from a lower layer. For example, you could implement a service that returns your Note instances. Anytime you ask that service for Note #3, it returns the same instance of Note that it created from persisted data. You could further more add another item to your business layer to supplement your presenters - it could be call a WorkItem or a Controller. All of your presenters could consult their WorkItem instance to get the current instance of Note upon which the user will be working.

I would consider looking into examples of how the Composite Application Block (or CAB) uses these patterns to create smart client applications. It's all design patterns and OO principles, the implementation of which is well worth the effort.

Travis Heseman
A: 
  • To Question 1: Implementing INotifyPropertyChanged seems to be a good idea to me. Probably you would however split the many properties into some classes.
  • To Question 2: I am currently using a Singleton pattern for sharing my MVP Model with multiple presenters. I am happy so far, as this guarantees, that there is really ever only one instance of my model.
Marcel