I've got 17 currency fields on a form and need to do some calculations with their values. I wrote this method some time ago and have come back to try and refactor it to make it more efficient and/or more readable. Could I have some pointers please?
Note: The variables appended with d
are the decimals used to hold the field values, their counterparts without the d
are the fields themselves. I am aware that variables could do with renaming to make more sense. Hopefully it's fairly clear what this is doing by looking at the comments.
private void getTotals() {
//declarations of decimal variables
decimal curPurc1d; decimal curPurc2d; decimal curPurc3d; decimal curPurc4d; //purItemxCost
decimal curItem1Totd; decimal curItem2Totd; decimal curItem3Totd; decimal curItem4Totd; //curItemxTot
decimal LessItem1Costd; decimal LessItem2Costd; decimal LessItem3Costd; decimal LessItem4Costd; decimal LessItem5Costd; //LessItemxCost
decimal ditem1Cost = 0; decimal ditem2Cost = 0; decimal ditem3Cost = 0; decimal ditem4Cost = 0; //Full cost of items
//Check if we have some valid numbers, stop if we don't
if ( !decimal.TryParse(curPurc1.Value.ToString(), out curPurc1d)
|| !decimal.TryParse(curPurc2.Value.ToString(), out curPurc2d)
|| !decimal.TryParse(curPurc3.Value.ToString(), out curPurc3d)
|| !decimal.TryParse(curPurc4.Value.ToString(), out curPurc4d)
|| !decimal.TryParse(curItem1Tot.Value.ToString(), out curItem1Totd)
|| !decimal.TryParse(curItem2Tot.Value.ToString(), out curItem2Totd)
|| !decimal.TryParse(curItem3Tot.Value.ToString(), out curItem3Totd)
|| !decimal.TryParse(curItem4Tot.Value.ToString(), out curItem4Totd)
|| !decimal.TryParse(LessItem1Cost.Value.ToString(), out LessItem1Costd)
|| !decimal.TryParse(LessItem2Cost.Value.ToString(), out LessItem2Costd)
|| !decimal.TryParse(LessItem3Cost.Value.ToString(), out LessItem3Costd)
|| !decimal.TryParse(LessItem4Cost.Value.ToString(), out LessItem4Costd)
|| !decimal.TryParse(LessItem5Cost.Value.ToString(), out LessItem5Costd)
)
return;
//For each of the 4 'items', try to get the value as a decimal, but only if the related checkbox is checked.
if (Item1RateYN.Checked)
{
decimal.TryParse(curItem1Cost.Value.ToString(), out ditem1Cost);
}
if (Item2RateYN.Checked)
{
decimal.TryParse(curItem2Cost.Value.ToString(), out ditem2Cost);
}
if (Item3RateYN.Checked)
{
decimal.TryParse(curItem3Cost.Value.ToString(), out ditem3Cost);
}
if (Item4RateYN.Checked)
{
decimal.TryParse(curItem4Cost.Value.ToString(), out ditem3Cost);
}
//Add up some values which are always part of the subtotal and then the ditemx ones, which will be 0 or set to a numerical value depending if the checkbox is checked
decimal subtotals = curPurc1d + curPurc2d + curPurc3d + curPurc4d + curItem1Totd + curItem2Totd + curItem3Totd + curItem4Totd
+ ditem1Cost + ditem2Cost + ditem3Cost + ditem4Cost;
subtotal.Value = subtotals;
//Get total minus the cost of the property (curPurc1d)
costPlusSubTotal.Value = subtotals - curPurc1d;
//add up all the "less" items to know how much to reduce by
decimal lessTotals = LessItem1Costd + LessItem2Costd + LessItem3Costd + LessItem4Costd + LessItem5Costd;
totalLess.Value = lessTotals;
//Total Balance due
//subtotal minus the 'less' values total
decimal total = (subtotals - lessTotals);
//set the final figure into the relevant field
balanceDueTot.Value = total;
}