Populating a listbox in a mock invoice with selected items

Question

So I'm almost 100% certain I've done something wrong simply due to the inefficiency of what I've written, I'm entirely a novice to coding and am working on an assignment for my introductory programming course. The only way I could think to display the input from 4 check boxes was to go down 16 different branches to cover every possible input option, is there any simpler or quicker way to show the results on a listbox? This project is for a mock cruise invoice

 //Onboard services expense clarification//
 int finedining;
 finedining = int.Parse("200");
 int fitnesstrainer;
 fitnesstrainer = int.Parse("50");
 decimal excursions;
 excursions = decimal.Parse("399.9");
 decimal airporttransfer;
 airporttransfer = decimal.Parse("45.5");
 decimal noservices;
 noservices = decimal.Parse("0.00");
 //Check Box Data//
 if (diningCheckBox.Checked && !fitnessCheckBox.Checked && !excursionsCheckBox.Checked && !airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fine Dining");
 valueListBox.Items.Add(finedining);
 }
 else if (diningCheckBox.Checked && fitnessCheckBox.Checked && !excursionsCheckBox.Checked && !airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fine Dining, Fitness Trainer");
 valueListBox.Items.Add(finedining + fitnesstrainer);
 }
 else if (diningCheckBox.Checked && fitnessCheckBox.Checked && excursionsCheckBox.Checked && !airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fine Dining, Fitness Trainer, Excursions");
 valueListBox.Items.Add(finedining + fitnesstrainer + excursions);
 }
 else if (diningCheckBox.Checked && fitnessCheckBox.Checked && excursionsCheckBox.Checked && airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fine Dining, Fitness Trainer, Excursions, Airport Transfer");
 valueListBox.Items.Add(finedining + fitnesstrainer + excursions + airporttransfer);
 }
 else if (diningCheckBox.Checked && !fitnessCheckBox.Checked && excursionsCheckBox.Checked && airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fine Dining, Excursions, Airport Transfer");
 valueListBox.Items.Add(finedining + excursions + airporttransfer);
 }
 else if (diningCheckBox.Checked && fitnessCheckBox.Checked && !excursionsCheckBox.Checked && airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fine Dining, Fitness Trainer, Airport Transfer");
 valueListBox.Items.Add(finedining + fitnesstrainer + airporttransfer);
 }
 else if (diningCheckBox.Checked && !fitnessCheckBox.Checked && !excursionsCheckBox.Checked && airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fine Dining, Airport Transfer");
 valueListBox.Items.Add(finedining + airporttransfer);
 }
 else if (diningCheckBox.Checked && !fitnessCheckBox.Checked && excursionsCheckBox.Checked && !airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fine Dining, Excursions");
 valueListBox.Items.Add(finedining + excursions);
 }
 else if (!diningCheckBox.Checked && fitnessCheckBox.Checked && excursionsCheckBox.Checked && airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fitness Trainer, Excursions, Airport Transfer");
 valueListBox.Items.Add(fitnesstrainer + excursions + airporttransfer);
 }
 else if (!diningCheckBox.Checked && fitnessCheckBox.Checked && excursionsCheckBox.Checked && !airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fitness Trainer, Excursions");
 valueListBox.Items.Add(fitnesstrainer + excursions);
 }
 else if (!diningCheckBox.Checked && fitnessCheckBox.Checked && !excursionsCheckBox.Checked && airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fitness Trainer, Airport Transfer");
 valueListBox.Items.Add(fitnesstrainer + airporttransfer);
 }
 else if (!diningCheckBox.Checked && fitnessCheckBox.Checked && !excursionsCheckBox.Checked && !airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Fitness Trainer");
 valueListBox.Items.Add(fitnesstrainer);
 }
 else if (!diningCheckBox.Checked && !fitnessCheckBox.Checked && excursionsCheckBox.Checked && airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Excursions, Airport Transfer");
 valueListBox.Items.Add(excursions + airporttransfer);
 }
 else if (!diningCheckBox.Checked && !fitnessCheckBox.Checked && excursionsCheckBox.Checked && !airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Excursions");
 valueListBox.Items.Add(excursions);
 }
 else if (!diningCheckBox.Checked && !fitnessCheckBox.Checked && !excursionsCheckBox.Checked && airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("Airport Transfer");
 valueListBox.Items.Add(airporttransfer);
 }
 else if (!diningCheckBox.Checked && !fitnessCheckBox.Checked && !excursionsCheckBox.Checked && !airportCheckBox.Checked)
 {
 titlesListBox.Items.Add("No onboard services requested");
 valueListBox.Items.Add(noservices);``

• Mathieu Guindon: Where is this code located exactly? It seems to be living in a form's code-behind.. is it in some Click handler for some Ok button?
• Jason: The code is from a button click to compile the results of all imputed data onto a listbox. If this question isn't intended for this site I'm sorry I'm not quite sure where to ask it nor where to find the answer

Answer 1

From the comments:

The code is from a button click to compile the results of all imputed data onto a listbox.

It's not the job of a button's Click handler to to all that work... and the work in question should be much, much simpler ;-)

How?

Your approach is very prone to bugs and errors, and writing a test to programmatically ensure that this complicated logic works as intended, is pretty much impossible.

Create an abstraction to represent any of the items - think in terms of objects here, ask yourself what's common to all these things? - the answer is probably something along these lines:

public class PackageItem
{
 public PackageItem(string description, decimal price)
 { 
 Description = description;
 Price = price;
 } 
 bool IsIncluded { get; set; }
 string Description { get; set; }
 decimal Price { get; set; }
}

You'd have a class like this for each item that's possible to select - notice how the type of Price doesn't need to change from int to decimal if you go on a 30% sale.. don't pick a type for its value, pick a type for its usage: currency should be decimal, regardless of whether the value is 200 or 199.99.

By using a class to encapsulate this concept, you're making sure that the Price is always going to be a decimal - that will avoid quite a few headaches when comes the time to tally up the bill.

Then you would make a "model" for your form, that would include a list of all possible items:

private PackageItem _excursions = new PackageItem("Excursion",399.9);
private PackageItem _fineDining = new PackageItem("Fine dining", 200);
private PackageItem _fitnessTraining = new PackageItem("Fitness trainer", 50);
private PackageItem _airportTransfer = new PackageItem("Airport transfer", 45.5);

Now, when a checkbox' Checked value changes, you can toggle the corresponding item's IsIncluded property:

private void diningCheckBox_Checked(object sender, EventArgs e)
{
 _fineDining.IsIncluded = diningCheckBox.Checked;
}
private void fitnessCheckBox_Checked(object sender, EventArgs e)
{
 _fitnessTraining.IsIncluded = fitnessCheckBox.Checked;
}
//...

---

Your approach for adding the descriptions into the titlesListBox suggests that a simple TextBox would have worked just as well, since you're only ever adding a single string to that list.

Consider simply iterating your PackageItem objects and adding an item whenever it's included - your click handler could be as simple as this:

var items = new[] { _excursions, _fineDining, _fitnessTraining, _airportTransfer };
foreach (var item in items)
{
 if (item.IsIncluded)
 {
 titlesListBox.Add(item.Description);
 valueListBox.Add(item.Price);
 }
}

From there, computing the total value should be a breeze.

Notice that the click handler doesn't care about the checkboxes - it's only looking at abstractions representing whatever these checkboxes stand for.

It's not Model-View-Presenter yet, but it would be a good step in the right direction I think.

I've left out the "No services" item for you to figure out.