I am building an Automative application that lets the user select the options from the check boxes etc. and the price is shown at the end for charges.
Problem is that the way I completed this application is awkward. For example, see my if else statements they don't cover every aspect of the decision.
For example, here is the part of my code
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;
}
What if the user selects the radiatorRushCheckBox.Checked
option and some other options ONLY which are not it this method let's say oilChangeCheckBox.Checked
then how will I cover all of that decision. It will be a way too lengthy to make if else statements for all because who knows what the user selects.
Here is what happens if I select all the options then it doesn't show the correct price.
Here is the FULL application CODE:
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 void ClearOilLube()
{
oilChangeCheckBox.CheckState = CheckState.Unchecked;
lubeJobCheckBox.CheckState = CheckState.Unchecked;
}
private void ClearFlushes()
{
radiatorRushCheckBox.CheckState = CheckState.Unchecked;
transmissionFlushCheckBox.CheckState = CheckState.Unchecked;
}
private void ClearMisc()
{
inspectionCheckBox.CheckState = CheckState.Unchecked;
replaceMufflerCheckBox.CheckState = CheckState.Unchecked;
tireRotationCheckBox.CheckState = CheckState.Unchecked;
}
private void ClearOthers()
{
partsTextBox.Text = "";
laborTextBox.Text = "";
}
private void ClearFees()
{
serviceLaborAnsLabelBox.Text = "";
partsSummaryAnsLabelBox.Text = "";
taxPartsAnsLabelBox.Text = "";
totalFeesAnsLabelBox.Text = "";
}
private decimal TotalCharges()
{
decimal rushesVar = RushesMethod();
decimal oiLAndLubeVar = OilLubeCharges();
decimal miscVar = MiscMethod();
decimal partsLaborVar = PartsLaborMethod();
decimal storeTaxCharges = TaxCharges();
decimal totalSum;
decimal totalSum1;
totalSum1 = (rushesVar + oiLAndLubeVar + miscVar);
totalSum = (rushesVar + oiLAndLubeVar + miscVar + partsLaborVar);
partsSummaryAnsLabelBox.Text = partsLaborVar.ToString();
partsSummaryAnsLabelBox.Text = partsTextBox.Text;
serviceLaborAnsLabelBox.Text = "Total Services fee is " + " " + totalFeesAnsLabelBox.Text + " " + "and Labor amount is" + " " + laborTextBox.Text;
taxPartsAnsLabelBox.Text = storeTaxCharges.ToString();
return totalSum;
}
private decimal TaxCharges()
{
const decimal PARTS_TAX_VAR = 0.6m;
decimal storeTax;
decimal taxCal;
storeTax = decimal.Parse(partsTextBox.Text);
taxCal = PARTS_TAX_VAR * storeTax;
return taxCal;
}
private decimal PartsLaborMethod()
{
decimal PL=0m;
decimal labor;
decimal totalPL = 0m;
PL = decimal.Parse(partsTextBox.Text);
labor = decimal.Parse(laborTextBox.Text);
totalPL= PL* labor;
return totalPL;
}
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 (tireRotationCheckBox.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 totalStore= TotalCharges();
totalFeesAnsLabelBox.Text = totalStore.ToString();
}
private void clearButton_Click(object sender, EventArgs e)
{
ClearOilLube();
ClearFlushes();
ClearMisc();
ClearOthers();
ClearFees();
}
}
}
EDIT:
Here is what the question says,
The application should have the following value-returning methods:
• OilLubeCharges —Returns the total charges for an oil change and/or a lube job, if any.
• FlushCharges —Returns the total charges for a radiator flush and/or a transmission flush, if any.
• MiscCharges —Returns the total charges for an inspection, muffler replacement, and/or a tire rotation, if any.
• OtherCharges —Returns the total charges for other services (parts and labor), if any.
• TaxCharges —Returns the amount of sales tax, if any. Sales tax is 6% and is charged only on parts. If the customer purchases services only, no sales tax is charged.
• TotalCharges —Returns the total charges.
The application should have the following void methods, called when the user clicks the Clear button:
• ClearOilLube —Clears the check boxes for oil change and lube job.
• ClearFlushes —Clears the check boxes for radiator flush and transmission flush.
• ClearMisc —Clears the check boxes for inspection, muffler replacement, and tire rotation.
• ClearOther —Clears the text boxes for parts and labor.
• ClearFees —Clears the labels that display the labels in the section marked Summary
Finally, I did it by using the @OmegaMan's solution. I wanted to post the changes so that it can help anyone
I made changes to the TotalCharges() method. Compare the earlier one and this.
private decimal TotalCharges()
{
decimal total = 0.0m;
if ( inspectionCheckBox.Checked)
total += 15.00m;
if (replaceMufflerCheckBox.Checked)
total += 100.00m;
if (tireRotationCheckBox.Checked)
total += 20.00m;
if (oilChangeCheckBox.Checked)
total += 26.00m;
if (lubeJobCheckBox.Checked)
total += 18.00m;
if (radiatorRushCheckBox.Checked)
total += 30.00m;
if (transmissionFlushCheckBox.Checked)
total += 80.00m;
return total;
I also made changes to the Calculate button CLICK EVENT HANDLER:
private void calculateButton_Click(object sender, EventArgs e)
{
decimal totalStore= TotalCharges();
decimal taxCharge = TaxCharges();
totalFeesAnsLabelBox.Text = (totalStore + taxCharge).ToString();
taxPartsAnsLabelBox.Text = taxCharge.ToString();
partsSummaryAnsLabelBox.Text = partsTextBox.Text;
serviceLaborAnsLabelBox.Text = "The Total Service charges are" + totalStore + "and Labor is " + laborTextBox.Text;
Rest of it is same and the logic worked successfully. Thanks to all of you who contributed.
Sum up and centralize the totaling operation in a separate method and check the state each of the Checkboxes while doing the sum:
public decimal TotalCosts()
{
decimal total = 0.0m;
if ({Oil Changed Checked})
total += {Oil Cost};
if ({Transmission checked})
total += {Transmission total};
if ({Repair Clutch})
total += {Clutch Cost}; // Maybe call a separate method ClutchTotal()?
... { Do this for all check boxes }
return total;
}
Don't attempt to individually add things together for disparate operations (as you did with the both_var = radiatorRush_var + transmissionFlush_var;
, that is your confusion.
Finite State Machine
I mentioned a finite state machine logic which is useful in organizing any code.
Think of a vending machine, it has to take all different types of coins as well as dollar bills and it has certain states before it can provide product. By mapping out all states and then centralizing the code to handle each state, that will go far in make the code bug free and maintainable.
If one adds a dime into the machine, the state goes from Welcome
into summing up the current coins and dollars but not providing the product. That state is true until total > cost, then the Vend
state is hit which distributes the product. It doesn't go back to the Welcome
state until it finishes a final step of provide any monies if overpayment
.
By setting up states one can organize one's code to handle all situations as you are seeing in your app.
To minimize this, you can create a mapping of Checkbox to decimal (Price). When you need to calculate the price related to checked checkboxes you can do it using one line only.
So first, create a mapping variable and populate it at some point (for me i chose form load - You can do it in the constructor after InitializeComponent();
:
Note : Rename the checkboxes to your valid ones.
List<KeyValuePair<CheckBox, decimal>> checkboxPriceMapping;
private void Form1_Load(object sender, EventArgs e)
{
checkboxPriceMapping = new List<KeyValuePair<CheckBox, decimal>>
{
new KeyValuePair<CheckBox, decimal>(checkBox1, 26.0m),
new KeyValuePair<CheckBox, decimal>(checkBox2, 18.0m),
new KeyValuePair<CheckBox, decimal>(checkBox3, 15.0m),
new KeyValuePair<CheckBox, decimal>(checkBox4, 100.0m),
new KeyValuePair<CheckBox, decimal>(checkBox5, 20.0m),
new KeyValuePair<CheckBox, decimal>(checkBox6, 30.0m),
new KeyValuePair<CheckBox, decimal>(checkBox7, 80.0m),
};
}
Then, this is a simple method to calculate the total price of the checked controls:
private decimal CalculateTotalPrice()
{
return checkboxPriceMapping.Where(x => x.Key.Checked).Sum(x => x.Value);
}
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With