Calculate subtotals and totals from a form with many decimal values 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;    
}
Two words: loops and arrays.

There a bunch of variable names that differ only by a number. Whenever you see code like this, you can probably clean it up by creating a collection of those things and looping over them. Basically, you have a checkbox and text field repeated multiple times, and one field where you display the total of all checked fields.


Create a user control to encapsulate the checkbox and text field
Make sure this user control has a public property decimal TotalCost { get } that will:


Return the decimal-parsed values of total - cost for the fields when checked, and zero when unchecked.
Throws an exception if the decimal cannot be parsed

Expose a boolean property bool IsValid that returns true when the user as entered a valid decimals
Expose a public property bool IsChecked that returns whether or not the checkbox is checked
Create a collection of these user controls, one for each checkbox/text field combo on screen


Now loop and process:

decimal purchaseTotal = 0m;
decimal totalAmount = 0m;

foreach (var control in PurchasedItems.Where(p =&gt; p.IsChecked &amp;&amp; p.IsValid))
{
    purchaseTotal += control.TotalCost;
}


I left some details out, but your code isn't really clear on what the UI looks like, or what the business use case is, but it really just boils down to:


Create an abstraction for what each combo of controls represents (e.g. create a new user control)
Create a collection of these controls
Loop and calculate