Calculating Earnings from attendance details

In a C# win forms payroll application I need to calculate employee salary based on their attendance. I've written a stored procedure which calculates earnings of all employees from the data available in attendance table. So when the application calls the SP the data will be shown on a data grid view and then immediately these data will be saved in SalaryTrans table.

Please note in following demo code I've excluded the use of Interfaces and DI.

Since this is an object oriented development employing MVP design pattern, could you please advice me whether the way I've done this acceptable?

I know that I've not used a Model, in this case SalaryModel as the DataService return a DataSet object with all earnings figures of all employees. So Could you please show me a way of using a Model (in this case SalaryModel) in this scenario if possible?

Form

public partial class frmSalary : Form
{
    public frmSalary()
    {
        InitializeComponent();
    }

    private void btnProcessEarning_Click(object sender, EventArgs e)
    {
        SalaryPresenter presenter= new SalaryPresenter();
        presenter.ShowEarnings();
        presenter.SaveEarnings();
    }
}

Presenter

class SalaryPresenter
{
    private void ShowEarnings()
    {
        _View.dgEmployeeSalary.DataSource = CalculateEarnings(dtpStartDate.Value,dtpEndDate.Value);
    }

    private void SaveEarnings()
    {
        _DataService.InsertEarnings(dgEmployeeSalary);
    }

    private DataTable CalculateEarnings(DateTime startDate, DateTime endDate)
    {
        return _DataService.Calculate(startDate, endDate);
    }
}

Data Service

```
class DataService
{
public DataTable Calculate( DateTime fromDate, DateTime toDate)
{
using (SqlConnection sqlConnection = new SqlConnection(db.GetConnectionString))
{
using (SqlCommand sqlCommand = new SqlCommand("dbo.sp_Earnings", sqlConnection))
{
sqlCommand.CommandType = Comm

General point

Is there a strong reason for going with MVP? One of the downsides of an MVP approach is that a lot of the nice data-binding facilities within the architecture get bypassed. A Presentation Model / Model-View-ViewModel (MVVM) approach is pushed for WPF and can be used with Winforms.

There is a nice write-up on MVP to be found at MVC vs. MVP.

Form / Presenter

Whichever flavour of MVx one goes with, a basic idea is that the View doesn't need to know about the Presenter (and vice versa). You mentioned excluding the interfaces and DI but in the Form code, it still needs to know about the Presenter class - its method signatures et al - and the Presenter knows about each of the controls on the Form.

Another way of structuring this is to create an interface

interface ISalaryFormView {
    event EventHandler RequestProcessEarnings;
    void SetSalaryDetails(IEnumerable salaryDetailsList);
    DateTime StartDateTime { get; set; }
    DateTime EndDateTime { get; set; }
}

(The SetSalaryDetail() method and StartDateTime, EndDateTime properties are part of the reason why MVVM is being pushed. A lot less plumbing code)

and have the Presenter take an ISalaryFormView as an argument

class SalariesPresenter {

    private readonly ISalaryForm _form;
    private readonly SalariesService _service;

    public SalariesPresenter(ISalaryForm form, SalariesService service) {
        _form = form;
        _service = service;
        _form.ProcessEarningsRequested += HandleProcessEarningsRequested;
    }

    private void HandleProcessEarningsRequested(object sender, EventArgs e) {
        _form.SetSalaryDetails(_service.GetSalaries(_form.StartDateTime, _form.EndDateTime));
    }
}

In the form, we now implement the interface and rout the click to raising the event

public partial class MainWnd : Form, ISalaryForm  {

    public MainWnd() {
       InitializeComponent();
    }

    private void btnProcess_Click(object sender, EventArgs e) {
        RaiseProcessEarningsRequested();
    }

    #region ISalaryForm

    public void SetSalaryDetails(IEnumerable salaryDetailsList) {
        dgvSalaries.DataSource = salaryDetailsList;
        dgvSalaries.Refresh();
    }

    public DateTime StartDateTime { 
        get { return dtpStartDate.Value; }
        set { dtpStartDate.Value = value; }
    }

    public DateTime EndDateTime {
        get { return dtpEndDate.Value; }
        set { dtpEndDate.Value = value; }
    }

    #region ProcessEarningsRequested

    public event EventHandler ProcessEarningsRequested;

    protected void RaiseProcessEarningsRequested() {
        OnRaiseProcessEarningsRequested(EventArgs.Empty);
    }

    protected virtual void OnRaiseProcessEarningsRequested(EventArgs e) {
        var handler = ProcessEarningsRequested;
        if (handler != null) {
            handler(this, e);
        }
    }

    #endregion

    #endregion

}

We do this to help the testing in the Presenter

• It doesn't need to know the concrete class of the Form - it is working against an interface.

• It doesn't need to know the final screen format - swap out a listview for the datagrid and it doesn't impact.

• We can unit test by having a Mock raise and event and verifying that the Setter is called.

DataService

It is a personal preference but I have a dislike of passing around data in DataTables, especially untyped datatables. A quick web search will give lots of reasons against doing so. Adding data containers and reading into them doesn't add that much work. That said, it is a personal preference. The datatable is a lot faster and doesn't impact the MVx shape at all.

Passing the DataGrid in as a parameter though is a bad idea. We have now a linkage to System.Windows.Controls that is totally unnecessary and impacts portability (say we want to use this on the server side of a Web app or migrate to WPF). Even changing the screen layout - changing to a DataGridView instead of a DataGrid is impacted. Passing in a IEnumerable<> of some Data Transfer Object (DTO) (or even a datatable) is a better idea. No layer behind the view should know about or be concerned with the screen controls.

EDIT

Generally I would return an IEnumerable rather than an IList but in the example I am working on this is giving strange results => the DataGridView is not refreshing correctly for IEnumerable but works fine for IList.

For the minimal example I have two columns, Name and Value,

I have a SalaryDetails class

public class SalaryDetails {
    public string Name { get; set; }
    public decimal Value { get; set; }
}

and the SalariesService class

```
public class SalariesService {

public IList GetSalaries(DateTime start, DateTime end) {
return ReadTable(start, end).Rows.OfType().Select(CreateSalaryDetails).ToList();
}

private static DataTable ReadTable(DateTime start, DateTime end) {
var ret = new DataTable();
ret.Columns.Add("Name", typeof (string));

StackExchange Code Review Q#49499, answer score: 4