TStringList.Delete example

[Dan Rawlings]

Below is an example of the problem I posted last week. The question I
have is about the use of TSringList.Delete and how to manage the
movement of objects between two or more TSringList objects.

The example has four buttons which:
Create two StringList's.
Create, display and add a red panel to PanelList1.
Create, display and add a blue panel to PanelList1.
Moves the blue panel to PanelList2.
Free's PanelList1, free's PanelList2.

I get an address error when the application free's PanelList2, why does
this happen?

Dan

var
Form1: TForm1;
PanelList1: TStringList;
PanelList2: TStringList;

implementation

{$R *.DFM}

procedure TForm1.CreateListButtonClick(Sender: TObject);
begin
PanelList1 := TStringlist.Create;
PanelList2 := TStringList.Create;
end;

procedure TForm1.AddRedPanelButtonClick(Sender: TObject);
var
TempPanel: TPanel;
begin
TempPanel := TPanel.Create(Form1);
TempPanel.Color := clRed;
TempPanel.Parent := Form1;
TempPanel.Visible := True;
PanelList1.AddObject('Red', TempPanel);
end;

procedure TForm1.AddBluePanelButtonClick(Sender: TObject);
var
TempPanel: TPanel;
begin
TempPanel := TPanel.Create(Form1);
TempPanel.Color := clBlue;
TempPanel.Parent := Form1;
TempPanel.Top := (TempPanel.Height);
TempPanel.Visible := True;
PanelList1.AddObject('Blue', TempPanel);
end;

procedure TForm1.SwitchListButtonClick(Sender: TObject);
begin
PanelList2.AddObject('Blue', PanelList1.Objects[1]);
PanelList1.Delete(1);
end;

procedure TForm1.FreePanelsButtonClick(Sender: TObject);
var
i, i2: Integer;
begin
for i := 1 to PanelList1.Count do
begin
PanelList1.Objects[i - 1].Free;
end;
for i2 := 1 to PanelList2.Count do
begin
PanelList1.Objects[i2 - 1].Free; // Memory error occurs here.
end;
end;

end.

[Rick Rogers [TeamB]]

In article <3470D7...@appmethods.com>, Dan Rawlings wrote:
> I get an address error when the application free's
> PanelList2, why does this happen?

Your code has a very simple and obvious typographical error. The code:

for i2 := 1 to PanelList2.Count do
begin
PanelList1.Objects[i2 - 1].Free;

end;

Should read:

for i2 := 1 to PanelList2.Count do
begin

PanelList2.Objects[i2 - 1].Free;
^^^
end;

Note that your original code iterated through PanelList2.Count, but freed
the objects in PanelList1 by accident. As I said in my earlier message,
you've received the error because you free the same object twice -- in
this case, you freed the objects in PanelList1 twice by accident.

I would actually implement it like this:

for i2 := 0 to (PanelList2.Count - 1) do
begin
PanelList2.Objects[i2].Free;
end;

I'm not sure what you think you've gained by subtracting one whenever you
use the loop index; it is unusual, slower, less direct, and error-prone
(you have to remember to subtract every single time you use the index).

--
Rick Rogers (TeamB) | Fenestra Technologies

[Rick Rogers [TeamB]]

Three other notes about your code. First, In this statement you've made
a common mistake which will lead to lots of future misery:

TempPanel := TPanel.Create(Form1);

In this case, Form1 is an instance variable which is declared in the
"var" section of the unit. Form1, like any object reference variable,
can hold ONE AND ONLY ONE instance of the TForm1 class.

When your forms are auto-created, Delphi inserts code into the project
source file (.dpr) which instantiates TForm1 and assigns the returned
object pointer to the Form1 variable.

The problem with passing Form1 as the owner to TempPanel is this: what
happens if you wish to create more than one instance of your form? You
cannot use the Form1 variable to store multiple instances of the form,
so you'll have to stop using it (and use Application.Forms to manage
the form instances, or implement your own structure to do so).

This means your hard-coded reference to the Form1 variable will no
longer be valid. Delphi provides a perfect way to handle this sort of
situation, which you should use:

TempPanel := TPanel.Create(Self);

In this case, the Owner is Self -- within a class method Self always
returns the current class instance. This is exactly what you want.

Second, and in a similar vein, your use of unit variables for
PanelList1 and PanelList2 will cause problems if you have more than one
form instance. It would be far better to declare these as class fields
(or better yet, properties) like this:

TForm1 = class(TForm)
private
FPanelList1 : TStringList;
FPanelList2 : TStringList;
public
constructor Create(AOwner: TComponent); override;
destructor Destroy; override;
{ These properties are read-only on purpose. You can
still invoke class methods such as Add, Delete, etc. }
property PanelList1: TStringList read FPanelList1;
property PanelList2: TStringList read FPanelList2;
end;

constructor TForm1.Create(AOwner: TComponent);
begin
inherited Create(AOwner);
FPanelList1 := TStringList.Create;
FPanelList2 := TStringList.Create;
end;

destructor TForm1.Destroy;
begin
FPanelList1.Free;
FPanelList2.Free;
inherited Destroy;
end;

Third, your code has a potential memory leak in it. You never created
but failed to free PanelList1 and PanelList2. The approach in my second
point addresses this problem.

[Dan Rawlings]

> Your code has a very simple and obvious typographical error. The code:
>
> for i2 := 1 to PanelList2.Count do
> begin
> PanelList1.Objects[i2 - 1].Free;
> end;
>
> Should read:
>
> for i2 := 1 to PanelList2.Count do
> begin
> PanelList2.Objects[i2 - 1].Free;
> ^^^
> end;

You are correct, the example works fine after your suggested change.
Which is good, at least I've proven that Delete does not Free.
Sorry to have wasted your time.

> I would actually implement it like this:
>
> for i2 := 0 to (PanelList2.Count - 1) do
> begin
> PanelList2.Objects[i2].Free;
> end;

Right again, it's an old habit I picked up in school.
Thanks for your help.

Dan

[Rick Rogers [TeamB]]

In article <3470F4...@appmethods.com>, Dan Rawlings wrote:
> Sorry to have wasted your time.

Apology not accepted <g>. It gave me a chance to cover some common
issues which many people face when first starting to use objects in
Delphi. Hopefully lurkers will have learned from the exchange.

[Dan Rawlings]

Thanks for the code examples.

I've been using the article 'Nice and Nicer' from the August '97
Informant writen by Peter Roth as a guide. Peter suggest implementing a
couple of procedures I never bothered with before, the Clone Constructor
and Copy procedure.

Dan

[Wayne Niddery]

Dan,

Rick has given you some good tips that should help you.

I'll point out just one more.

Although only intended as a test program, there is one important
consideration you should watch out for:

All objects descending from TComponent (which includes all visual
objects such as TPanel) have the concept of an Owner, and each of these
objects can, conversely, own other objects. If an object is owned, then
it is the owner's responsibility to free it when done. For example, all
the controls you drop on Form1 at design time are owned by the form and
therefore, when Form1 gets destroyed (when you close the app), it first
goes through its list of components and frees them all.

Although there is nothing wrong with freeing them yourself as you are
doing, the TPanel objects you are creating at runtime, you are assigning
Form1 as the parent (or some instance of TForm1, if you pass Self as
Rick properly advised). Therefore, even though you created them
manually, there is no need for you to manually free them manually - they
will all be correctly freed when the owning form is destroyed.

Any objects you create which do not have an Owner property, or where you
pass nil as the owner, you *are* responsible for freeing yourself. The
example here being the two TStringList objects.

In short, your code for creating and freeing both the TStringList and
the TPanel objects is fine, but I wanted to make sure you understood the
relationships inherent in these different types of objects.

--
* Wayne Niddery - WinWright Consulting
* Host of RADBooks at http://home.ican.net/~wniddery/RADBooks.html
* -- Amazon.com Associate -- Delphi, C++Builder, JBuilder, InterDev
* ...remove BitBucket when replying...

[Daniel Rawlings]

Wayne,

> will all be correctly freed when the owning form is destroyed.

> Any objects you create which do not have an Owner property, or where you
> pass nil as the owner, you *are* responsible for freeing yourself. The
> example here being the two TStringList objects.

All objects in this app. can be traced back to the form and yes I am
using Self as the owner of the Super classes.

Could I be releasing objects that don't need to be?

SuperPanel is owned by the form.
PanelList is owned by SuperPanel.
DisplayPanels are in the PanelList.

PanelList Destroy's each item in it's list.
SuperPanel Destroy's PanelList.
By default(design) SuperPanel is Destroyed by the form.

For some reason I thought the proper method was to Free all items
in a list with it's owners' destructor.

Please do not feel a need to reply you've been extremely
helpful. The suggested Master List will manage data updates and
house business rules, thanks for the idea.
And thanks for recognizing that my example was merely an
exhibition of how fast(poorly) I can type.

Dan

[Wayne Niddery]

Daniel Rawlings wrote:
>
First, to correct a typo in my last post: I used the term "parent" in
one sentence when I meant to use "owner". This is very critical because
these are very different concepts and my entire post was meant to
strictly describe the Owner concept.

> SuperPanel is owned by the form.

If SuperPanel is of type TPanel, then this is correct.

> PanelList is owned by SuperPanel.

If PanelList is a TStringList, it has no owner (indicated by the fact it
accepts no parameters in its Create contructor). This means you are
repsonsible for destroying it when done.

> DisplayPanels are in the PanelList.

Yes, but because they are TPanels, they are also in the Form's Compnents
list and will therefore be destroyed by the form when the form is
destroyed.

> PanelList Destroy's each item in it's list.

Because the items in its list are components (TPanels) that are owned by
the form, destroying these yourself is not necessary (but is optional,
not wrong).

> SuperPanel Destroy's PanelList.

Again assuming PanelList is a TStringList, then it is not automatically
destroyed by anyone, you must do so yourself.

> By default(design) SuperPanel is Destroyed by the form.

Yes.

> For some reason I thought the proper method was to Free all items
> in a list with it's owners' destructor.

This is where you need to destroy any TStringList components you create.
If the objects you have added to the TStringList are ones that do not
have an owner (or one assigned) *then* you need to iterate the list
items and destroy each one before destroying the list itself.

A short example should clarify everything (I did not test this code):

TForm1 = class(TForm)
{...}
private
FntList: TStringList;
LblList: TStringList;
end;

{...}

procedure TForm1.FormCreate(Sender: TObject);
begin
FntList := TStringList.Create;
LblList := TStringList.Create;
end;

procedure Button1Click(Sender: TObject);
var i: integer;
lbl: TLabel;
fnt: TFont;
begin
for i := 0 to 4 do begin
// TFont does not have an Owner property
fnt := TFont.Create;
FntList.AddObject('Font'+ IntToStr(i), fnt);

// TLabel does have an owner property
lbl := TLabel.Create(Self);
lbl.Parent := Self;
lbl.Top := i * lbl.Height;
lbl.Caption := 'Label'+ IntToStr(i);
LblList.AddObject(lbl.Caption, lbl);
end;
end;

procedure TForm1.FormDestroy(Sender: TObject);
var i: integer;
begin
// need to manually free the TFont objects
for i := 0 to pred(FntList.Count) do
FntList.Objects[i].Free;
// now ok to free the list
FntList.Free;

// don't worry about the TLabel components,
// the Form will take care of freeing them
// just go ahead and free the list
LblList.Free;
end;