Code Logic problem using group boxes, text boxes and check boxes

[pickslides]

Hi all,

I have made a VB form for a pizza calculator
Users are to populate the size of pizza they want (Check Box 1, 2 &3) they can only click one
Which extra toppings they like (Combo Box 4, 5, 6 & 7- Multiple allowed)
Sides they want (Textbox 2, 3, 4) - enter a value
And a button to calculate the cost

For some reason the code does not always include all costs or parameters

Q

Public Class Form1
    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
        Dim TotalCost As Double
        TotalCost = 0

        If CheckBox1.Checked Then TotalCost = TotalCost + 7
        If CheckBox2.Checked Then TotalCost = TotalCost + 9
        If CheckBox3.Checked Then TotalCost = TotalCost + 10

        If CheckBox4.Checked Then TotalCost = TotalCost + 2
        If CheckBox5.Checked Then TotalCost = TotalCost + 2
        If CheckBox6.Checked Then TotalCost = TotalCost + 2
        If CheckBox7.Checked Then TotalCost = TotalCost + 2

        TotalCost = TotalCost + 2 * TextBox2.Text + 3 * TextBox3.Text + 5 * TextBox4.Text

        TextBox1.Text = TotalCost
    End Sub
End Class

 

[jmcilhinney]

You should be using RadioButtons for the size rather than CheckBoxes. CheckBoxes are independent of each other and so are appropriate for the toppings but RadioButtons specifically act as a group within the same container and will thus inherently prevent the user selecting more than one. A RadioButton has a Checked property so it works just like a CheckBox in code from that perspective. Because you know for a fact that, if one is checked, no others will be, you should be using If...ElseIf to test them rather than three independent If statements.

Ideally, you'd be using NumericUpDowns for the sides as well, rather than TextBoxes. That way, you ensure that the user can only enter a numeric value.

Give everything a proper name. Don't just accept the default names for controls because they are all but meaningless. If you have a CheckBox for salami then name it salamiCheckBox. Etc.

As for the problem, you need to debug your code. If you can't work out what's going wrong simply by reading your code, the next step is not to ask someone else to read it too. The next step is to debug it. Place a breakpoint on the first line of that code and then, when it gets hit, step through the code line by line. Before each step, work out exactly what it is that you expect to happen. After the step compare what actually did happen to your expectation. If they differ, you have found an issue and you can examine it more closely. If you get to the end and the code does exactly what you expect but you still get the wrong result then it's your expectations that are wrong and you need to reevaluate them. If you find a specific issue but don;t know how to fix it, then is the time to post for help, providing all the relevant information you learned when debugging.

On another small note, you may prefer writing it out in full if it's clearer to you but you can write X += Y instead of X = X + Y, which would make your code rather more succinct. There are a number of operators that can be combined with an assignment that way, including most arithmetic operators and also the string concatenation operator (&).

 

[jmcilhinney]

Another tip: given that it is for output rather than input, should you be using a TextBox for the total cost? A Label might be more appropriate but, if you do use a TextBox, at least set the ReadOnly property to True. Also, instead of having the $ symbol outside the control, you can make it part of the text inside the control by formatting your output:

TextBox1.Text = TotalCost.ToString("c")

If you want to assign a number where text is expected then you should be calling ToString anyway and you have to if you have Option Strict On, which it should always be. Unfortunately, many classes/courses/tutorials don't teach that and they do their students a disservice as a result. When you call ToString on a number, you can specify how it should be formatted. "C" or "c" means local currency format so, if you're in America or Australia or somewhere else that uses dollars, you'll get a $ symbol and probably two decimal places in the output text.