0

I use a form that has controls that are dynamically added according to the class of object for which it is being used. In order to load the next object into the form it is necessary to delete and rebuild the dynamic part. I am experiencing random results when removing the objects using the code below, can you tell me where my error may be?

Sub CntrlKill(KillName As String)
    For Each c In Me.Controls
        If Strings.InStr(c.name, KillName) > 0 Then
            Me.Controls.Remove(c)
        End If
    Next
End Sub
Kenny
  • 3
  • 2
  • Where does `KillName` come from? Does it represent a partial match of a Control's `Name` property (so you should check `c.Name.Contains(KllName))`? Are all Controls child of `Me` (assuming `Me` is the instance of a Form)? -- BTW, to remove a Control, call `c.Dispose()`, not `Controls.Remove(c)` (unless these Controls are part of a collection of objects that is reused). -- Post the code that adds these Controls. – Jimi Jul 25 '21 at 13:27
  • Does this answer your question? [Removing controls in a loop](https://stackoverflow.com/questions/737005/removing-controls-in-a-loop) – GSerg Jul 25 '21 at 13:44
  • 1
    Does this answer your question? [foreach control c# skipping controls](https://stackoverflow.com/q/17627038/11683) – GSerg Jul 25 '21 at 13:44
  • 3
    Never use a For Each loop to enumerate a collection and then modify that collection inside the loop. That is always wrong. – jmcilhinney Jul 25 '21 at 13:47

1 Answers1

1

Translating the second link provided by GSerg to vb.net.

Create a list to hold the controls you want to delete. You can loop through the all the controls and add certain ones to a list. This does not effect the Controls collection. I haven't seen InStr used in a very long time. The .net .Contains will probably do what you need.

The second loop loops through the lstToDelete. This list is not effected by removing controls from the Controls collection.

The rule for For Each loops is don't effect the Collection you are looping through. You can change properties of the items in the collections. Just don't remove any items.

Sub CntrlKill(KillName As String)
    Dim lstToDelete As New List(Of Control)
    For Each c As Control In Controls
        If c.Name.Contains(KillName) Then
            lstToDelete.Add(c)
        End If
    Next
    For Each c In lstToDelete
        c.Dispose()
    Next
End Sub

Or the backwards loop way.

Private Sub NukeControls(KillString As String)
    For i = Controls.Count - 1 To 0 Step -1
        If Controls(i).Name.Contains(KillString) Then
            Controls(i).Dispose()
        End If
    Next
End Sub
Mary
  • 14,926
  • 3
  • 18
  • 27
  • 1
    A parallel collection of Controls, is not necessary. You need a backwards `for` and call `Dispose()` on a list of Controls that match the creteria. Don't call `Remove()`. A Control, when disposed of, will be also removed from the Controls collection. Calling `Remove()`, you just remove the Control from the collection, but the Control is not disposed. E.g., `dim controlsToRemove = Controls.OfType(Of Control).Where(Function(c) c.Name.Contains(KillName)).ToArray() for i as Integer = controlsToRemove.Length - 1 To 0 Step -1 controlsToRemove(i).Dispose() next` – Jimi Jul 26 '21 at 00:51
  • A `foreach` loop cannot be used because you modify the collection you're iterating over. Hence, whether you call `Remove()` or `Dispose()`, you remove or dispose of half the Controls. – Jimi Jul 26 '21 at 00:53
  • @Jimi I am not iterating over the Controls collection, I am iterating my List(OfT) – Mary Jul 26 '21 at 00:56
  • That's what the *A parallel collection of Controls is not necessary.* part is about. If you want to use another collection for some reason, you still need to call `Dispose()`, not `Remove()`, for the reason described. The difference becomes quite evident when you have Controls that contain graphic objects. None of these objects are disposed when you call `Remove()`. If you keep on adding/removing Controls without disposing of any, your app may (will) crash. Closing the Form won't help. – Jimi Jul 26 '21 at 01:09
  • Comments not necessarily directed to you. – Jimi Jul 26 '21 at 01:15
  • @Jimi I guess I think of a parallel collection in terms of parallel arrays where the index of the element of the arrays are related. I don't understand why you are using a backwards for loop over ControlsToRemove. – Mary Jul 26 '21 at 01:37
  • It just *enforces the rule*. This: `for each ctrl as Control In Me.Controls ctrl.Dispose() next` disposes of half of the Controls. This other: `for i as Integer = Controls.Count - 1 To 0 Step -1 Controls(i).Dispose() next` disposes of all of them. -- I.e., if you have a collection of disposable object, you may prefer to iterate it backwards, so you don't need to worry about the how the collection *behaves* when you dispose or remove an object from it. Use the first example I posted and check `Controls.Count` each time you dispose of one those Controls. – Jimi Jul 26 '21 at 02:15
  • You could also iterate forward and always dispose at `0`, but this recalculates the collection each time. Of course, the difference could be negligible if you have only a few Controls and the operation is not time-sensitive. – Jimi Jul 26 '21 at 02:18
  • 1
    Sorry Mary - new to this - yes it is helpful and I've just accepted it - many thanks. – Kenny Jul 27 '21 at 18:09