[RESOLVED] For Each error?

I think i got out of bed the wrong side.... My issue, i have a graphics object and i need to remove an item named "YearClock" before it is relaced with a new value.

My code

VB.Net Code:

For Each i As Canvas In cnvYearClock.Children
    If i.Name = "YearClock" Then
        cnvYearClock.Children.Remove(i)
    End If
Next
cnvYearClock.Children.Add(newCanvas)

when run i get an error that one of the other items contained in this object is not of type Canvas. now what is wrong with this? shouldn't this loop return only items of type canvas and ignore all other types?

This sounds rather like a WPF question, so I've asked the mods to move it to the WPF forum.

yes it is wpf jmcilhinney, kinda thought however it was a more general vb collection issue...

It is a more general collection issue. You've got two problems.

The first problem is explained by the error message: one of the items in the collection isn't a Canvas. Your For Each loop would be expanded by the compiler like this:

Code:

---

Dim iter as Enumerator = cnvYearClock.Children.GetEnumerator()
While iter.Next()
    Dim i As Canvas = CType(iter.Current, Canvas)
    If i.Name = "YearClock" Then
        ...
    End If
End While

---

Since one of the items is not a Canvas, the CType() cast fails. You're going to have to use a common ancestor type. If cnvYearClock is itself a Canvas, then Children is a UIElementCollection and thus UIElement is the only safe type for the loop variable. But it won't matter because of your second mistake.

You cannot modify a collection while you are enumerating it in a For Each loop. If you fix the last point, you'll find you get an exception that says this. The solution is to use a For...Next loop instead. It's safest to go in reverse when removing items:

Code:

---

For i As Integer = cnvYearClock.Children.Count - 1 To 0 Step -1
    Dim child As UIElement = cnvYearClock.Children(i)
    If child.Name="YearClock" Then
        cnvYearClock.Children.RemoveAt(i)
    End If
Next

---

excellent, indeed before your reply i had found a way using gettype and worked around it by exiting the loop, using a for loop will be an elegant solution i will implement immediately :)

Moved.

@Sitten Spynne, tried your method, says Name is not a member of UIElement.

here is my code, it works but can it be done another way?

vb Code:

For Each i In cnvYearClock.Children
            If i.[GetType]() = GetType(Canvas) Then
                cnvYearClock.Children.Remove(CType(i, UIElement))
                Exit For
            End If
        Next

seems to me to be overly complicated but at least it doesn't involve trees... also obviously this will only work when there is one canvas object in the collection, how could it be done when there are many of the required objects? how can i do it as per my initial request and delete the object with the name property?

I don't know how to stress it enough: you cannot modify a collection that is being iterated by a For Each loop. There is no way to make code of this form work:

Code:

---

For Each something In some_collection
    some_collection.Remove(anything)
' OR
    some_collection.Add(anything)
Next

---

It's illegal and will throw an exception. End of line. If you want it to work, go write your own programming language because this is part of the contract defined by IEnumerator (emphasis mine):

Quote:

---

The foreach statement of the C# language (for each in Visual Basic) hides the complexity of the enumerators. Therefore, using foreach is recommended instead of directly manipulating the enumerator.

Enumerators can be used to read the data in the collection, but they cannot be used to modify the underlying collection.

---

This is why I suggested a For Next loop. Since enumerators aren't used the restrictions of enumerators aren't in place.

It's not generally a good idea to completely toss out an example because you get a minor error. Let's go through the thought process I went to when deciding how to fix the code. As usual, the brain is more important than the clipboard.

• Huh, is Name really not a member of UIElement? I'll search Google for "msdn UIElement".
• Yep, Name isn't there.
• I could always use TryCast... maybe there's a different common ancestor. Let's look at Canvas's inheritance hierarchy; it's conveniently at the top.
• The hierarchy is ... -> UIElement -> FrameworkElement -> Panel -> Canvas. I know Panel's a bad idea because that's kind of specific. FrameworkElement is a pretty good choice; anything that displays has to derive from it. I wonder if it has a Name property? *click*
• Yep, FrameworkElement.Name.

Unfortunately I don't feel comfortable assuming that everything in Children can cast to FrameworkElement. I think it's probably true, but nothing guarantees it. You only care about a canvas with a specific name anyway, so it's not even worth being so generic. The TryCast() cast operator will attempt to perform a cast. If the cast doesn't work, it returns Nothing; if it works it returns the cast object. Combining this with a For Next loop should work; it's not a really big change.

Code:

---

For i As Integer = cnvYearClock.Children.Count - 1 To 0 Step -1
    Dim child As Canvas = TryCast(cnvYearClock.Children(i), Canvas)
    If child IsNot Nothing Then
        If child.Name="YearClock" Then
            cnvYearClock.Children.RemoveAt(i)
        End If
    End If
Next

---

If you want to get really fancy you can simplify it greatly with LINQ:

Code:

---

Dim target = cnvYearClock.Children.OfType(Of Canvas)().Where(Function(c) c.Name = "YearClock").FirstOrDefault()
If target IsNot Nothing Then
    cnvYearClock.Children.Remove(target)
End If

---

But if you don't understand how that works then it's wise to not use it. (Note it assumes there's only one to remove; I believe Name has to be unique within a namescope so it seems a safe assumption.)

Thanks again for the prompt reply. lol i guess i got lazy with it i had a working example of code which although not good logic or coding it none the less worked, then i had your code which after pasting didn't work. ironically i found it easier to post here than fix the issue myself lol.

using the for next loop as initially posted would have been great but casting is something i kinda shy away from due to not having become familiar with that side of the CLR yet (though that in itself is normally enough to make me wanna know...) but i digress...

I am indeed familiar with linq the project this is from uses it extensively. I'm kinda surprised that i didn't think of linq in the first place. I like the use of the lambda function in this code to find the required element.

I will give that a try.