strange problem For Each loop in Treeview
Hi,
I think the problem might be that your statement: For Each nodChild in MainForm.TreeView1.Nodes is wrong. This will direct the routine to start at the top of the tree each time. I would imagine you want something like: Option Explicit Dim sItems() As String Dim nItem As Long Private Sub Command1_Click() Dim i As Long nItem = 0 IterateTreeView tv1.Nodes(1) For i = 0 To UBound(sItems) Debug.Print sItems(i) Next i End Sub Private Sub Form_Load() ' Fill the tree for testing purposes Dim X As Node, Y As Node Set X = tv1.Nodes.Add(, , "N1", "N1") Set Y = tv1.Nodes.Add(X, tvwChild, "N11", "N11") tv1.Nodes.Add Y, tvwChild, "N111", "N111" tv1.Nodes.Add Y, tvwChild, "N112", "N112" Set X = tv1.Nodes.Add(, , "N2", "N2") tv1.Nodes.Add X, tvwChild, "N21", "N21" tv1.Nodes.Add X, tvwChild, "N22", "N22" End Sub Sub IterateTreeView(a_oNode As Node) Dim X As Node ReDim Preserve sItems(nItem) As String sItems(nItem) = a_oNode.Text Set X = a_oNode.Child If Not X Is Nothing Then nItem = nItem + 1 IterateTreeView X End If Set X = a_oNode.Next Do While Not X Is Nothing nItem = nItem + 1 IterateTreeView X Set X = X.Next Loop End Sub HTH Peter Beach "RB Smissaert" wrote in message ... Use a recursive Sub to traverse the nodes of a Treeview control from top to bottom. Noticed that there is a problem if 2 nodes in the Treeview have the same text. Rather than ending at the lowest node the sub returns to the higher node with the same text and the Sub doesn't exit. This are the 2 Subs that demonstrate it: Sub Tester(ByRef nodcurrent As Node, _ ByVal intdepth As Integer) Dim nodChild As Node If nodcurrent.Children 0 Then For Each nodChild In MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then MsgBox nodChild.Text Call Tester(nodChild, (intdepth + 1)) End If End If Next nodChild End If End Sub Sub DoTester() Tester MainForm.TreeView1.Nodes(1), 0 End Sub Is this a known bug, or am I doing something wrong? One way to solve this problem is to fill an array with the indeces of the visited nodes and make sure the same node doesn't get visited twice, but perhaps there is a better solution for this. Thanks for any advice. RBS |
strange problem For Each loop in Treeview
Peter,
The Sub is fine. It was just the typo as stated in my previous post. RBS "Peter Beach" wrote in message ... Hi, I think the problem might be that your statement: For Each nodChild in MainForm.TreeView1.Nodes is wrong. This will direct the routine to start at the top of the tree each time. I would imagine you want something like: Option Explicit Dim sItems() As String Dim nItem As Long Private Sub Command1_Click() Dim i As Long nItem = 0 IterateTreeView tv1.Nodes(1) For i = 0 To UBound(sItems) Debug.Print sItems(i) Next i End Sub Private Sub Form_Load() ' Fill the tree for testing purposes Dim X As Node, Y As Node Set X = tv1.Nodes.Add(, , "N1", "N1") Set Y = tv1.Nodes.Add(X, tvwChild, "N11", "N11") tv1.Nodes.Add Y, tvwChild, "N111", "N111" tv1.Nodes.Add Y, tvwChild, "N112", "N112" Set X = tv1.Nodes.Add(, , "N2", "N2") tv1.Nodes.Add X, tvwChild, "N21", "N21" tv1.Nodes.Add X, tvwChild, "N22", "N22" End Sub Sub IterateTreeView(a_oNode As Node) Dim X As Node ReDim Preserve sItems(nItem) As String sItems(nItem) = a_oNode.Text Set X = a_oNode.Child If Not X Is Nothing Then nItem = nItem + 1 IterateTreeView X End If Set X = a_oNode.Next Do While Not X Is Nothing nItem = nItem + 1 IterateTreeView X Set X = X.Next Loop End Sub HTH Peter Beach "RB Smissaert" wrote in message ... Use a recursive Sub to traverse the nodes of a Treeview control from top to bottom. Noticed that there is a problem if 2 nodes in the Treeview have the same text. Rather than ending at the lowest node the sub returns to the higher node with the same text and the Sub doesn't exit. This are the 2 Subs that demonstrate it: Sub Tester(ByRef nodcurrent As Node, _ ByVal intdepth As Integer) Dim nodChild As Node If nodcurrent.Children 0 Then For Each nodChild In MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then MsgBox nodChild.Text Call Tester(nodChild, (intdepth + 1)) End If End If Next nodChild End If End Sub Sub DoTester() Tester MainForm.TreeView1.Nodes(1), 0 End Sub Is this a known bug, or am I doing something wrong? One way to solve this problem is to fill an array with the indeces of the visited nodes and make sure the same node doesn't get visited twice, but perhaps there is a better solution for this. Thanks for any advice. RBS |
strange problem For Each loop in Treeview
Hi,
Glad you got it worked out. I'm still a bit puzzled by your code, as you seem to iterate through the entire tree each time, searching for your target node whereas it is surely more efficient simply to work down and across from the target node. The code I posted earlier only traverses the tree a single time which would seem to be more efficient. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, The Sub is fine. It was just the typo as stated in my previous post. RBS |
strange problem For Each loop in Treeview
Peter,
Will give your code a try. RBS "Peter Beach" wrote in message ... Hi, Glad you got it worked out. I'm still a bit puzzled by your code, as you seem to iterate through the entire tree each time, searching for your target node whereas it is surely more efficient simply to work down and across from the target node. The code I posted earlier only traverses the tree a single time which would seem to be more efficient. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, The Sub is fine. It was just the typo as stated in my previous post. RBS |
strange problem For Each loop in Treeview
Peter,
Given your code a try, but it doesn't work. UBound(sItems) gives 67000, which I presume is the limit of what the array can hold and it gets into an endless loop. The code I use now is actually very efficient as it passes every node only once. Have you tried both ways? RBS "Peter Beach" wrote in message ... Hi, Glad you got it worked out. I'm still a bit puzzled by your code, as you seem to iterate through the entire tree each time, searching for your target node whereas it is surely more efficient simply to work down and across from the target node. The code I posted earlier only traverses the tree a single time which would seem to be more efficient. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, The Sub is fine. It was just the typo as stated in my previous post. RBS |
strange problem For Each loop in Treeview
Hi,
Odd. My code worked fine for me. The point I was making was that I can't see the purpose of the lines: For Each nodChild in MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then which would seem to iterate through the tree until the target node is reached. But you already know what the target node is - it's either the first node (for the initial call) or the it's the node currently being inspected, so it seems to me that you don't need to go iterating through the tree looking for it. You can from the node recursively call the routine for the first child and then iterate through all its siblings. However programming is that art of achieving results, if your code works, stick with it. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, Given your code a try, but it doesn't work. UBound(sItems) gives 67000, which I presume is the limit of what the array can hold and it gets into an endless loop. The code I use now is actually very efficient as it passes every node only once. Have you tried both ways? RBS "Peter Beach" wrote in message ... Hi, Glad you got it worked out. I'm still a bit puzzled by your code, as you seem to iterate through the entire tree each time, searching for your target node whereas it is surely more efficient simply to work down and across from the target node. The code I posted earlier only traverses the tree a single time which would seem to be more efficient. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, The Sub is fine. It was just the typo as stated in my previous post. RBS |
strange problem For Each loop in Treeview
Hi Peter,
I formulated the same comment (others have also agreed) to the almost the same question a month ago and got the same answer.. Basically, it is NOT efficient to iterate through all nodes of a Tree structure AND ALSO having recursive calls on each iteration. http://groups.google.com/groups?hl=f...TNGP12.phx.gbl (the thread is interesting too as it replicates this one!) BTW, your code works fine for me too. :-) Regards, Daniel M. "Peter Beach" wrote in message ... Hi, Odd. My code worked fine for me. The point I was making was that I can't see the purpose of the lines: For Each nodChild in MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then which would seem to iterate through the tree until the target node is reached. But you already know what the target node is - it's either the first node (for the initial call) or the it's the node currently being inspected, so it seems to me that you don't need to go iterating through the tree looking for it. You can from the node recursively call the routine for the first child and then iterate through all its siblings. However programming is that art of achieving results, if your code works, stick with it. Regards, Peter Beach |
strange problem For Each loop in Treeview
Daniel,
What same question? I put the code exactly as in the posting and it doesn't work. Have you tried the code with a treeview where 2 nodes have the same .text? Have you actually tried the other code and did it actually show to be inefficient? In my Treeview the recursive Sub gets none more called than the number of nodes. RBS "Daniel.M" wrote in message ... Hi Peter, I formulated the same comment (others have also agreed) to the almost the same question a month ago and got the same answer.. Basically, it is NOT efficient to iterate through all nodes of a Tree structure AND ALSO having recursive calls on each iteration. http://groups.google.com/groups?hl=f...TNGP12.phx.gbl (the thread is interesting too as it replicates this one!) BTW, your code works fine for me too. :-) Regards, Daniel M. "Peter Beach" wrote in message ... Hi, Odd. My code worked fine for me. The point I was making was that I can't see the purpose of the lines: For Each nodChild in MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then which would seem to iterate through the tree until the target node is reached. But you already know what the target node is - it's either the first node (for the initial call) or the it's the node currently being inspected, so it seems to me that you don't need to go iterating through the tree looking for it. You can from the node recursively call the routine for the first child and then iterate through all its siblings. However programming is that art of achieving results, if your code works, stick with it. Regards, Peter Beach |
strange problem For Each loop in Treeview
Tried the code again, but it just doesn't work, it keeps repeating at the
last few nodes of the tree. One thing might be important is that we are talking about the same tree. The one I am testing on has a structure like this: R _N _N _N _ _N _ _ _ N _ _ _ N _ _ _ _ N _ _ _ N _ _ _ N _ _ _ _ N _ _ _ _ _ N I hope this shows the tree structure clearly. Actually in the code I use I don't need the intDepth argument as this doesn't do anything. RBS "Daniel.M" wrote in message ... Hi Peter, I formulated the same comment (others have also agreed) to the almost the same question a month ago and got the same answer.. Basically, it is NOT efficient to iterate through all nodes of a Tree structure AND ALSO having recursive calls on each iteration. http://groups.google.com/groups?hl=f...TNGP12.phx.gbl (the thread is interesting too as it replicates this one!) BTW, your code works fine for me too. :-) Regards, Daniel M. "Peter Beach" wrote in message ... Hi, Odd. My code worked fine for me. The point I was making was that I can't see the purpose of the lines: For Each nodChild in MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then which would seem to iterate through the tree until the target node is reached. But you already know what the target node is - it's either the first node (for the initial call) or the it's the node currently being inspected, so it seems to me that you don't need to go iterating through the tree looking for it. You can from the node recursively call the routine for the first child and then iterate through all its siblings. However programming is that art of achieving results, if your code works, stick with it. Regards, Peter Beach |
strange problem For Each loop in Treeview
Peter,
Just tried your code on a tree with a structure as you gave in your code and on that tree it works fine. So it seems it doesn't work with me as I used a different tree. It seems therefor that your code needs altering somehow. RBS "Peter Beach" wrote in message ... Hi, Odd. My code worked fine for me. The point I was making was that I can't see the purpose of the lines: For Each nodChild in MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then which would seem to iterate through the tree until the target node is reached. But you already know what the target node is - it's either the first node (for the initial call) or the it's the node currently being inspected, so it seems to me that you don't need to go iterating through the tree looking for it. You can from the node recursively call the routine for the first child and then iterate through all its siblings. However programming is that art of achieving results, if your code works, stick with it. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, Given your code a try, but it doesn't work. UBound(sItems) gives 67000, which I presume is the limit of what the array can hold and it gets into an endless loop. The code I use now is actually very efficient as it passes every node only once. Have you tried both ways? RBS "Peter Beach" wrote in message ... Hi, Glad you got it worked out. I'm still a bit puzzled by your code, as you seem to iterate through the entire tree each time, searching for your target node whereas it is surely more efficient simply to work down and across from the target node. The code I posted earlier only traverses the tree a single time which would seem to be more efficient. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, The Sub is fine. It was just the typo as stated in my previous post. RBS |
strange problem For Each loop in Treeview
Peter,
The tree I used was actually one level deeper, like this: R _N _ _N _ _N _ _ _N _ _ _ _ N _ _ _ _ N _ _ _ _ _ N _ _ _ _ N _ _ _ _ N _ _ _ _ _ N _ _ _ _ _ _ N I have tried your code on a few more tree structures and it works fine, but not on this one. Hopefully you are not getting too bored with this. RBS "Peter Beach" wrote in message ... Hi, Odd. My code worked fine for me. The point I was making was that I can't see the purpose of the lines: For Each nodChild in MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then which would seem to iterate through the tree until the target node is reached. But you already know what the target node is - it's either the first node (for the initial call) or the it's the node currently being inspected, so it seems to me that you don't need to go iterating through the tree looking for it. You can from the node recursively call the routine for the first child and then iterate through all its siblings. However programming is that art of achieving results, if your code works, stick with it. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, Given your code a try, but it doesn't work. UBound(sItems) gives 67000, which I presume is the limit of what the array can hold and it gets into an endless loop. The code I use now is actually very efficient as it passes every node only once. Have you tried both ways? RBS "Peter Beach" wrote in message ... Hi, Glad you got it worked out. I'm still a bit puzzled by your code, as you seem to iterate through the entire tree each time, searching for your target node whereas it is surely more efficient simply to work down and across from the target node. The code I posted earlier only traverses the tree a single time which would seem to be more efficient. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, The Sub is fine. It was just the typo as stated in my previous post. RBS |
strange problem For Each loop in Treeview
Peter,
I can see the point you are making. My trees are not big, so speed is not really an issue. If I could get your code to work I would use it however. Now have you tried your code on a tree with exactly the structure I posted last? I can't really see why there should be something funny with my tree structure. A treeview is a treeview and there is not that much to it apart from the normal parameters such as index, key, text etc. My trees are generated in a really complicated and big Userform, that is part of an Excel add-in. It is an interface to search an Interbase database. If you want I can send you the zipped add-in, which is about 350 kB. Will have a look again at your code and try to figure out why it trips up. Regards, RBS "Peter Beach" wrote in message ... Hi, Well mine ran OK on a tree that went down as far as that. I can't explain why it shouldn't work on the tree you specify. I have never had the problems you are having. Would you like to send me the code you are using to populate the treeview. It maybe that there is something funny in its structure. To return to my earlier point though, look again at your code (slightly simplified): Sub Tester(ByRef nodcurrent As Node) Dim nodChild As Node If nodcurrent.Children 0 Then For Each nodChild In MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then debug.print nodchild.text Tester nodChild End If End If Next nodChild End If End Sub Now you iterate through potentially every node in the treeview trying to find a node whose parent is nodCurrent. But you don't need to do that. Easier, and quicker surely, to examine each node and then look for its children and siblings? Trust me, that For Each nodChild loop is potentially going through the entire treeview looking for a cell whose parent is nodCurrent and is doing it each time the routine is called. That's not necessary. The whole idea behind a linked list (which is what a treeview control represents graphically) is that from the top of the tree you can navigate to any point in the tree without every having to know the structure of the entire list. The .Child and .Next property (together with the .Parent and .Previous properties) allow you to traverse the entire structure. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, The tree I used was actually one level deeper, like this: R _N _ _N _ _N _ _ _N _ _ _ _ N _ _ _ _ N _ _ _ _ _ N _ _ _ _ N _ _ _ _ N _ _ _ _ _ N _ _ _ _ _ _ N I have tried your code on a few more tree structures and it works fine, but not on this one. Hopefully you are not getting too bored with this. RBS "Peter Beach" wrote in message ... Hi, Odd. My code worked fine for me. The point I was making was that I can't see the purpose of the lines: For Each nodChild in MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then which would seem to iterate through the tree until the target node is reached. But you already know what the target node is - it's either the first node (for the initial call) or the it's the node currently being inspected, so it seems to me that you don't need to go iterating through the tree looking for it. You can from the node recursively call the routine for the first child and then iterate through all its siblings. However programming is that art of achieving results, if your code works, stick with it. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, Given your code a try, but it doesn't work. UBound(sItems) gives 67000, which I presume is the limit of what the array can hold and it gets into an endless loop. The code I use now is actually very efficient as it passes every node only once. Have you tried both ways? RBS "Peter Beach" wrote in message ... Hi, Glad you got it worked out. I'm still a bit puzzled by your code, as you seem to iterate through the entire tree each time, searching for your target node whereas it is surely more efficient simply to work down and across from the target node. The code I posted earlier only traverses the tree a single time which would seem to be more efficient. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, The Sub is fine. It was just the typo as stated in my previous post. RBS |
strange problem For Each loop in Treeview
Peter,
Had another look at your code. Looks to me it is just wrong. Corrected it and it now works fine. You gave this for the recursive sub: Sub IterateTreeView(a_oNode As Node) Dim X As Node ReDim Preserve sItems(nItem) As String sItems(nItem) = a_oNode.Text Set X = a_oNode.Child If Not X Is Nothing Then nItem = nItem + 1 IterateTreeView X End If Set X = a_oNode.Next Do While Not X Is Nothing nItem = nItem + 1 IterateTreeView X Set X = X.Next Loop End Sub But it should be this: Sub IterateTreeView(a_oNode As Node) Dim X As Node ReDim Preserve sItems(nItem) As String sItems(nItem) = a_oNode.Text Set X = a_oNode.Child If Not X Is Nothing Then nItem = nItem + 1 IterateTreeView X End If Set X = a_oNode.Next If Not X Is Nothing Then nItem = nItem + 1 IterateTreeView X End If End Sub Hope this settles it. RBS "Peter Beach" wrote in message ... Hi, Well mine ran OK on a tree that went down as far as that. I can't explain why it shouldn't work on the tree you specify. I have never had the problems you are having. Would you like to send me the code you are using to populate the treeview. It maybe that there is something funny in its structure. To return to my earlier point though, look again at your code (slightly simplified): Sub Tester(ByRef nodcurrent As Node) Dim nodChild As Node If nodcurrent.Children 0 Then For Each nodChild In MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then debug.print nodchild.text Tester nodChild End If End If Next nodChild End If End Sub Now you iterate through potentially every node in the treeview trying to find a node whose parent is nodCurrent. But you don't need to do that. Easier, and quicker surely, to examine each node and then look for its children and siblings? Trust me, that For Each nodChild loop is potentially going through the entire treeview looking for a cell whose parent is nodCurrent and is doing it each time the routine is called. That's not necessary. The whole idea behind a linked list (which is what a treeview control represents graphically) is that from the top of the tree you can navigate to any point in the tree without every having to know the structure of the entire list. The .Child and .Next property (together with the .Parent and .Previous properties) allow you to traverse the entire structure. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, The tree I used was actually one level deeper, like this: R _N _ _N _ _N _ _ _N _ _ _ _ N _ _ _ _ N _ _ _ _ _ N _ _ _ _ N _ _ _ _ N _ _ _ _ _ N _ _ _ _ _ _ N I have tried your code on a few more tree structures and it works fine, but not on this one. Hopefully you are not getting too bored with this. RBS "Peter Beach" wrote in message ... Hi, Odd. My code worked fine for me. The point I was making was that I can't see the purpose of the lines: For Each nodChild in MainForm.TreeView1.Nodes If Not nodChild.Parent Is Nothing Then If nodChild.Parent = nodcurrent Then which would seem to iterate through the tree until the target node is reached. But you already know what the target node is - it's either the first node (for the initial call) or the it's the node currently being inspected, so it seems to me that you don't need to go iterating through the tree looking for it. You can from the node recursively call the routine for the first child and then iterate through all its siblings. However programming is that art of achieving results, if your code works, stick with it. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, Given your code a try, but it doesn't work. UBound(sItems) gives 67000, which I presume is the limit of what the array can hold and it gets into an endless loop. The code I use now is actually very efficient as it passes every node only once. Have you tried both ways? RBS "Peter Beach" wrote in message ... Hi, Glad you got it worked out. I'm still a bit puzzled by your code, as you seem to iterate through the entire tree each time, searching for your target node whereas it is surely more efficient simply to work down and across from the target node. The code I posted earlier only traverses the tree a single time which would seem to be more efficient. Regards, Peter Beach "RB Smissaert" wrote in message ... Peter, The Sub is fine. It was just the typo as stated in my previous post. RBS |
All times are GMT +1. The time now is 02:49 PM. |
Powered by vBulletin® Copyright ©2000 - 2024, Jelsoft Enterprises Ltd.
ExcelBanter.com