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

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

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

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

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

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

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

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

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

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

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

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

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