HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Enhance MVC implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mvcimplementationenhance

Problem

I know that there are several ways to develop the MVC pattern. To improve consistently my knowledge, I would like to get some advice and feedback. How could I enhance my code? I knew that there are several lines of code, but it would be great if you help me improve my skills.

The Model store the object Person and notify the view if there are any changes. The view implements the interface IModelPersonObserver to receive the notification. The view notify the controller about any changes. The controller starts a new task for long calculations. The result is passed to the model.

To pass different events I am able to use multiple events, like ModelHandler and ViewHandler based on the real action or just one event and extend the EventArgs class.

IModelPerson

public delegate void ModelHandler(IModelPerson sender, ModelEventArgs e);

public class ModelEventArgs : EventArgs
{
    public Person person;
    public ModelEventArgs(Person p) { person = p; }
}

public interface IModelPersonObserver
{
    void Add(IModelPerson model, ModelEventArgs e);
}

public interface IModelPerson : IModel
{
    void Add(Person person);

    void Remove(Person person);

    void Observe(IModelPersonObserver obsever);
}


ModelPerson

public class ModelPerson : IModelPerson
{
    private readonly ArrayList persons = new ArrayList();

    public event ModelHandler change;

    public void Add(Person person)
    {
        this.persons.Add(person);
        this.change.Invoke(this, new ModelEventArgs(person));
    }

    public void Remove(Person person)
    {
        this.persons.Remove(person);
    }

    public void Observe(IModelPersonObserver obsever)
    {
        this.change += obsever.Add;
    }
}


IView

public interface IView
{
    Controller Controller { set; }    
}


IViewPerson

```
public delegate void ViewHandler(IViewPerson sender, ViewEventArgs e);

public class ViewEventArgs: EventArgs
{
public Person person;
public ViewEventArgs(Person

Solution

public interface IView
{
    Controller Controller { set; }    
}


NO!!

You have just coupled the View with the Controller, which defeats the entire purpose.

This is also broken:

public abstract class Controller
{
    protected readonly IModel model;

    protected readonly IView view;

    protected Controller(IModel model, IView view)
    {


Now you have a circular dependency, with the controller knowing about the view, and the view knowing about the controller - but the controller knowing about the view and model is fine:

I don't understand this line:

Thread.Sleep(5000);


Why sleep for five whole seconds?

I'll just add that the naming convention, as noted by BCdotNET, is off: ControllerPerson should be PersonController - not because that's how ASP.NET MVC has it, but because C# and the .NET framework in general does it that way; it's just a matter of consistency... and it's what someone maintaining your code would expect to see.

Code Snippets

public interface IView
{
    Controller Controller { set; }    
}
public abstract class Controller
{
    protected readonly IModel model;

    protected readonly IView view;

    protected Controller(IModel model, IView view)
    {
Thread.Sleep(5000);

Context

StackExchange Code Review Q#66471, answer score: 5

Revisions (0)

No revisions yet.