In this project (C# Win forms) I'm supposed to calculate wages of employees.These are the steps:Calculate earnings from Attendance data. Here all regular earnings like BasicSalary, OverTimeAmount etc will be calculated.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)Then user can save these earning details on a table called SalaryTrans. (Each record on the grid has a Save button.Calculate deduction. For all the records (employees) which is in SalaryTrans, deductions will be calculated. (see class Deduction)Then these deduction details will be shown in a grid where user can enter non fixed / random deductions like OtherDeductions etc.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.Then user can balance wages [BalanceWage = Earnings-Deduction]. 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)Following diagram shows a portion of class relationships with regard to the above scenario.Following code shows the solution I've implemented for the above case. This is 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 ExplanationWhen 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; }                    public Deduction Deduction;                    public List<Deduction> DeductionList;                    private Earning Earning;                    private List<Earning> EarningList;                    public WageBalance WageBalance;                    public List<WageBalance> WageBalanceList;                    public WageInfo()                    {                        Deduction = new Deduction();                        DeductionList = new List<Payroll.Deduction>();                        Earning = new Earning();                        EarningList = new List<Payroll.Earning>();                        WageBalance = new WageBalance();                        WageBalanceList = new List<WageBalance>();                    }                    public Earning AddEarning(int employeeID, int workDays, int dayOffs, int leaveDays, int extraShifts, decimal extraShiftsAmount, decimal basicSalary, decimal budjetoryAllowance,                         int noPayDays, decimal lessNoPayAmount, decimal amountForEpf, decimal overTimeAmount, decimal broughtForwardAmount, decimal incentiveAllowance, decimal otherAllowance, decimal specialAllowance)                    {                        Earning.EmployeeID = employeeID;                        Earning.WorkDays = workDays;                        Earning.DayOffs = dayOffs;                        Earning.LeaveDays = leaveDays;                        Earning.ExtraShifts = extraShifts;                        Earning.BasicSalaryAmount = basicSalary;                        Earning.BudjetoryAllowance = budjetoryAllowance;                        Earning.NoPayDays = noPayDays;                        Earning.LessNoPayAmount = lessNoPayAmount;                        Earning.AmountForEPF = amountForEpf;                        Earning.OverTimeAmount = overTimeAmount;                        Earning.ExtraShiftAmount = extraShiftsAmount;                        Earning.IncentiveAllowance = incentiveAllowance;                        Earning.OtherAllowance = otherAllowance;                        Earning.SpecialAllowance=specialAllowance;                        Earning.BroughtForwardAmount = broughtForwardAmount;                        return Earning;                    }                    public List<Earning> CreateEarningList(DataTable dt)                    {                        EarningList= dt.Rows.OfType<DataRow>().Select(InsertEarningsToList).ToList();                        return EarningList;                    }                    private Earning InsertEarningsToList(DataRow row)                    {                        WageInfo wi = new WageInfo();                        return wi.AddEarning(                        row[Emp_ID] == DBNull.Value ? 0 : Convert.ToInt32(row[0]),                        row[2] == DBNull.Value ? 0 : Convert.ToInt32(row[2]),                        row[3] == DBNull.Value ? 0 : Convert.ToInt32(row[3]),                        row[4] == DBNull.Value ? 0 : Convert.ToInt32(row[4]),                        row[5] == DBNull.Value ? 0 : Convert.ToInt32(row[5]),                        row[7] == DBNull.Value ? 0 : Convert.ToDecimal(row[7]),                        row[8] == DBNull.Value ? 0 : Convert.ToDecimal(row[8]),                        row[9] == DBNull.Value ? 0 : Convert.ToDecimal(row[9]),                        row[10] == DBNull.Value ? 0 : Convert.ToInt32(row[10]),                        row[11] == DBNull.Value ? 0 : Convert.ToDecimal(row[11]),                        row[12] == DBNull.Value ? 0 : Convert.ToDecimal(row[12]),                        row[13] == DBNull.Value ? 0 : Convert.ToDecimal(row[13]),                        row[14] == DBNull.Value ? 0 : Convert.ToDecimal(row[14]),                        row[15] == DBNull.Value ? 0 : Convert.ToDecimal(row[15]),                        row[16] == DBNull.Value ? 0 : Convert.ToDecimal(row[16]),                        row[15] == DBNull.Value ? 0 : Convert.ToDecimal(row[17])                        );                    }                    public Earning UpdateEarnings(int row)                    {                        return EarningList[row];                    }        class WageManager            {                WageInfo _WageInfo;                DataService _DataService;                public WageManager(WageInfo wageInfo, DataService dataService )                {                    _WageInfo = wageInfo;                    _DataService = dataService;                }                # region Earning                public List<Earning> PrepareEarnings(DateTime wagePeriodStartDate, DateTime wagePeriodEndDate)                {                    var info = _DataService.GetEarnings(wagePeriodStartDate, wagePeriodEndDate);                    return _WageInfo.CreateEarningList(info);                }                public void UpdateEarnings(int row)                {                    _DataService.InsertEarnings( _WageInfo.UpdateEarnings(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.WagePeriodStartDate, _WageInfo.WagePeriodEndDate);                _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;                        }                    }                }            }        }}
Employee wage/salary calculation MVP solution
c#;winforms;mvp
It seems there is enough missing here it is hard to determine intent of some of your code so I just made assumptions and changed as I thought some good practices would dictate the code to be.I think the usage of public members is really bad. It allows for anyone to modify them in any way they please. I always take the stance of build your classes as if someone is going to try to abuse them, either by passing in nulls, or overwriting public members. This forces you to think how to encapsulate as much data and only expose the bare minimums.From the code provided I couldn't see a reason for the public members of: DeductionList, EarningList and WageBalanceList. If you want to share those with people you could make them properties with a public get and a private set, but I wouldn't even expose the list unless you expose a readonly version, because if you return the list from the get someone can just add whatever earnings they please to the list and it will mutate the state inside the WageInfo class.The AddEarnings method has a horrid signature. Are there various earning scenarios they don't require as many parameters? Perhaps you could create a few that show more purposeful intent and require less parameters. My comment later in your code about the parsing of the row and calling AddEarning makes that extremely difficult to maintain. I also would suggest when you see repetition (the parsing) encapsulate in to a method.I also don't understand why you have public members of single instance of each of the Deduction, Earning and WageBalance classes. Looking at the code if the WageInfo AddEarning is called more than once you are changing the single instance of the member, is that correct? Furthermore, should AddEarning be just mutating the internal state of the WageInfo?Code cleaned up: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; }//delete the single instance of the classes and made these privateprivate 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 Earning AddEarning(int employeeID, int workDays, int dayOffs, int leaveDays, int extraShifts, decimal extraShiftsAmount, decimal basicSalary, decimal budjetoryAllowance,     int noPayDays, decimal lessNoPayAmount, decimal amountForEpf, decimal overTimeAmount, decimal broughtForwardAmount, decimal incentiveAllowance, decimal otherAllowance, decimal specialAllowance){    Earning = new Earning();    Earning.EmployeeID = employeeID;    Earning.WorkDays = workDays;    Earning.DayOffs = dayOffs;    Earning.LeaveDays = leaveDays;    Earning.ExtraShifts = extraShifts;    Earning.BasicSalaryAmount = basicSalary;    Earning.BudjetoryAllowance = budjetoryAllowance;    Earning.NoPayDays = noPayDays;    Earning.LessNoPayAmount = lessNoPayAmount;    Earning.AmountForEPF = amountForEpf;    Earning.OverTimeAmount = overTimeAmount;    Earning.ExtraShiftAmount = extraShiftsAmount;    Earning.IncentiveAllowance = incentiveAllowance;    Earning.OtherAllowance = otherAllowance;    Earning.SpecialAllowance=specialAllowance;    Earning.BroughtForwardAmount = broughtForwardAmount;    return Earning;}public List<Earning> CreateEarningList(DataTable dt){    EarningList= dt.Rows.OfType<DataRow>().Select(InsertEarningsToList).ToList();    //consider making this list read only like: EarningList.AsReadOnly()    return EarningList;}private Earning InsertEarningsToList(DataRow row){    WageInfo wi = new WageInfo();    //this is a wall of junk, it extremely hard to know which parameter is which in the method    return wi.AddEarning(    row[Emp_ID] == DBNull.Value ? 0 : Convert.ToInt32(row[0]),    GetIntOrDefault(row,2),    GetIntOrDefault(row,3),    GetIntOrDefault(row,4),    GetIntOrDefault(row,5),    GetDecimalOrDefault(row,6),    GetDecimalOrDefault(row,8),    GetDecimalOrDefault(row,9),    GetIntOrDefault(row,10),    GetDecimalOrDefault(row,11),    GetDecimalOrDefault(row,12),    GetDecimalOrDefault(row,13),    GetDecimalOrDefault(row,14),    GetDecimalOrDefault(row,15),    GetDecimalOrDefault(row,16),    GetDecimalOrDefault(row,17)    );}private int GetIntOrDefault(DataRow row, int index) {    return row[index] == DBNull.Value ? 0 : Convert.ToInt32(row[index]);}private int GetDecimalOrDefault(DataRow row, int index) {    return row[index] == DBNull.Value ? 0 : Convert.ToDecimal(row[index]);}public Earning UpdateEarnings(int row){    return EarningList[row];}}class WageManager{WageInfo _WageInfo;DataService _DataService;public WageManager(WageInfo wageInfo, DataService dataService ){    _WageInfo = wageInfo;    _DataService = dataService;}public List<Earning> PrepareEarnings(DateTime wagePeriodStartDate, DateTime wagePeriodEndDate){    var info = _DataService.GetEarnings(wagePeriodStartDate, wagePeriodEndDate);    return _WageInfo.CreateEarningList(info);}public void UpdateEarnings(int row){    _DataService.InsertEarnings( _WageInfo.UpdateEarnings(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.WagePeriodStartDate, _WageInfo.WagePeriodEndDate);    _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;                }            }        }    }}}