Procedure inside a procedure?

Question

I found this code online that has a procedure inside a procedure. I cannot understand why the author would have chosen to write it this way. What I do notice is a recursive function being executed.

Why did he not separate the procedures like most code I have seen.

His implementation:

procedure XML2Form(tree : TJvPageListTreeView; XMLDoc : TXMLDocument); var   iNode : IXMLNode;    procedure ProcessNode(     Node : IXMLNode;      tn   : TTreeNode);   var     cNode : IXMLNode;   begin     if Node = nil then Exit;     with Node do     begin       tn := tree.Items.AddChild(tn, Attributes['text']);       tn.ImageIndex := Integer(Attributes['imageIndex']);       tn.StateIndex := Integer(Attributes['stateIndex']);     end;      cNode := Node.ChildNodes.First;     while cNode <> nil do     begin       ProcessNode(cNode, tn);       cNode := cNode.NextSibling;     end;   end; (*ProcessNode*)  begin   tree.Items.Clear;   XMLDoc.FileName := ChangeFileExt(ParamStr(0),'.XML');   XMLDoc.Active := True;    iNode := XMLDoc.DocumentElement.ChildNodes.First;    while iNode <> nil do   begin     ProcessNode(iNode,nil);     iNode := iNode.NextSibling;   end;   XMLDoc.Active := False; end; (* XML2Form *)   procedure Form2XML(tree: TJVPageListTreeView); var   tn : TTreeNode;   XMLDoc : TXMLDocument;   iNode : IXMLNode;    procedure ProcessTreeItem(     tn    : TTreeNode;     iNode : IXMLNode);   var     cNode : IXMLNode;   begin     if (tn = nil) then Exit;     cNode := iNode.AddChild('item');     cNode.Attributes['text'] := tn.Text;     cNode.Attributes['imageIndex'] := tn.ImageIndex;     cNode.Attributes['stateIndex'] := tn.StateIndex;     cNode.Attributes['selectedIndex'] := tn.SelectedIndex;      //child nodes     tn := tn.getFirstChild;     while tn <> nil do     begin       ProcessTreeItem(tn, cNode);       tn := tn.getNextSibling;     end;   end; (*ProcessTreeItem*) begin   XMLDoc := TXMLDocument.Create(nil);   XMLDoc.Active := True;   iNode := XMLDoc.AddChild('tree2xml');   iNode.Attributes['app'] := ParamStr(0);    tn := tree.TopItem;   while tn <> nil do   begin     ProcessTreeItem (tn, iNode);      tn := tn.getNextSibling;   end;    XMLDoc.SaveToFile(ChangeFileExt(ParamStr(0),'.XML'));   XMLDoc := nil; end; (* Form2XML *) 

or modified implementation:

procedure ProcessNode(Node : IXMLNode; tn : TTreeNode); var   cNode : IXMLNode; begin   if Node = nil then Exit;   with Node do   begin     tn := tree.Items.AddChild(tn, Attributes['text']);     tn.ImageIndex := Integer(Attributes['imageIndex']);     tn.StateIndex := Integer(Attributes['stateIndex']);   end;    cNode := Node.ChildNodes.First;   while cNode <> nil do   begin     ProcessNode(cNode, tn);     cNode := cNode.NextSibling;   end; end; (*ProcessNode*)  procedure ProcessTreeItem(tn : TTreeNode; iNode : IXMLNode); var   cNode : IXMLNode; begin   if (tn = nil) then Exit;   cNode := iNode.AddChild('item');   cNode.Attributes['text'] := tn.Text;   cNode.Attributes['imageIndex'] := tn.ImageIndex;   cNode.Attributes['stateIndex'] := tn.StateIndex;   cNode.Attributes['selectedIndex'] := tn.SelectedIndex;    //child nodes   tn := tn.getFirstChild;   while tn <> nil do   begin     ProcessTreeItem(tn, cNode);     tn := tn.getNextSibling;   end; end; (*ProcessTreeItem*)  procedure XML2Form(tree : TJvPageListTreeView; XMLDoc : TXMLDocument); var   iNode : IXMLNode; begin   tree.Items.Clear;   XMLDoc.FileName := ChangeFileExt(ParamStr(0),'.XML');   XMLDoc.Active := True;    iNode := XMLDoc.DocumentElement.ChildNodes.First;    while iNode <> nil do   begin     ProcessNode(iNode,nil);     iNode := iNode.NextSibling;   end;   XMLDoc.Active := False; end;  procedure Form2XML(tree: TJVPageListTreeView); var   tn : TTreeNode;   XMLDoc : TXMLDocument;   iNode : IXMLNode; begin   XMLDoc := TXMLDocument.Create(nil);   XMLDoc.Active := True;   iNode := XMLDoc.AddChild('tree2xml');   iNode.Attributes['app'] := ParamStr(0);    tn := tree.TopItem;   while tn <> nil do   begin     ProcessTreeItem (tn, iNode);      tn := tn.getNextSibling;   end;    XMLDoc.SaveToFile(ChangeFileExt(ParamStr(0),'.XML'));   XMLDoc := nil; end; (* Form2XML *) 

Answer 1

Nested procedures like that do make sense in this XML related code. To process all nodes, a recursive call of ProcessNode is needed. You have to note that sometimes, inner functions need to access a lot more data than a few parameters.

Potential implementations may be:

• Use "flat" procedures, as in your implementation;
• Use "nested" procedures, as in the original implementation;
• Create a dedicated class (or record + methods) which will remain private to the implementation part of the unit.

Of course, the 3rd option sounds the more maintainable. It will allow clear separation of the process, and allow the use of variables local to their methods. Using a record (or an object for older versions of Delphi) will allow the processing object to be allocated on the stack of the main procedure, so you won't need to write Obj := TInterType.Create; try .. finally Obj.Free. But if you use an object please note that some new version of Delphi has compilation issue - you should better use record with methods.

The "flat" procedure style is IMHO not better than "nested" procedure, and even worse, since it would need to add additional parameters to the inner calls, or use some global variables. By the way, having a lot of variables for every call will increase stack space, and reduce speed.

The "nested" style is in fact OOP oriented. When an inner function is called, the compiler pass the caller stack base in a register to the nested function (just like the additional self parameter of an object). So the inner function is able to access all the caller stack variables, just as if they were declared in a private object (the 3rd solution).

The Delphi IDE and internal debugger handles nested procedures quite well. IMHO it could make sense for some small piece of code (that is, something that can be read on the same screen height). Then, when you need more process, a dedicated record/object with methods and explicit variables will be more maintainable. But the "flat" option is IMHO not to be coded.

I've just written a blog article about these implementation patterns, which will present some source code of a QuickSort implementation, which will use as little stack space as possible, and will avoid a call to a nested procedure inside a procedure, and use a dedicated private object instead.

In all cases, do not be afraid of creating some internal objects/classes to implement your algorithms. The latest versions of Delphi allows even private types in class definition - but sometimes, I feel more comfortable with making the internal object totally private to the implementation part of the unit, i.e. non even appearing as private members of the interface part of the unit.

Classes are not only meant for publishing your process outside of the unit: OOP applies also to implementation patterns. Your code will be more maintainable, and in most case, the self parameter will be used to refer to all associated data at once, so your code may also be even faster and lighter!