How can we accommodate two or more method call statements into one Click Event Handler?

Question

I'm making an application in which a user selects one or two options using checkbox and then click the calculate button and the total price is shown

For this, I MUST use my own method for example, for the Oil and Lube part and Rushes and call it using the click event handler to display the value in the label box.

I can easily call one method and display its value in the label box, but if I accommodate another call statement then the logical error comes i.e first call statement is ignored and the second call statement's values are displayed in the label box incorrectly.

Here are my two methods:

private decimal RushesMethod(out decimal radiatorRush_var, out decimal transmissionFlush_var, out decimal both_var)
{

    radiatorRush_var = 0m;
    transmissionFlush_var = 0m;
    both_var = 0m;

    if (radiatorRushCheckBox.Checked)
    {
        radiatorRush_var = 30.00m;  
    }
    else if (transmissionFlushCheckBox.Checked)
    {
        transmissionFlush_var = 80.00m;  
    }
    else if (radiatorRushCheckBox.Checked && transmissionFlushCheckBox.Checked)
    {
        radiatorRush_var = 30.00m;
        transmissionFlush_var = 80.00m;            
        both_var = radiatorRush_var + transmissionFlush_var;
    }

    return both_var + transmissionFlush_var + radiatorRush_var;   
}

private decimal OilLubeCharges(out decimal ValueTotal, out decimal ValueOilChange, out decimal ValueOilLube )
{

    ValueTotal = 0m;
    ValueOilChange = 0m;
    ValueOilLube = 0m;

    if (oilChangeCheckBox.Checked && lubeJobCheckBox.Checked)
    {
        ValueOilChange = 26.00m;
        ValueOilLube = 18.00m;
        ValueTotal = ValueOilChange + ValueOilLube;
    }
    else if (oilChangeCheckBox.Checked)
    {
        ValueOilChange = 26.00m;
    }
    else if (lubeJobCheckBox.Checked)
    {
        ValueOilLube = 18.00m;
    }

    return ValueOilLube +ValueOilChange+ValueTotal;             
}  

Here is the calculate event button handler:

private void calculateButton_Click(object sender, EventArgs e)
{           

    decimal totalOilLubeChargesAccept_var;
    decimal oilChangeAccept_var;
    decimal oilLubeChangesAccept_var;

    decimal storeValue = OilLubeCharges(out totalOilLubeChargesAccept_var, out oilChangeAccept_var, out oilLubeChangesAccept_var );

    totalFeesAnsLabelBox.Text = storeValue.ToString();

    decimal radiatorRush_var;
    decimal transmissionFlush_var;
    decimal bothRadAndTran;

    decimal storeRadTran = RushesMethod(out radiatorRush_var, out transmissionFlush_var, out bothRadAndTran);

    totalFeesAnsLabelBox.Text = storeRadTran.ToString();           
}  

You can see above if I use ONLY OilLubeCharges for calling the first method it's absolutely fine, but the problem is when I include the second call method RushesMethod it ignores the first call method and use the second call method.

Even if I use ONLY RushesMethod that also it doesn't display the value if both Radiator Flush and Transmission Flush check button are checked. It only displays if either of them is checked. This is not true for the OilLubeCharges method that is if I use it alone it will display values in either of the cases.

Consider Only Oil and Lube , Rushes and Total Fees:

EDIT:

This is what happening after I used totalFeesAnsLabelBox.Text = (storeRadTran + storeValue).ToString(); I think IF statements are culprit.

HERE IS ANOTHER EDIT:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace Automative_APP
{
    public partial class automativeForm : Form
    {
        public automativeForm()
        {
            InitializeComponent();
        }



        private decimal MiscMethod()
        {
            decimal valueTotal2 = 0m;

            if (inspectionCheckBox.Checked && replaceMufflerCheckBox.Checked && tireRotationCheckBox.Checked)
            {
                valueTotal2 += (15.00m + 100.00m + 20.00m);
            }

            else if (inspectionCheckBox.Checked)
            {
                valueTotal2 += 15.00m;
            }

            else if (replaceMufflerCheckBox.Checked)
            {
                valueTotal2 += 100.00m;
            }

            else if (replaceMufflerCheckBox.Checked)
            {
                valueTotal2 += 20.00m;
            }

            return valueTotal2;


        }






          private decimal RushesMethod()
        {
            decimal valueTotal = 0m;

            if (radiatorRushCheckBox.Checked)
            {
                valueTotal += 30.00m;  
            }

            else if (transmissionFlushCheckBox.Checked)
            {
                valueTotal += 80.00m;  
            }

            else if (radiatorRushCheckBox.Checked && transmissionFlushCheckBox.Checked)
            {

                valueTotal += (80.00m + 30.00m);
            }

            return valueTotal;


        }


        private decimal OilLubeCharges()
        {

            decimal valueTotalOL=0m;

            if (oilChangeCheckBox.Checked && lubeJobCheckBox.Checked)
            {
                valueTotalOL += (26.00m + 18.00m);

            }

            else if (oilChangeCheckBox.Checked)
            {
                valueTotalOL += 26.00m;

            }

            else if (lubeJobCheckBox.Checked)
            {
                valueTotalOL += 18.00m;
            }

            return valueTotalOL;            

        }


        private void partsSummaryLabelBox_Click(object sender, EventArgs e)
        {

        }

        private void taxPartsLabelBox_Click(object sender, EventArgs e)
        {

        }

        private void exitButton_Click(object sender, EventArgs e)
        {
            this.Close(); //close the form
        }

        private void calculateButton_Click(object sender, EventArgs e)
        {
            decimal rushesVar = RushesMethod();
            decimal oiLAndLubeVar = OilLubeCharges();
            decimal miscVar = MiscMethod();
            decimal totalSum;

            totalSum = (rushesVar + oiLAndLubeVar + miscVar);

            totalFeesAnsLabelBox.Text = totalSum.ToString();






        }
    }
}

Answer 1

For calculating the price for the various sections (like for instance the oilchange) I would suggest the following

private decimal OilLubeCharges() 
{
    decimal ValueTotal=0m;

    if (oilChangeCheckBox.Checked) 
        ValueTotal += 26.00m;

    if (lubeJobCheckBox.Checked)
        ValueTotal += 18.00m;

    return ValueTotal;
}

I don't see the purpose of all those out variables, so I removed them. The only value of concern is the total sum. To calculate this, you just have to check if the various checkboxes are checked, and if yes, just add the respective value to the total sum. No need for all those if ... else if cascades.

Furthermore one case, you calculated

ValueTotal = ValueOilChange + ValueLubJob;

and a few lines later you returend

return ValueTotal + ValueOilChange + ValueLubeJob;

which is, if you consider the above calculation:

return ValueOilChange + ValueLubeJob + ValueOilChange + ValueLubeJob;

ie, twice the correct sum.

For the calculations in the eventhandler, you should calculate the sums for each of the various parts, like you already do. Then add up all those partial sums and set the textbox content to this sum.

private void calculateButton_Click(object sender, EventArgs e)
{           
    decimal storeValue = OilLubeCharges();
    decimal storeRadTran = RushesMethod();

    var totalSum = storeValue + storeRadTran;
    totalFeesAnsLabelBox.Text = storeRadTran.ToString(); 
}

EDIT

If you really need those outvariables, you have to calculate them correctly. Just assign them, if the respective checkbox is checked. The total is again, the sum of all parts. Again as an example:

private decimal OilLubeCharges(out decimal oilcharges, out decimal lubecharges) {
    oilcharges = 0m;
    lubecharges = 0m;

    if (oilChangeCheckBox.Checked)
        oilcharges = 26m;

    if (lubeJobCheckBox.Checked)
        lubecharges = 18m;

     //the total is the sum of oilcharges and lubecharges
    return oilcharges + lubecharges;
}

EDIT

You should REALLY get rid of all these if ... else if constructructs. They make your code more or less unmaintainable. What if you need an additional checkbox? You'll have to add a ton of new else if to cover all possible situations. Furthermore it's very error prone, as your code shows.

First problem: In your MiscMethod you are checking the replaceMufflerCheckBox twice but never the tireRotationCheckBox alone.

Second problem: What happens if inspectionCheckBox and replaceMufflerCheckBox are checked, but not tireRotationCheckbox? To handle such a constellation correctly with your beloved else if you would need the following:

if (a && b && c)
else if (a && b)
else if (a && c)
else if (b && c)
else if (a)
else if (b)
else if (c)

Now consider you have to add an additional condition d ... So you should REALLY, REALLY, get rid of the else if constructs and refactor your code like I've shown you above.

And your problem with Transmission flush is the same. Although, you have checked all possible combinations, you are doing it in the wrong order. If radiationRush is checked, it always executes the first case, no matter if transmissionFlush is checked or not. Because of the else if it does not check any of the other conditions any more.

Answer 2

You need to sum of two decimal variables first. (The comments already suggested it)

totalFeesAnsLabelBox.Text = (storeValue + storeRadTran).ToString();

To improve the logic. Use the same decimal variable. For example;

 decimal priceValue = 0;
 decimal totalOilLubeChargesAccept_var;
 decimal oilChangeAccept_var;
 decimal oilLubeChangesAccept_var;
 priceValue += OilLubeCharges(out totalOilLubeChargesAccept_var, out oilChangeAccept_var, out oilLubeChangesAccept_var );


 totalFeesAnsLabelBox.Text = storeValue.ToString();

 decimal radiatorRush_var;
 decimal transmissionFlush_var;
 decimal bothRadAndTran;
 priceValue += RushesMethod(out radiatorRush_var, out transmissionFlush_var, out bothRadAndTran);

 totalFeesAnsLabelBox.Text = priceValue.ToString(); 

You check if the checkbox is selected in your methods and if nothing calculated it will return 0.

Hope helps,

Answer 3

You update the same label text value twice :

 totalFeesAnsLabelBox.Text = storeValue.ToString();
 totalFeesAnsLabelBox.Text = storeRadTran.ToString(); 

Maybe this is what you are looking for(sum the decimal values then set the label's text to this sum) :

totalFeesAnsLabelBox.Text = (storeValue + storeRadTran).ToString();