Populating a listbox in a mock invoice with selected items

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.Ad

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.

StackExchange Code Review Q#118548, answer score: 2