Implementing both poor-man's MVP and MVPVM framework

I'm actually building some kind of framework to promote code reuse without over-reusing it. Besides, sometimes as it occurs with generic types, one expects type parameters to be constrained for the purpose.

In MVP, those constraints translate well enough.

Presenter

public abstract class Presenter where V : IView where M : IModel {
    protected Presenter(V view) {
        if (view == null) throw new ArgumentNullException("view");
        View = view;
    }

    public V View { get; private set; }

    public void CloseView() { View.CloseSelf(); }
    public void HideView() { View.HideSelf(); }
    public void SetViewMessage(string message) { View.Message = message; }
    public void SetViewTitle(string title) { View.Title = title; }
    public void ShowView() { View.ShowSelf(); }
}

IView

public interface IView where M : IModel {
    string Message { get; set; }
    M Model { get; set; }
    string Title { get; set; }

    void CloseSelf();
    void HideSelf();
    void ShowSelf();
}

IModel

// This is called the marker interface pattern.
public interface IModel { }

So any business domain object shall implement interface IModel or be marked as being an IModel in order to be accepted in type parameter M to the IView interface.

But now, when comes time to play with the MVPVM UI architecture, this gets kind of repetitive, or else, annoying, to have to specify all those types because the same type can be stated twice and even three times for a single class declaration.

Presenter

```
public abstract class Presenter
where V : IView
where VM : ViewModel
where M : IModel {

protected Presenter(V view) {
if (view == null) throw new ArgumentNullException("view");
View = view;
}

public V View { get; private set; }

public void CloseView() { View.CloseSelf(); }
public void HideView() { View.HideSelf(); }
public void SetViewMessage(string message) { View.Message = message; }
publ

public abstract class Presenter 
    where V : IView 
    where VM : ViewModel 
    where M : IModel

Convention for single-letter generic type parameter is to start with T, followed by U. If you're going to have more than that, In any case, it's better to give them meaningful names, and to start them with a T to stick with the convention.

This would be:

public abstract class Presenter 
    where TView : IView 
    where TViewModel : ViewModel 
    where TModel : IModel

This isn't a useful comment:

// This is called the marker interface pattern.
public interface IModel { }

A more use useful comment would be a XML summary that shows up on tooltips and IntelliSense when writing the client code. A good framework is a documented framework: every exposed member should at least have a short descriptive XML summary:

/// A marker interface that marks a class as a model.
public interface IModel { }

So that the consumer of your code knows what to expect, as they type : IModel - these XML comments show up in IntelliSense.

But the problem is not the comment; Marker Interface is not only a pattern, it's also a code/design smell, 99% of the time (as would be, say, a Singleton Pattern).

You're probably better off just constraining the M/TModel type parameter like this:

where TModel : class

Which ensures the model is a reference type, and leaves you with more flexibility and less boilerplate in implementing classes - limiting valid models to types implementing an empty IModel interface, serves no real purpose.

protected void RaisePropertyChangedFor(string propertyName) {
    if (propertyName == null) throw new ArgumentNullException("propertyName");
    if (PropertyChanged != null)
        PropertyChanged(this, new PropertyChangedEventArgs(propertyName);
}

You're missing a ) on the event method call line, just before the ;. Visual Studio has it squiggly-red underlined, and refuses to build your project. Pretend I didn't see it.

The invocation isn't thread-safe. This would be:

protected void RaisePropertyChangedFor(string propertyName) 
{
    if (propertyName == null) 
    { 
        throw new ArgumentNullException("propertyName");
    }

    var handler = PropertyChanged;
    if (handler != null)
    {
        handler(this, new PropertyChangedEventArgs(propertyName));
    }
}

Notice the local handler variable belongs to whatever thread is running this; it isn't the case for PropertyChanged, so there's a little race condition where a thread would remove the last handler on PropertyChanged after the null check, but before the invocation.

StackExchange Code Review Q#70969, answer score: 6