# Can this combo box code be more compact and prettier?

I have 3 combo boxes with the same 3 selections in each. If the user picks the same selection a 2nd time, the first combo box resets. I have 3 other sections that will do the same with about 15 combo boxes.

Is there a shorter/compact way to code what I'm doing?

Protected Sub ComBox_GER1_SelectedIndexChanged(sender As Object, e As EventArgs) Handles ComBox_GER1.SelectedIndexChanged
Dim ValueSelected = ComBox_GER1.SelectedIndex

If ComBox_GER2.SelectedIndex = ValueSelected Then
ComBox_GER2.ClearSelection()
End If
If ComBox_GER3.SelectedIndex = ValueSelected Then
ComBox_GER3.ClearSelection()
End If

End Sub

Protected Sub ComBox_GER2_SelectedIndexChanged(sender As Object, e As EventArgs) Handles ComBox_GER2.SelectedIndexChanged
Dim ValueSelected = ComBox_GER2.SelectedIndex

If ComBox_GER1.SelectedIndex = ValueSelected Then
ComBox_GER1.ClearSelection()
End If
If ComBox_GER3.SelectedIndex = ValueSelected Then
ComBox_GER3.ClearSelection()
End If
End Sub

Protected Sub ComBox_GER3_SelectedIndexChanged(sender As Object, e As EventArgs) Handles ComBox_GER3.SelectedIndexChanged
Dim ValueSelected = ComBox_GER3.SelectedIndex

If ComBox_GER1.SelectedIndex = ValueSelected Then
ComBox_GER1.ClearSelection()
End If
If ComBox_GER2.SelectedIndex = ValueSelected Then
ComBox_GER2.ClearSelection()
End If
End Sub

-
Please don't append edited code from answers. They may get reviewed, which could complicate the review process. You may ask a new question with the updated code when ready. But if this updated is showing errors, then it should be asked on Stack Overflow. –  Jamal May 20 at 14:33

Remove the Thread.Sleep calls. All of them. You don't need them. If two comboboxes have that value it's a whole second you're sleeping for no reason. ASP.NET event handlers run on the server side, which means the user is going to be waiting longer than he needs to, to get a response back from your code.

Now for your handlers you'll have to be careful about the Page Life Cycle sequence of events; I don't do ASP.NET very often (read: never) but I think you'll want to register your event handlers in the Page_Load handler. You'll want to register a handler per "section", not per combobox.

I'd make a method that takes a ComboBox instance and an IEnumerable<ComboBox>, and compares the values of the ComboBox with that of each item in the IEnumerable<ComboBox> (just realized that's the C# notation for generics.. make that (Of ComboBox)) that is not the same reference than ComboBox, and clear selection if the values match.

Private Sub ClearDuplicateSelections(comboBox As ComboBox, comboBoxes As IEnumerable(Of ComboBox))
For Each cb In comboBoxes
If Not cb.Equals(comboBox) Then 'there is a better way to do this but I don't VB much
If cb.SelectedIndex = comboBox.SelectedIndex Then cb.ClearSelection()
End If
Next
End Sub


Now all you need is to pass the method a combobox (the "sender") and all the comboboxes in the same "group" or "section". In C# I'd do it like this:

// cast the sender from Object to ComboBox:
var box = (ComboBox)sender;

// put all comboboxes of that section in an array:
var boxes = new[] { ComBox_GER1, ComBox_GER2, ComBox_GER3 };

ClearDuplicateSelections(box, boxes);


Side note, be careful with naming: ValueSelected is lying - it represents the selected index, not the selected value. I almost got caught.

-

It has been a long time since I've done VB, but here goes:

If you put the comboboxes in an array, you can use 1 handler for all

Protected Sub ComBox_SelectedIndexChanged(sender As Object, e As EventArgs) Handles ComBox_GER1.SelectedIndexChanged ComBox_GER2.SelectedIndexChanged ComBox_GER3.SelectedIndexChanged
Dim cb = CType(sender, ComboBox) 'cast to ComboBox
Dim ValueSelected = cb.SelectedIndex

For Each combo As ComboBox In comboboxes
If Not combo Is cb And combo.SelectedIndex = ValueSelected Then

you didn't indent any of the code inside your Sub and you should, I am going to edit this in because it is standard formatting for VB. and +1 –  Malachi May 20 at 16:34