Validating a group box with 3 radio buttons

Question

I am getting the msgbox whether one of the boxes are selected or not. Please if anyone has an idea. I would be very thankful

Private Function ValidateDiscountSelected(ByRef blnDiscount As Boolean,
     ByRef strDiscount As String) As Integer

    '   This function confirms the user selected a discount available.
    Dim intDiscountSelected As Integer
    Try
        intDiscountSelected = Convert.ToInt32(grpDiscount.Controls)
        strDiscount = grpDiscount.Controls.ToString()
        blnDiscount = True
    Catch exception As SystemException
        '   Confirms a radio button was selected.
        MsgBox("Please Select If Discount Is Available", , "You must Choose A Discount If Avaiable")
        blnDiscount = False
    End Try
    Return intDiscountSelected

What does the debugger show you when you step through the code starting with `intDiscountSelected =`? Or if you comment out the `try catch` and let the exception be raised? What does the exception message tell you? — Ken White, Apr 20 '16 at 16:47
`grpDiscount.Controls` a controls collection is never going to be able to be converted to Int32 — Ňɏssa Pøngjǣrdenlarp, Apr 20 '16 at 16:50
hey all thank you for the response. when I run the debugger I am getting the message box stating that a discount has not been selected. — Miranda Lynn, Apr 20 '16 at 17:09
That is not an error message - it is your message. The exception will always be thrown because a controls collection (`grpDiscount.Controls`) cannot be converted to an integer, so an exception is always thrown. — Ňɏssa Pøngjǣrdenlarp, Apr 20 '16 at 17:11
I think you are right that the problem might be with grpDiscount.Controls. I have tried almost everything in intelisence and nothing is validating if a radio button has been selected. I hate to change everything and go to a lstbox. I am really lost with this one, — Miranda Lynn, Apr 20 '16 at 17:11

score 3 · Accepted Answer · edited May 23 '17 at 12:16

There are several things which can be improved in that code. First and foremost:

Convert.ToInt32(grpDiscount.Controls)

That code tries to convert an entire controls collection to an integer, which cannot ever work. As a result, the Exception is always thrown.

You apparently want to know which control is checked, but that alone will not tell you the Discount level desired: a control which says "2% Discount" is not the same a 2. A UI Control will tell you what the user wants, but you need some way to convert that back into something like a number.

Determine checked RB

Dim rb = GroupBox3.Controls.OfType(Of RadioButton).
                       FirstOrDefault(Function(w) w.Checked)

This will tell you which, if any, control is checked. If each control had the discount stored in the .Tag property (e.g. the "10% Discount" control had 10), you could get the amount back from the Checked one:

    If rb IsNot Nothing Then
        ' determine discount amount
        DiscountLevel = Convert.ToInt32(rb.Tag)
    End If

    Return DiscountLevel

If none are selected, then rb will be Nothing. That ought not ever happen - a group of RadioButtons should provide for a default.

Loop version

    Dim DiscountLevel As Int32 = -1
    For Each rbtn As RadioButton In GroupBox3.Controls.OfType(Of RadioButton)()
        If rbtn.Checked Then
            DiscountLevel = Convert.ToInt32(rb.Tag)
        End If
    Next

At the end, if one is checked, DiscountLevel will be >-1.

Exceptions

Exceptions are for exceptional situations, not flow control or data validation. Your code is actually chastising the user when your code fails. That code is not so complex or error prone that it needs a Try/Catch.

ComboBox

A ComboBox could do the same thing as a group of RadioButtons, but do so better and take less space. You could populate it with DiscountLevel objects, where the text says "10%" or whatever, while returning a numeric value to your code.

Basic class to keep a name/text and value together. This will display the name/text to the user, but return a value to your code.

score 0 · Answer 2 · answered Apr 20 '16 at 18:30

I wanted to leave this as a comment for comparison's sake, but I can't comment yet. For a selection between only 3 radiobuttons another alternative would be a simple If-ElseIf statement.

Dim intDiscountSelected As Integer = 0

If radioButton1.Checked = True Then
    intDiscountSelected = 1
    blnDiscount = True
ElseIf radioButton2.Checked = True Then
    intDiscountSelected = 2
    blnDiscount = True
ElseIf radioButton3.Checked = True
    intDiscountSelected = 3
    blnDiscount = True
End If

If intDiscountSelected = 0 Then
    MsgBox("Please Select If Discount Is Available", , "You must Choose A Discount If Available")
    blnDiscount = False
End If

That said, Plutonix's loop version is a more concise option that scales better.

Validating a group box with 3 radio buttons

2 Answers2