Change RadioButton checkstate in event

Question

I'm migrating some legacy code from VB6 to VB.NET and I've run into an annoying issue. What's currently happening is the user is provided with RadionButton controls to denote hierarchy. When they select a value the code verifies that it's valid (can't have C be the child of A, has to be the child of B) and if it's not it returns the RadioButton to the original setting.

The issue is that when the function that does this returns to the event handler it returns the RadioButton state to how it was when it was clicked (User clicked C, code returns it to B, when function exits it returns to C firing the event that'll turn it to B again, etc., etc. causing an infinite loop). Is there a way to change which RadioButton is selected inside a function called byCheckedChanged` event and have it stick?

I know the better design is to disable the invalid RadioButton controls ahead of time, but this is how it's designed and my job is to get it working in a limited time frame, so I'm stuck with bad design now as opposed to good design later.

Code:

Private Sub optIndent_2_ClickEvent(sender As Object, e As EventArgs) Handles optIndent_2.CheckedChanged
    optIndent_Click(2, sender.Checked)
End Sub

Private Sub optIndent_3_ClickEvent(sender As Object, e As EventArgs) Handles optIndent_3.CheckedChanged
    optIndent_Click(3, sender.Checked)
    Dim i As Integer = 1
End Sub

Private Sub optIndent_Click(ByRef Index As Short, ByRef value As Short)
    If value = False Then
        Exit Sub
    End If

    If Index = 2 Then
        Exit Sub
    End If
    If Index = 3 Then
        optIndent_2.Checked = True
        Exit Sub
    End If
End Sub

You'll see that when the code exits optIndent_Click (3, sender.checked) the value will transition from false to true, the process will repeat forever.

Answer 1

The problem is the use of ByRef:

Specifies that an argument is passed in such a way that the called procedure can change the value of a variable underlying the argument in the calling code.

Instead you should use ByVal:

Specifies that an argument is passed in such a way that the called procedure or property cannot change the value of a variable underlying the argument in the calling code.

Changing the argument value to be a ByVal argument will fix the problem. This will stop value from retaining its "value".

I appreciate that you're stuck with the bad design so this may not fit into this projects scope but at some point you should look at turning Option Strict On:

Restricts implicit data type conversions to only widening conversions, disallows late binding, and disallows implicit typing that results in an Object type.

This would have highlighted ByVal value As Short as being an issue:

Option Strict On disallows implicit conversions from 'Boolean' to 'Short'.

The fix would be; ByVal value As Boolean.

Having Option Strict On would also highlight sender.Checked as being an error:

Option Strict On disallows late binding

The fix would be to cast sender as a RadioButton so you can directly access its properties; DirectCast(sender, RadioButton).Checked.

Your code would look something like:

Private Sub optIndent_2_ClickEvent(sender As Object, e As EventArgs) Handles optIndent_2.CheckedChanged
    optIndent_Click(2, DirectCast(sender, RadioButton).Checked)
End Sub

Private Sub optIndent_3_ClickEvent(sender As Object, e As EventArgs) Handles optIndent_3.CheckedChanged
    optIndent_Click(3, DirectCast(sender, RadioButton).Checked)
End Sub

Private Sub optIndent_Click(ByVal Index As Short, ByVal value As Boolean)
    If value = False Then
        Exit Sub
    End If

    If Index = 2 Then
        Exit Sub
    End If

    If Index = 3 Then
        optIndent_2.Checked = True
        Exit Sub
    End If
End Sub