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

Using model view presenter (MVP) in C# code

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

Problem

I'm a beginner to Model-View-Presenter pattern and I'm finding a way to use it in a sample application

In my C# winforms application has a Employee Class and it has properties like EmployeeID,Name, Address, Designation etc. Also it has behaviors like viewEmployee(), AddNewEmployee(), PromoteEmployee() etc.

-
Could you please suggest how these things are organized in MVP pattern?

-
I have several entity classes in my project like Employee, Client, Attendance, Salary etc. So should I make separate DataService and Presenter classes for each one?

My current understanding is like this...

Implementing ViewEmployee()----------

Class Employee
{
    public string EmployeeID { get; set; }
    public string Name { get; set; }
    public string Address { get; set; }
    public string Designation { get; set; }
}

Class EmployeePresenter
{
   public void ViewEmployee(string employeeID, frmEmployeeDetails _form)
   {
     Employee Emp= EmloyeeDataService.GetEmployeeData(employeeID);
     _form.txtName=Emp.Name;
     _form.txtAddress=Emp.Address;
     _form.txtDesignation=Emp.Designation;
   }
}

Class EmployeeDataService
{
    public static Employee GetEmployeeData(String employeeID)
    {
      //Get data from database
      // Set Properties of Employee
      //Return Employee
    }
}

Partial Class frmEmployeeDetails : Form
{
    btnViewEmployee_Click() //Button Event
    {
      EmployeePresenter.ViewEmployee(txtEmployeeID.text,this);
    }
}

Solution

I think your question would be better if you fleshed it up with your actual working code, there's a lot of "placeholder" code in your post, which makes it border the line of off-topicness.

You may be able to find more information and high-level design guidance about MVP on Programmers.SE.

There isn't a lot to review here, but this is jumping at me:

public static Employee GetEmployeeData(String employeeID)


And then:

Employee Emp = EmloyeeDataService.GetEmployeeData(employeeID);


By doing that, you have tightly coupled the EmployeePresenter class to the EmployeeDataService class. The static method makes the ViewEmployee method harder to unit test - now whatever you do, if you call ViewEmployee you're going to call EmloyeeDataService.GetEmployeeData.

If you code to an abstraction instead of an implementation:

public interface IEmployeeRepository
{
    Employee GetById(string employeeId);
}

public class EmployeeDataService : IEmployeeRepository
{
    public Employee GetById(string employeeId)
    {
        // do whatever it takes to return an Employee object.
    }
}


Here you're accessing an SQL database with ADO.NET, but then later you might want to implement the same interface with Linq-to-SQL, or with Entity Framework - actually, the data could just as well be in XML files on the file system, or on Azure (cloud), or retrieved with some web service - all you need to care is the object's interface, which takes an employeeId and returns an Employee. Exactly how that's done, is irrelevant to the Presenter:

class EmployeePresenter
{
    private readonly IEmployeeRepository _service;

    public EmployeePresenter(IEmployeeRepository service)
    {
        _service = service;
    }
}


Now whoever is instantiating an EmployeePresenter will also need to instantiate some implementation of IEmployeeRepository, and inject that implementation into the presenter's constructor; you're no longer strongly coupled with a specific implementation, and you can change that anytime, without rewriting anything in the EmployeePresenter code.

This is more about SOLID and DI/IoC than MVP. I suggest you look at this read from Martin Fowler about GUI architectures, there's a section on MVP.

However I can tell you that this isn't MVP:

partial class frmEmployeeDetails : Form
{
    btnViewEmployee_Click() //Button Event
    {
        EmployeePresenter.ViewEmployee(txtEmployeeID.text,this);
    }
}


The Form shouldn't know anything about a presenter, or any repository or service - it's just a view. Instead, you fire events that the presenter can listen to, and respond. Shortly put, the Hollywood Principle: don't call them, they'll call you.

Code Snippets

public static Employee GetEmployeeData(String employeeID)
Employee Emp = EmloyeeDataService.GetEmployeeData(employeeID);
public interface IEmployeeRepository
{
    Employee GetById(string employeeId);
}

public class EmployeeDataService : IEmployeeRepository
{
    public Employee GetById(string employeeId)
    {
        // do whatever it takes to return an Employee object.
    }
}
class EmployeePresenter
{
    private readonly IEmployeeRepository _service;

    public EmployeePresenter(IEmployeeRepository service)
    {
        _service = service;
    }
}
partial class frmEmployeeDetails : Form
{
    btnViewEmployee_Click() //Button Event
    {
        EmployeePresenter.ViewEmployee(txtEmployeeID.text,this);
    }
}

Context

StackExchange Code Review Q#47068, answer score: 3

Revisions (0)

No revisions yet.