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

Calculate balance wages

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

Problem

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 BalanceWages()
{

var earningList = _DataService.GetEarningsList(); //DataService returns a List list
var deductionList = _DataService.GetDeductionList(); //DataService returns a List
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;

Solution

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.

Code Snippets

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;
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;
            ...
        }
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 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;

Context

StackExchange Code Review Q#51302, answer score: 13

Revisions (0)

No revisions yet.