patterncsharpModerate
Calculate balance wages
Viewed 0 times
balancecalculatewages
Problem
In the following code I'm trying to calculate Net Wage of employees and classes
Is the way I've implemented the relationships correct? Would you have a method to calculate
```
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;
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
This:
Should look like this
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
can be easier read when written like this:
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
This means that the
formatting inside your
foreach should be indentedThis:
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 objectThis 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.