CODEWITHSUNDEEP
delphidelphi-xe7virtualtreeview
Mike Torrettinni
Mike Torrettinni

Reputation: 1824

How to make this code re-usable

I have a specific way to build nodes for Virtual Treeview (I douns this example years ago and never had a reason to change it, until now). As I use almost identical code in around 150 cases, I would like to try and get it re-usable and reduce overall code lines.

I'm attaching full code of example with 2 buttons and a Vritual Treeview on the form:

unit Unit1;

interface

uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, VirtualTrees, Vcl.StdCtrls;

type

  rTreeData = record
    IndexInMyData: integer;
  end;

  tLine = record
    Level:integer;
    Txt:string;
    NodePointer: PVirtualNode;
  end;

  TForm1 = class(TForm)
    Button1: TButton;
    VTV: TVirtualStringTree;
    Button2: TButton;
    procedure Button1Click(Sender: TObject);
    procedure Button2Click(Sender: TObject);
  private
    { Private declarations }
  public
    { Public declarations }
  end;

var
  Form1: TForm1;
  vArray:array of tLine;

implementation

{$R *.dfm}



procedure TForm1.Button1Click(Sender: TObject);
var
  Node: PVirtualNode;
  Data: ^rTreeData;
  i, j: integer;
begin
  SetLength(vArray,5);
  vArray[0].Level:=0; vArray[0].Txt:='One';
  vArray[1].Level:=1; vArray[1].Txt:='Two';
  vArray[2].Level:=1; vArray[2].Txt:='three';
  vArray[3].Level:=2; vArray[3].Txt:='Four';
  vArray[4].Level:=0; vArray[4].Txt:='Give';

  VTV.BeginUpdate;
  VTV.Clear;
  for i := Low(vArray) to High(vArray) do
  begin
     if i = 0 then
    begin
      Node := VTV.AddChild(nil);
      Data := VTV.GetNodeData(Node);
    end
    else
    begin
      if vArray[i].Level = 0 then
        Node := VTV.AddChild(nil)
      else if vArray[i].Level > vArray[i - 1].Level then
        Node := VTV.AddChild(Node)
      else if vArray[i].Level < vArray[i - 1].Level then
      begin
        Node := Node.Parent;
        for j := 1 to (vArray[i - 1].Level - vArray[i].Level) do
          Node := Node.Parent;
        Node := VTV.AddChild(Node);
      end
      else
      begin
        Node := Node.Parent;
        Node := VTV.AddChild(Node);
      end;

      Data := VTV.GetNodeData(Node);
    end;

    // Create link to your data record into VST node
    Data.IndexInMyData := i;
    vArray[Data.IndexInMyData].NodePointer := Node;
  end;
  VTV.FullExpand;
  VTV.EndUpdate;
end;

function AddNode(vTV: TvirtualStringTree; vI, vLevel, vLevelPrev:integer; vNode:PVirtualNode; var vData:rTreeData):PVirtualNode;
var j:integer;
begin
   if vI = 0 then
    begin
      Result := vTV.AddChild(nil);
      vData := rTreeData(vTV.GetNodeData(Result)^);
    end
    else
    begin
      if vLevel = 0 then  Result := vTV.AddChild(nil)
      else if vLevel > vLevelPrev then  Result := vTV.AddChild(vNode)
      else if vLevel < vLevelPrev then
      begin
        Result := vNode.Parent;
        for j := 1 to (vLevelPrev - vLevel) do
          Result := Result.Parent;
        Result := vTV.AddChild(Result);
      end
      else
      begin
        Result := vNode.Parent;
        Result := vTV.AddChild(Result);
      end;

      vData := rTreeData(vTV.GetNodeData(Result)^);
    end;
    vData.IndexInMyData := vI;
end;

procedure TForm1.Button2Click(Sender: TObject);
var
  Node: PVirtualNode;
  Data: ^rTreeData;

  i, j,vLevelPrev: integer;
begin
  SetLength(vArray,5);
  vArray[0].Level:=0; vArray[0].Txt:='One';
  vArray[1].Level:=1; vArray[1].Txt:='Two';
  vArray[2].Level:=1; vArray[2].Txt:='three';
  vArray[3].Level:=2; vArray[3].Txt:='Four';
  vArray[4].Level:=0; vArray[4].Txt:='Give';

  VTV.BeginUpdate;
  VTV.Clear;
  for i := Low(vArray) to High(vArray) do
  begin
    if i = 0 then
      vLevelPrev:=0
    else
      vLevelPrev:=vArray[i-1].Level;

      Node:=AddNode(VTV,i,vArray[i].Level,vLevelPrev,Node,rTreeData(Data^));

//     if i = 0 then
//    begin
//      Node := VTV.AddChild(nil);
//      Data := VTV.GetNodeData(Node);
//    end
//    else
//    begin
//      if vArray[i].Level = 0 then
//        Node := VTV.AddChild(nil)
//      else if vArray[i].Level > vArray[i - 1].Level then
//        Node := VTV.AddChild(Node)
//      else if vArray[i].Level < vArray[i - 1].Level then
//      begin
//        Node := Node.Parent;
//        for j := 1 to (vArray[i - 1].Level - vArray[i].Level) do
//          Node := Node.Parent;
//        Node := VTV.AddChild(Node);
//      end
//      else
//      begin
//        Node := Node.Parent;
//        Node := VTV.AddChild(Node);
//      end;
//
//      Data := VTV.GetNodeData(Node);
//    end;

    // Create link to your data record into VST node
    Data.IndexInMyData := i;
    vArray[Data.IndexInMyData].NodePointer := Node;
  end;
  VTV.FullExpand;
  VTV.EndUpdate;
end;

end.

So, Button1 uses my curent style of code, to build nodes. It does work.

Button2 is trying to call AddNode procedure to create AddNode as re-usable code. It compiles, but it fails on line #: 101: EAccessViolation:

enter image description here

I think there is a problem with the how I use, assign pointer and values... I didn't get past this line, so I don't know if rest of the code works.

Any suggestions how to fix this, how to make the code re-usable?

EDIT

If I remove vData parameter works good: This is now reduced code:

VTV.BeginUpdate;
  VTV.Clear;
  vLevelPrev:=0
  for i := Low(vArray) to High(vArray) do
  begin
    if i > 0 then vLevelPrev:=vArray[i-1].Level;

    Node:=AddNode(VTV,i,vArray[i].Level,vLevelPrev,Node);

    // Create link to your data record into VST node
    Data := VTV.GetNodeData(Node);
    Data.IndexInMyData := i;
    vArray[Data.IndexInMyData].NodePointer := Node;
  end;
  VTV.FullExpand;
  VTV.EndUpdate;

And AddNode:

function AddNode(vTV: TvirtualStringTree; vI, vLevel, vLevelPrev:integer; vNode:PVirtualNode):PVirtualNode;
var j:integer;
begin
   if vI = 0 then
    begin
      Result := vTV.AddChild(nil);
    end
    else
    begin
      if vLevel = 0 then  Result := vTV.AddChild(nil)
      else if vLevel > vLevelPrev then  Result := vTV.AddChild(vNode)
      else if vLevel < vLevelPrev then
      begin
        Result := vNode.Parent;
        for j := 1 to (vLevelPrev - vLevel) do
          Result := Result.Parent;
        Result := vTV.AddChild(Result);
      end
      else
      begin
        Result := vNode.Parent;
        Result := vTV.AddChild(Result);
      end;

    end;
end;

Anyway to reduce code by processing Data in AddNode?

SOLUTION:

I put Data as local pointer into AddNode:

function AddNode(vTV: TvirtualStringTree; vI, vLevel, vLevelPrev:integer; vNode:PVirtualNode):PVirtualNode;
var j:integer;
    Data: ^rTreeData;
begin
   if vI = 0 then
      Result := vTV.AddChild(nil)
    else
    begin
      if vLevel = 0 then  Result := vTV.AddChild(nil)
      else if vLevel > vLevelPrev then  Result := vTV.AddChild(vNode)
      else if vLevel < vLevelPrev then
      begin
        Result := vNode.Parent;
        for j := 1 to (vLevelPrev - vLevel) do
          Result := Result.Parent;
        Result := vTV.AddChild(Result);
      end
      else
      begin
        Result := vNode.Parent;
        Result := vTV.AddChild(Result);
      end;
    end;
    Data := VTV.GetNodeData(Result);
    Data.IndexInMyData := vI;
end;

And now I have the final reduced code that uses AddNode:

vLevelPrev := 0;
  for i := Low(vArray) to High(vArray) do
  begin
    if i > 0 then
      vLevelPrev := vArray[i - 1].Level;
    Node := AddNode(VTV, i, vArray[i].Level, vLevelPrev, Node);
    vArray[i].NodePointer := Node;
  end;

Upvotes: 1

Views: 213

Answers (1)

Uwe Raabe
Uwe Raabe

Reputation: 47704

At the call to AddNode in Button2Click the pointer variable Data is still uninitialized and points to some arbitrary memory, which will be written to inside AddNode leading to that access violation.

I am still not sure if I understand, why you need that vData parameter at all. Make vData a local pointer to a rTreeData record as is Data in Button1Click and remove that parameter completely. In Button2Click use I to index into vArray.

Upvotes: 4

Related Questions