1

Written a quick subroutine in a class to move controls from one Panel to another in VB.NET, which seemed simple enough:

Public Sub Move(ByRef OldPanel As System.Windows.Forms.Panel)
    Dim panelControl As System.Windows.Forms.Control
    For Each panelControl In OldPanel.Controls
        MessageBox.Show(panelControl.Name) 'Debugging
        OldPanel.Controls.Remove(panelControl) 'Fairly certain this line makes no difference
        NewPanel.Controls.Add(panelControl)
    Next
End Sub

The problem is, it only moves about half the controls. The other panels aren't picked up by the loop at all and remain bound to OldPanel. I have verified that the controls are definitely part of the OldPanel (and not just visually floated above it).

For example, if there are 6 controls on the panel, MessageBox.Show(panelControl.Name) only feeds back 3 of them, and only those 3 controls move. This is... baffling.

I wrote a similar debugging loop inside the form class _Load event itself and this correctly picks up all 6 controls on the panel:

Dim panelControl As System.Windows.Forms.Control
For Each panelControl In Me.Panel1.Controls
    MessageBox.Show(panelControl.name)
Next

Any ideas?

Kai
  • 2,050
  • 8
  • 28
  • 46

2 Answers2

4

A common solution to this type of problem is to loop backwards over the collection. Then when you remove items it doesn't affect the index of the items you haven't seen yet:

Public Class Form1

    Private Sub Button1_Click(sender As System.Object, e As System.EventArgs) Handles Button1.Click
        MoveControls(Panel1, Panel2)
    End Sub

    Public Sub MoveControls(ByVal OldPanel As Panel, ByVal NewPanel As Panel)
        Dim ctlCount As Integer = OldPanel.Controls.Count - 1
        For i As Integer = ctlCount To 0 Step -1
            NewPanel.Controls.Add(OldPanel.Controls(i))
        Next
    End Sub

End Class
Idle_Mind
  • 38,363
  • 3
  • 29
  • 40
1

You are changing the collection while using for each to loop through it; that is asking for trouble: once the foreach is started and acquired the enumerator the enumerator is tied to the collection as it was at the start.

One way to solve this is by first looping and collecting a list of controls to be deleted.

Then loop the list and remove these controls.

Another way is to use for which doesn't create an enumerator.

Note that your code will not work if a control is nested within another control.

Emond
  • 50,210
  • 11
  • 84
  • 115
  • Sorted, cheers. I'll mark as answered in 5 mins. Could you expand on "your code will not work if a control is nested within another control"? Wouldn't moving the 'parent' control also move its children? – Kai May 13 '13 at 12:01
  • Yes it would, however if you do not want to move the parent control want to move a child control you would have to loop to the child controls of each control as well. Does that make sense? – Emond May 13 '13 at 12:07
  • Ah, I see. That's outside the scope of this function (it's supposed to just move everything), but good to know, thanks. – Kai May 13 '13 at 12:16
  • I thought the compiler threw an error when attempting to change a collection inside of a For Each loop. hmm – Tom F May 13 '13 at 16:13
  • 1
    @TomF - It should but it doesn't. Perhaps the controls collection is more forgiving than it should be. – Emond May 13 '13 at 16:37
  • He might not have Option Strict on or whatever the option is. – Tom F May 13 '13 at 19:27