Sorting TreeView nodes

Van_Oss_Greggory · February 21, 2021, 10:51am

I’m working with the TMSFNCTreeView and I need to sort all the subnodes under the currently selected node according to a custom sorting algorithm. I’d been using TTMSFNCTreeViewNode.MoveTo and that seemed to be working well, but something has changed and now I’m getting an access violation when I try that.

Below is the minimum code example for what I’m trying to do. It adds the text of the nodes to a sorted stringlist and then moves the nodes according to their order in the stringlist (sorting alphabetically isn’t my goal, but I can replace that simple logic with what I need once I can get the nodes moving without getting an access violation).

Is the MoveTo method the correct way to rearrange nodes? I also tried setting TreeView1.selectedNode.Collection.Items[x].Index to move the nodes; that didn’t give an access violation but it does not seem to effect the order of the subnodes.

procedure TMainForm.Sortsubnodesalphabetically1Click(Sender: TObject);
var y,x:integer;
    SortedList: tStringlist;
begin
  SortedList:=tStringlist.create;
  SortedList.Sorted:=true;
  TreeView1.BeginUpdate;
  TreeView1.ClearNodeList;
  for y:=0 to (TreeView1.selectedNode.Collection.Count-1) do
    SortedList.AddObject(TTMSFNCTreeViewNode(TreeView1.selectedNode.Collection.Items[y]).text[0], Pointer(y));
  for y:=0 to(SortedList.count-1) do
  begin
    x:=Longint(SortedList.Objects[y]);
    TreeView1.selectedNode.nodes[x].MoveTo(TreeView1.selectedNode,y);
  end;
  TreeView1.EndUpdate;
  SortedList.Free;
end;

Pieter_Scheldeman · February 22, 2021, 11:27am

Hi,

MoveTo can indeed be used, but I noticed you are mixing Collection & Nodes, which are not the same. The Collection property is the collection above the node. The Nodes collection is the sub collection of the node. So If you have 3 parent nodes and only 2 subnodes, the code will crash. I would expect to at least use the exact same collection before sorting them.

Van_Oss_Greggory · February 22, 2021, 7:48pm

Yes, apparently I went off the rails a bit when reducing that code to the example. I made that change in the code below and it works, but only if the call to ClearNodeList is commented out. In an earlier post, I asked about a similar node process being painfully slow and you suggested adding that call (which worked well in that case). Perhaps I’m not understanding when it’s safe to call ClearNodeList? Without the call, it works great on a small tree, but the sample I tested with 70,000 nodes took well over a minute for this little code snippet to sort six child nodes. With the call to ClearNodeList, this code snippet produces access violations.

procedure TMainForm.Sortsubseriesalphabetically1Click(Sender: TObject);
var y:integer;
    MyList: tStringlist;
begin
  if (TreeView1.selectedNode.nodes.count>1) then
  begin
    MyList:=tStringlist.create;
    MyList.Sorted:=true;
   // TreeView1.ClearNodeList;
    TreeView1.BeginUpdate;
    for y:=0 to (TreeView1.selectedNode.nodes.Count-1) do
      MyList.AddObject(TTMSFNCTreeViewNode(TreeView1.selectedNode.nodes.Items[y]).text[0], TreeView1.selectedNode.nodes.Items[y]);

    for y:=0 to (MyList.count-1) do
      TTMSFNCTreeViewNode(MyList.Objects[y]).MoveTo(TreeView1.selectedNode,y);
    TreeView1.EndUpdate;
    MyList.Free;
  end;
end;

Pieter_Scheldeman · February 23, 2021, 8:04am

Hi,

When sorting, big or small lists, you can always clear out the node list. It is safer and faster to add the line into the code. Only when manipulating a single node at a time, ClearNodeList is not recommended. ClearNodeList is required to re-structure, so if you move nodes in another location, you always need to use this method.

We have tried the new code snippet but aren't able to reproduce this here. When are you calling this an on which nodes? I have tried on a new TTMSFNCTreeView instance on the form, and selected "A5 series", then clicked on a Button and the executed the code. There is no exception.

Van_Oss_Greggory · February 24, 2021, 4:11am

Hi,

I put a sample app on my server at www.ExactChange.Info/TMSSample.rar . It’s not generating access violations but it does show that there is an issue nonetheless. Also included in this zip file is a short video that shows the program running into an issue.

The sample app creates a larger tree structure as it’s a little hard to test with the small sample tree; it creates 10 nodes which each have 10 children, which of which has 10 children and all the nodes have a random name.

There are two buttons which call the sort routine I’d posted on your forum. The only difference between the two buttons is that one calls ClearNodeList and the other does not.

One definite issue I’ve encountered is that ClearNodeList unselects the currently selected node and causes TreeView.SelectedNode to point at I-don’t-know-what. In the video you can see that when ClearNodeList is called, the currently selected node is unhighlighted. I believe this behavior is new as of the last update of the FNC UI pack, though I could not swear to that. Since SelectedNode is changed by the call to ClearNodeList, I had to make a slight change to the sort so that it’d store the node that’s being sorted (it was getting access violations because I was referencing SelectedNode but ClearNodeList was wiping out SelectedNode):

procedure TForm1.Sortsubseriesalphabetically1Click(Sender: TObject);

var y:integer;

HoldNode: TTMSFNCTreeViewNode;

begin

HoldNode:=TreeView1.selectedNode;

Listbox1.Items.Clear;

if (HoldNode.nodes.count>1) then

begin

if (Sender=Button2) then TreeView1.ClearNodeList;

TreeView1.BeginUpdate;

for y:=0 to (HoldNode.nodes.Count-1) do

ListBox1.Items.AddObject(TTMSFNCTreeViewNode(HoldNode.nodes.Items[y]).text[0], HoldNode.nodes.Items[y]);

for y:=0 to (ListBox1.Items.count-1) do

TTMSFNCTreeViewNode(ListBox1.Items.Objects[y]).MoveTo(HoldNode,y);

TreeView1.EndUpdate;

end;

(This is probably obvious, but Instead of the Stringlist I used in the example of the forum, I used a tListbox just to make the list visible)

On a similar note, moving a node via drag-n-drop causes the SelectedNode to point at something else. In this sample app, if you select a node and drag to on to another node, you’ll notice that there’s no longer a selected node. That seems like new behavior, prior to the update it seems the node that was moved would remain selected, though again I can’t swear to that.

As you’ll see in the video, I sorted the children of the first node and everything was great. Then I sorted the children of the second node and things went haywire; the first node showed perhaps a hundred children suddenly and the tree’s indentation was totally screwy. I added a CollapseAll button to show that if the nodes are then all collaped, it seems to resolve the issue (until another sort is done).

Btw: I don’t want you to get the impression that I’m getting pissy; I really like your product but this does seem like something isn’t quite right there.

Gregg Van Oss

Wild Man Software

WildMan@ExactChange.info

(707)987-4499

Pieter_Scheldeman · February 24, 2021, 1:37pm

Dear Mr. Van Oss,

Thank you for your sample. We can confirm there is an issue with rebuilding the node structure when not using ClearNodeList, however it is strongly recommended to use ClearNodeList for operations that change the node structure / collection as this improves performance as well as makes sure the structure is rebuild properly. We'll investigate if we can see an issue when the collection is manipulated without ClearNodeList. Included is an update code snippet to work with ClearNodeList. You can save and re-select the node that is used to sort the nodes:

procedure TForm1.Sortsubseriesalphabetically1Click(Sender: tobject);
var
  y: integer;
  n: tTMSFNCTreeViewNode;
begin
  ListBox1.Items.Clear;
  if (TreeView1.SelectedNode.nodes.Count > 1) then
  begin
    n := TreeView1.SelectedNode;
    TreeView1.BeginUpdate;
    TreeView1.ClearNodeList;
    for y := 0 to (n.nodes.count - 1) do
      ListBox1.Items.AddObject(n.Nodes[y].Text[0], n.Nodes[y]);

    for y := 0 to ListBox1.Items.Count - 1 do
      TTMSFNCTreeViewNode(ListBox1.Items.Objects[y]).MoveTo(n, y);
    TreeView1.EndUpdate;
    TreeView1.SelectNode(n);
  end;
end;

Van_Oss_Greggory · February 24, 2021, 6:48pm

It’s the other way around; when I don’t call ClearNodeList it works but when I do (as in the video) that’s when I run into the issue.

Pieter_Scheldeman · February 24, 2021, 7:01pm

Did you try with the code snippet?

Van_Oss_Greggory · February 24, 2021, 7:18pm

Perhaps I’m not understanding the question; in the video I tried it with the code that’s in the sample, the code that uses that snippet. The example in the video includes the call to ClearNodeList. If I do that same code snippet without a call to ClearNodeList, it seems to work but it takes a big performance hit.

Pieter_Scheldeman · February 25, 2021, 8:45am

The question is if you have applied this code snippet:

procedure TForm1.Sortsubseriesalphabetically1Click(Sender: tobject);
var
  y: integer;
  n: tTMSFNCTreeViewNode;
begin
  ListBox1.Items.Clear;
  if (TreeView1.SelectedNode.nodes.Count > 1) then
  begin
    n := TreeView1.SelectedNode;
    TreeView1.BeginUpdate;
    TreeView1.ClearNodeList;
    for y := 0 to (n.nodes.count - 1) do
      ListBox1.Items.AddObject(n.Nodes[y].Text[0], n.Nodes[y]);

    for y := 0 to ListBox1.Items.Count - 1 do
      TTMSFNCTreeViewNode(ListBox1.Items.Objects[y]).MoveTo(n, y);
    TreeView1.EndUpdate;
    TreeView1.SelectNode(n);
  end;
end;

with this code snippet, we don't see issues with sorting.

Van_Oss_Greggory · February 25, 2021, 10:24am

Hi,

That code snippet is the same code snippet that’s used in the sample app I sent, with just one small difference; I was calling ClearNodeList just before then TreeView1.BeginUpdate and you call it after the BeginUpdate. I swapped those two lines in my code, making it identical to yours, and the that does stop the odd behavior. So I guess I’m just confused about what ClearNodeList does; I was under the impression it just did some housekeeping memory releases. When I look at your source code, ClearNodeList appears to just set a simple Boolean property:

procedure TTMSFNCTreeViewData.ClearNodeList;

begin

BeginUpdate;

NodeListBuild := False;

EndUpdate;

end;

Why, then, does it make a difference if the call to ClearNodeList is within a BeginUpdate…EndUpdate block?

Gregg Van Oss

Wild Man Software

WildMan@ExactChange.info

(707)987-4499

Pieter_Scheldeman · February 25, 2021, 11:27am

When calling ClearNodeList, it triggers the DoGetNumberOfNodes again. This can be sufficient when using a virtual treeview. As you are not using a virtual treeview, but actually use a collection based treeview, it is better to clear the node list together with collection manipulations inside a BeginUpdate & EndUpdate wrapper otherwise it is the same as if you would not do a ClearNodeList or as if you are not using the method at all. the EndUpdate rebuilds the internal node structure. So if that happens outside of a BeginUpdate & EndUpdate wrapper, it basically introduces the same slowness.