Employee wage/salary calculation MVP solution - follow-up

Question

This is a follow-up to this post.

In this project (C# Win forms) I'm supposed to calculate wages of employees.

These are the steps:

1. Calculate earnings from Attendance data. Here all regular earnings like BasicSalary, OverTimeAmount etc will be calculated.

2. Then these earning details will be shown in a grid where user can enter values for blank columns ( non fixed earning figures like SpecialAllowance which is not attendance based ) (See class Earning)

3. Then user can save these earning details on a table called SalaryTrans. (Each record on the grid has a Save button.)

4. Calculate deduction. For all the records (employees) which is in SalaryTrans, deductions will be calculated. (see class Deduction)

5. Then these deduction details will be shown in a grid where user can enter non fixed / random deductions like OtherDeductions etc.

6. Then user can save these deduction details on the same table SalaryTrans. (Actually, in this case it updates deduction related columns of current records existing in the table.)

7. Then user can balance wages [BalanceWage = Earnings-Deduction].

8. After balancing, Net wage should be calculated deducting CarriedForwardAmount from BalanceWage. This carried forward amount is the less than 100 fraction of BalanceWage. (ex: if BalanceWage is 7,526.50 then CF amount is 26.50 and NetWage should be 7500.00)

The following diagram shows a portion of class relationships with regard to the above scenario.

The following code shows the solution I've implemented for the above case. This is the MVP pattern (for this demonstration I've removed interface usage and only the concrete classes are shown).

WageInfo has a one-to-many relationship with Earning, Deduction and WageBalance. Hence WageInfo maintains three list List<Earning>, List<Deduction> and List<WageBalance> as this is performed in 3 steps by the user.

Calculate Earnings Code Explanation

When requested, DataService returns a DataTable with all earings. Then WageManager passes it to WageInfo to fill the EarningList with data (Earning Objects) and then this list is returned back to WageManager where it is returned back to presenter to show the details on the grid.

Note: Earnings, Deductions and WageBalances are shown on separate grids.

Could you please review this code and let me know whether this is an acceptable solution?

    public class Earning 
    {
        public int EmployeeID { get; set; }
        public string Name { get; set; }
        public decimal BasicSalaryAmount { get; set; }
        public decimal BudjetoryAllowance { get; set; }
        public int WorkDays { get; set; }
        public int DayOffs { get; set; }
        public int LeaveDays { get; set; }
        public int AbsentDays { get; set; }
        public int ExtraShifts { get; set; }
        public int NoPayDays { get; set; }
        public decimal LessNoPayAmount { get; set; }
        public decimal AmountForEPF { get; set; }
        public decimal OverTimeAmount { get; set; }
        public decimal IncentiveAllowance { get; set; }
        public decimal SpecialAllowance { get; set; }
        public decimal OtherAllowance { get; set; }
        public decimal ExtraShiftAmount { get; set; }
        public decimal BroughtForwardAmount { get; set; }
        public string Remarks { get; set; }
    }


    class WageInfo
    {
        public DateTime WagePeriodStartDate { get; private set; }
        public DateTime WagePeriodEndDate { get; private set; } 
        public string Reference { get; private set; }

        private List<Deduction> DeductionList;
        private List<Earning> EarningList;
        private List<WageBalance> WageBalanceList;

        public WageInfo()
        {
            DeductionList = new List<Payroll.Deduction>();
            EarningList = new List<Payroll.Earning>();
            WageBalanceList = new List<WageBalance>();

        }


        public List<Earning> CreateEarningList(DataTable dt)
        {
            EarningList= dt.Rows.OfType<DataRow>().Select(InsertEarningsToList).ToList();
            return EarningList;

        }

        private Earning InsertEarningsToList(DataRow row)
        {
            return new Earning
            {

                EmployeeID = row["Emp_ID"] == DBNull.Value ? 0 : Convert.ToInt32(row["Emp_ID"]),
                WorkDays = row["work_days"] == DBNull.Value ? 0 : Convert.ToInt32(row["work_days"]),
                DayOffs = row["day_offs"] == DBNull.Value ? 0 : Convert.ToInt32(row["day_offs"]),
                LeaveDays = row["leave_days"] == DBNull.Value ? 0 : Convert.ToInt32(row["leave_days"]),
                ExtraShifts = row["extra_shifts"] == DBNull.Value ? 0 : Convert.ToInt32(row["extra_shifts"]),
                ExtraShiftAmount = row["Extra_Shift_Amount"] == DBNull.Value ? 0 : Convert.ToDecimal(row["Extra_Shift_Amount"]),
                BasicSalaryAmount = row["Basic_Sal"] == DBNull.Value ? 0 : Convert.ToDecimal(row["Basic_Sal"]),
                BudjetoryAllowance = row["budj_allowance"] == DBNull.Value ? 0 : Convert.ToDecimal(row["budj_allowance"]),
                NoPayDays = row["no_pay_days"] == DBNull.Value ? 0 : Convert.ToInt32(row["no_pay_days"]),
                LessNoPayAmount = row["less_no_pay_amt"] == DBNull.Value ? 0 : Convert.ToInt32(row["less_no_pay_amt"]),
                AmountForEPF = row["amt_for_epf"] == DBNull.Value ? 0 : Convert.ToDecimal(row["amt_for_epf"]),
                OverTimeAmount = row["over_time_amt"] == DBNull.Value ? 0 : Convert.ToDecimal(row["over_time_amt"]),
                BroughtForwardAmount = row["cf_amount"] == DBNull.Value ? 0 : Convert.ToDecimal(row["cf_amount"]),
                IncentiveAllowance = row["incentive_allowance"] == DBNull.Value ? 0 : Convert.ToDecimal(row["incentive_allowance"]),
                OtherAllowance = row["other_allowance"] == DBNull.Value ? 0 : Convert.ToDecimal(row["other_allowance"]),
                SpecialAllowance = row["special_allowance"] == DBNull.Value ? 0 : Convert.ToDecimal(row["special_allowance"])
            };
        }

        public Earning GetAnItemFromEarningsList(int index)
        {
            return EarningList[index];

        }
}


    class WageManager
    {
        WageInfo _WageInfo;
        DataService _DataService;


        public WageManager(WageInfo wageInfo, DataService dataService )
        {
            _WageInfo = wageInfo;
            _DataService = dataService;
        }

        public List<Earning> PrepareEarnings(WageInfo wageInfo)
        {
            var info = _DataService.GetEarnings(wageInfo.WagePeriodStartDate, wageInfo.WagePeriodEndDate);
            return wageInfo.CreateEarningList(info);
        }

        public void UpdateEarnings(WageInfo wageInfo, int row)
        {
            _DataService.InsertEarnings(wageInfo.GetAnItemFromEarningsList(row));

        }
    }


    class WagesPresenter : BasePresenter
    {
         WageInfo _WageInfo;
         frmWages _WageView;
         WageManager _WageManager;

         BindingSource bS = new BindingSource();

         public WagesPresenter(WageManager wageManger, WageInfo wageInfo, frmWages wageView  )
         {
             this._WageInfo = wageInfo;
             this._WageView = wageView;
             _WageManager = wageManger;  
         }

        private void _WageView_OnProcessingEarnings(object sender, EventArgs e)
        {
            ShowEarnings();
        }


        private void ShowEarnings()
        {
            bS.DataSource = _WageManager.PrepareEarnings(_WageInfo);

            _WageView.EarningDetails = bS;
        }
        }


    class DataService
    {
            public DataTable GetEarnings(DateTime fromDate, DateTime toDate)
            {
                using (SqlConnection sqlConnection = new SqlConnection(db.GetConnectionString))
                {
                    using (SqlCommand sqlCommand = new SqlCommand("dbo.sp_Earnings", sqlConnection))
                    {
                        sqlCommand.CommandType = CommandType.StoredProcedure;
                        sqlCommand.Parameters.Add("@fromDate", SqlDbType.DateTime).Value = fromDate;
                        sqlCommand.Parameters.Add("@toDate", SqlDbType.DateTime).Value = toDate;
                        using (SqlDataAdapter sqlAdapter = new SqlDataAdapter(sqlCommand))
                        {
                            using (DataSet dataSet = new DataSet())
                            using (DataTable dataTable = new DataTable())
                            {
                                sqlConnection.Open();
                                dataSet.Tables.Add(dataTable);
                                sqlAdapter.Fill(dataTable);
                                return dataTable;
                            }
                        }
                    }
                }
            }
    }

Answer 1

I don't know which variant of MVP you are attempting to use (Supervising Controller or Passive View), but this:

class WagesPresenter : BasePresenter
{
    frmWages _WageView;
}

...is incorrect.

In MVP, the Presenter interacts with the View via an interface. This allows the Presenter to be unit-tested more easily and also allows the implementation of the View to change independently of its behavior. So what you'll want is something like this:

interface IWagesView
{
    BindingSource EarningDetails { get; set; }
}

class frmWages : IWagesView
{
    public frmWages()
    {
        Presenter = new WagesPresenter(new WagesManager(), new WageInfo(), this);
    }

    public WagesPresenter Presenter { get; private set; }

    public BindingSource EarningDetails { get; set; }
}

class WagesPresenter : BasePresenter
{
    WageInfo _wageInfo;
    IWagesView _view;
    WageManager _wageManager;

    public WagesPresenter(WageManager wageManger, WageInfo wageInfo, IWagesView view)
    {
        _wageManager = wageManger;
        _wageInfo = wageInfo;
        _view = view;
    }
}

Also, use var wherever appropriate (at least in my opinion) to reduce code redundancy:

class DataService
{
    public DataTable GetEarnings(DateTime fromDate, DateTime toDate)
    {
        using (var sqlConnection = new SqlConnection(db.GetConnectionString))
        using (var sqlCommand = new SqlCommand("dbo.sp_Earnings", sqlConnection))
        {
            sqlCommand.CommandType = CommandType.StoredProcedure;
            sqlCommand.Parameters.Add("@fromDate", SqlDbType.DateTime).Value = fromDate;
            sqlCommand.Parameters.Add("@toDate", SqlDbType.DateTime).Value = toDate;

            using (var sqlAdapter = new SqlDataAdapter(sqlCommand))
            using (var dataSet = new DataSet())
            using (var dataTable = new DataTable())
            {
                sqlConnection.Open();
                dataSet.Tables.Add(dataTable);
                sqlAdapter.Fill(dataTable);

                return dataTable;
            }
        }
    }
}

EDIT: I would also refactor your Earning property assignments to use helper methods to reduce the amount of repetition you have when converting possibly-null DB values:

static class DbConvert
{
    static decimal ToDecimal(object value)
    {
        return value == DBNull.Value ? 0 : Convert.ToDecimal(value);
    }

    static int ToInt32(object value)
    {
        return value == DBNull.Value ? 0 : Convert.ToInt32(value);
    }

    // and other appropriate methods...
}

return new Earning
{
    EmployeeID = DbConvert.ToInt32(row["Emp_ID"]),
    ExtraShiftAmount = DbConvert.ToDecimal(row["Extra_Shift_Amount"])
    // and so on...
};