1
\$\begingroup\$

I have some currency fields which need to be added together. However, some of them have a checkbox nearby. If this is ticked, the field is added to the subtotal, if not, nothing is added.

I'm currently using the code below, which works, but doesn't seem very elegant or easy to maintain. I'm sure there is an easier way but I'm struggling to think of any other methods to use. There will be more checkboxes added later on, which will only lengthen the code. The below example is for two checkboxes.

decimal ditem1Cost = 0;
bool item1Checked = false;
if (Item1RateYN.Checked) {
    if (decimal.TryParse(item1Cost.Value.ToString(), out ditem1Cost))
        item1Checked = true;
}

decimal ditem2Cost = 0;
bool item2Checked = false;
if (Item2RateYN.Checked) {
    if (decimal.TryParse(item2Cost.Value.ToString(), out ditem2Cost))
        item2Checked = true;
}
//Add up some values which are always part of the subtotal
decimal subtotals = val1 + val2 + val3 + val4 + val5 + val6 + val7 + val8;

//Only add the ones which are near the checkboxes, if the checkbox is checked
subtotals = item1Checked ? subtotals + ditem1Cost : subtotals;
subtotals = item2Checked ? subtotals + ditem2Cost : subtotals;
\$\endgroup\$
2
  • 1
    \$\begingroup\$ val1 to val8? No arrays? You've stripped your code so much that it doesn't make any sense anymore. \$\endgroup\$
    – t3chb0t
    Sep 11, 2017 at 17:56
  • \$\begingroup\$ Not stripped, that's how it actually is. There's only ever 8 values required and I get their values from controls on the form. As long as they all parse to decimal, the function continues. \$\endgroup\$ Sep 11, 2017 at 17:59

2 Answers 2

2
\$\begingroup\$

Lot of logic that is not necessary

x = x + 0

decimal ditem1Cost = 0;
decimal ditem2Cost = 0;
if (Item1RateYN.Checked) 
{
    decimal.TryParse(item1Cost.Value.ToString(), out ditem1Cost);
}
if (Item2RateYN.Checked) 
{
    decimal.TryParse(item2Cost.Value.ToString(), out ditem2Cost);
}
//Add up some values which are always part of the subtotal
decimal subtotals = val1 + val2 + val3 + val4 + val5 + val6 + val7 + val8 
                  + ditem1Cost + ditem2Cost;
\$\endgroup\$
1
  • \$\begingroup\$ This looks great. I've tried it out and it seems to do the trick, with far less code. Thanks! \$\endgroup\$ Sep 11, 2017 at 17:42
2
\$\begingroup\$
//Add up some values which are always part of the subtotal
decimal subtotals = val1 + val2 + val3 + val4 + val5 + val6 + val7 + val8;

decimal ditem1Cost;
if (Item1RateYN.Checked) {
    decimal.TryParse(item1Cost.Value.ToString(), out ditem1Cost); 
    subtotals+=ditem1Cost;
}
decimal ditem2Cost;
if (Item2RateYN.Checked) {
    decimal.TryParse(item2Cost.Value.ToString(), out ditem2Cost);
    subtotals+=ditem2Cost;
}

Is slightly better.

If using C#7 remove decimal ditem1Cost = 0; and replace decimal.TryParse(item2Cost.Value.ToString(), out ditem1Cost) with decimal.TryParse(item2Cost.Value.ToString(), out decimal cost)

\$\endgroup\$
1
  • \$\begingroup\$ If not using C#7 remove = 0 (twice): out parameters are always overwritten. \$\endgroup\$ Sep 13, 2017 at 11:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.