Calculate balance wages

Question

In the following code I'm trying to calculate Net Wage of employees and classes Earning, Deduction and WageBalance has a composition relationship with WageInfo. And there is a class called WageManager which is having a association with WageInfo, handling all operations like BalanceWage(). DataService class encapsulates all db logic. Please review my code and let me know of any flaws.

Is the way I've implemented the relationships correct? Would you have a method to calculate BalanceWage in a more elegant way?

    class WageManager
    {
        WageInfo _WageInfo;
        DataService _DataService;


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


        public List<WageBalance> BalanceWages()
        {

            var earningList = _DataService.GetEarningsList(); //DataService returns a List<Earning> list
            var deductionList = _DataService.GetDeductionList(); //DataService returns a List<Deduction>
            int i = 0;

            foreach (Earning e in earningList)
            {

    // Sum all earnings...

            var totalEarnings = earningList[i].AmountForEPF + earningList[i].OverTimeAmount + earningList[i].IncentiveAllowance + earningList[i].SpecialAllowance + earningList[i].OtherAllowance + earningList[i].ExtraShiftAmount + earningList[i].BroughtForwardAmount;

    // Sum all deductions...

            var totalDeductions = deductionList[i].AdvanceAmount + deductionList[i].CarriedForwardAmount + deductionList[i].DeathDonationAmount + deductionList[i].EPFAmount + deductionList[i].FineAmount + deductionList[i].InsuranceInstalmentAmount + deductionList[i].LoanInstalmentAmount + deductionList[i].MealsAmount + deductionList[i].OtherDeductionAmount + deductionList[i].UniformInstalmentAmount + deductionList[i].WelfareAmount;
            var employeeID = earningList[i].EmployeeID;
            var netEarning = totalEarnings - totalDeductions;

            WageInfo wageInfo = new WageInfo();

            wageInfo.WageBalance.EmployeeID = employeeID;
            wageInfo.WageBalance.NetWage = decimal.Truncate(netEarning / 100) * 100;
            wageInfo.WageBalance.CarriedForwardAmount = netEarning - wageInfo.WageBalance.NetWage;

            _WageInfo.WageBalanceList.Add(wageInfo.WageBalance);
            i += 1;
        }

        _DataService.BalanceWages(_WageInfo.WageBalanceList);

        return _WageInfo.WageBalanceList;
    }
}


class WageInfo
{
    public DateTime WagePeriodStartDate { get; set; }
    public DateTime WagePeriodEndDate { get; set; } 
    public string Reference { get; set; }
    public Deduction Deduction = new Deduction();
    public Earning Earning = new Earning();
    public WageBalance WageBalance = new WageBalance();
    public List<WageBalance> WageBalanceList = new List<WageBalance>();

    public WageInfo()
    {
    }

}

public class WageBalance
{
    public int EmployeeID {get;set;}
    public string EmployeeName { get; set; }
    public decimal NetWage { get; set; }
    public decimal CarriedForwardAmount { get; set; }
}

Answer 1

It looks like you have a pair of lists which you iterate through based on the elements contained in the earningList. This is potentially dangerous without validation. When the earningList is longer than deductionList, you will run into an IndexOutOfRangeException.

var earningList = _DataService.GetEarningsList(); //DataService returns a List<Earning> list
var deductionList = _DataService.GetDeductionList(); //DataService returns a List<Deduction>

int i = 0;
foreach (Earning e in earningList)
{
    //...
    i += 1;
}

So in order to prevent that to happen, detect it in advance and handle it.

if (earningList.Count != deductionList.Count)
{
    //handle exception...
}

Since you dealing with two identical list, you can zip them together with Enumerable.Zip and loop through both at same time

var pairedList = earningList.Zip(deductionList, (e,d) => new { Earning = e, Deduction = d });
foreach(var pair in pairedList)
{
    //calculate wage with pair.Earning and pair.Deduction...
}

If I zip two list as above, is it possible to show these two lists in two DataGridViews?

Assume that we lost the reference to earningList and deductionList somehow and that you want to extract the original lists back from the pairedList to display each separately on DataGridView, it can be done with IEnumerable.Select :

//As dgv's DataSource doesn't take IEnumerable as valid source,
//we need to convert it to one of the interface it accepts : IList
dgvEarning.DataSource = pairedList.Select(p => p.Earning).ToList();
dgvDeduction.DataSource = pairedList.Select(p => p.Deduction).ToList();

Answer 2

Formatting

formatting inside your foreach should be indented

This:

        foreach (Earning e in earningList)
        {

// Sum all earnings...

        var totalEarnings = earningList[i].AmountForEPF + earningList[i].OverTimeAmount + earningList[i].IncentiveAllowance + earningList[i].SpecialAllowance + earningList[i].OtherAllowance + earningList[i].ExtraShiftAmount + earningList[i].BroughtForwardAmount;

Should look like this

        foreach (Earning e in earningList)
        {
            // Sum all earnings...
            var totalEarnings = earningList[i].AmountForEPF + earningList[i].OverTimeAmount + earningList[i].IncentiveAllowance + earningList[i].SpecialAllowance + earningList[i].OtherAllowance + earningList[i].ExtraShiftAmount + earningList[i].BroughtForwardAmount;
            ...
        }

---

When you are adding things together (either in concatenation or variable addition) you can multi-line the expression because C# is awesome and isn't NewLine Terminated, so this:

  var totalDeductions = deductionList[i].AdvanceAmount + deductionList[i].CarriedForwardAmount + deductionList[i].DeathDonationAmount + deductionList[i].EPFAmount + deductionList[i].FineAmount + deductionList[i].InsuranceInstalmentAmount + deductionList[i].LoanInstalmentAmount + deductionList[i].MealsAmount + deductionList[i].OtherDeductionAmount + deductionList[i].UniformInstalmentAmount + deductionList[i].WelfareAmount;

can be easier read when written like this:

  var totalDeductions = deductionList[i].AdvanceAmount +
      deductionList[i].CarriedForwardAmount + 
      deductionList[i].DeathDonationAmount + 
      deductionList[i].EPFAmount + 
      deductionList[i].FineAmount + 
      deductionList[i].InsuranceInstalmentAmount + 
      deductionList[i].LoanInstalmentAmount + 
      deductionList[i].MealsAmount + 
      deductionList[i].OtherDeductionAmount + 
      deductionList[i].UniformInstalmentAmount + 
      deductionList[i].WelfareAmount;

I am looking into making this easier to do as well, that just looks messy

---

Organization

The more that I look at your code, there are more things that I would change.

your Deduction and Earning objects are created and maintained by your WageInfo class so really you should be using a wageInfoList and each item in that list will have one Deduction object and one Earning object

This means that the WageInfo object should have all the information to hand off to the WageManager. The WageManager shouldn't directly see the Deduction or Earning objects that live in the WageInfo Object that is called by the WageManager.

Answer 3

It seems to me that you're hard-coding far too much here.

IMO, this type of task should be heavily database driven. For example, each item of earning and each deduction should have an amount, and a category. The category should come from a table in the database. So, you'd end up with one table of categories of deductions including the ones you have above (welfare, insurance, EPF, fine, meals, etc.) That table would probably have its permissions set so most normal users could only select from among the existing categories, but someone with administrative rights could add new ones. Of course, earnings would be pretty much the same way.

The code would then query the database for all deductions that apply to a particular employee during a specified time period, and add them together to get that employee's deductions for a pay period, total for the year, or whatever. This lets you (among other things) add or remove deductions without modifying the code.

The simple fact is that it's vain to even hope that when you write the code you've thought of every possible category of deduction somebody might want or need. Hard-coding leaves only two real choices: either resign yourself to modifying the code constantly forever, or force users to twist and stretch everything that arises to fit the categories that are currently available. In practice, such systems typically end up with some ugly combination of both.

Moving the categories to a database probably won't eliminate that--if somebody needs an administrator to add a category means somebody will inevitably find it more work than it's worth at least once in a while. Even if you let normal users add categories, some will still be too lazy to add a category sometimes¹. Nonetheless, with a reasonable system it takes only fairly minimal work on the part of a few people to keep the system working fairly smoothly, at least as a rule.

---

_{1. For comparison, I'd cite the tags system on this site (and most others like it). Yes, moderators need to edit tags on questions semi-regularly (hi @Jamal) -- but at least being an entry in a database table means they have the tools available to do that editing, without having to ask somebody at Stack Exchange to re-compile the code every time they need a new tag.}