TStringList with objects

[Sieben]

I found "AddObject", "InsertObject", but I didn't find "DeleteObject".

If you just want to remove it from the list:

Code: Pascal  [Select][+][-]

1. mylist.Objects[i] := nil;

[PascalDragon]

I found "AddObject", "InsertObject", but I didn't find "DeleteObject". Do you have any idea for deleting an object ? Maybe "Reduce" ?

???
Not tested.....

Code: Pascal  [Select][+][-]

1.   MyList.Objects[SomeValidIndex].Free;
2.   //FreeAndNil(MyList.Objects[SomeValidIndex]);
3.  

You should also set it to Nil cause otherwise there will be problems if TStringList.OwnsObjects is set to True.

I found "AddObject", "InsertObject", but I didn't find "DeleteObject".

If you just want to remove it from the list:

Code: Pascal  [Select][+][-]

1. mylist.Objects[i] := nil;

Just in case this isn't clear: this will not free the object instance. So if you set it to Nil without having a reference to that object instance somewhere else you'll have a memory leak.

[Sieben]

Just in case this isn't clear: this will not free the object instance.

That's why I wrote 'remove it from the list'. But you're right of course, always better to point it out.

[engkin]

What class(es) would you suggest as an improvement on TStringList, and how are they better?

Probably class TFPHashObjectList from unit Contnrs.

[SymbolicFrank]

What class(es) would you suggest as an improvement on TStringList, and how are they better?

Any generic map or dictionary? Because they are type safe for all elements. But you know all that....

A common problem with descendants of classes like TFPGMap (TFPSList) is, that they have one or more public constructors and/or methods you really don't want any interfacing code to use. (Anything that handles plain pointers, for a start.) Not a problem if you're the sole developer, but not so good in a team or interface. And writing your own custom container class from scratch every time gets old fast.

[jcmontherock]

FreeAndNil(MyList.Objects[SomeValidIndex]);  gives a compile error   ;O(

[Handoko]

You did it wrong. See the line #69:

Code: Pascal  [Select][+][-]

1. unit Unit1;
2.  
3. {$mode objfpc}{$H+}
4.  
5. interface
6.  
7. uses
8.   Classes, SysUtils, Forms, Controls, Graphics, Dialogs, StdCtrls, ExtCtrls;
9.  
10. type
11.  
12.   { TForm1 }
13.  
14.   TForm1 = class(TForm)
15.     Button1: TButton;
16.     Label1: TLabel;
17.     procedure Button1Click(Sender: TObject);
18.     procedure FormClose(Sender: TObject; var CloseAction: TCloseAction);
19.     procedure FormCreate(Sender: TObject);
20.   private
21.     FStringList: TStringList;
22.     function GenerateRandomItem: TShape;
23.     function GenerateText(aShape: TShape): string;
24.     procedure ShowInformation;
25.   end;
26.  
27. var
28.   Form1: TForm1;
29.  
30. implementation
31.  
32. {$R *.lfm}
33.  
34. { TForm1 }
35.  
36. procedure TForm1.Button1Click(Sender: TObject);
37. const
38.   ClickCount: Integer = 0;
39. var
40.   Something: TShape;
41.   S:         string;
42.   i:         Integer;
43. begin
44.  
45.   // === First click
46.   // Create some items and store the information to FStringList
47.   if ClickCount = 0 then
48.   begin
49.     for i := 0 to 15 do
50.     begin
51.       Something := GenerateRandomItem;
52.       S         := GenerateText(Something);
53.       Something.Hint := '#' + i.ToString + ' - ' + S;
54.       FStringList.Add('Item #' + i.ToString + ' is a ' + LowerCase(S) + '.');
55.       FStringList.Objects[i] := Something;
56.     end;
57.     ShowInformation;
58.     Button1.Caption := 'Click me AGAIN !';
59.     Inc(ClickCount);
60.     Exit;
61.   end;
62.  
63.   // === Second click
64.   // Delete all the items in the FStringList
65.   if ClickCount = 1 then
66.   begin
67.     repeat
68.       Something := (FStringList.Objects[0] as TShape);
69.       FreeAndNil(Something);
70.       FStringList.Delete(0);
71.     until FStringList.Count <= 0;
72.     ShowInformation;
73.     Button1.Enabled := False;
74.   end;
75.  
76. end;
77.  
78. procedure TForm1.FormClose(Sender: TObject; var CloseAction: TCloseAction);
79. begin
80.   FStringList.Free;
81. end;
82.  
83. procedure TForm1.FormCreate(Sender: TObject);
84. begin
85.   FStringList := TStringList.Create;
86. end;
87.  
88. function TForm1.GenerateRandomItem: TShape;
89. var
90.   NewShape: TShape;
91. begin
92.   NewShape := TShape.Create(Self);
93.   with NewShape do
94.   begin
95.     Parent   := Self;
96.     Width    := 50;
97.     Height   := 50;
98.     Left     := Random(Self.Width  - 50);
99.     Top      := Random(Self.Height - 90);
100.     ShowHint := True;
101.     case Random(3) of
102.       0: Shape := stRectangle;
103.       1: Shape := stCircle;
104.       2: Shape := stTriangle;
105.     end;
106.     case Random(3) of
107.       0: Brush.Color := clYellow;
108.       1: Brush.Color := clGreen;
109.       2: Brush.Color := clRed;
110.     end;
111.   end;
112.   Result := NewShape;
113. end;
114.  
115. function TForm1.GenerateText(aShape: TShape): string;
116. var
117.   S: string;
118. begin
119.   S := '';
120.   case aShape.Brush.Color of
121.     clYellow: S := 'Yellow';
122.     clGreen:  S := 'Green';
123.     clRed:    S := 'Red';
124.   end;
125.   case aShape.Shape of
126.     stRectangle: S := S + ' Retangle';
127.     stCircle:    S := S + ' Circle';
128.     stTriangle:  S := S + ' Triangle';
129.   end;
130.   Result := S;
131. end;
132.  
133. procedure TForm1.ShowInformation;
134. var
135.   S: string;
136.   i: integer;
137. begin
138.   S := '';
139.   for i := 0 to FStringList.Count-1 do
140.     S := S + FStringList[i] + LineEnding;
141.   Label1.Caption := S;
142.   Label1.Visible := True;
143.   Height := Label1.Height + 250;
144. end;
145.  
146. end.

[wp]

FreeAndNil(MyList.Objects[SomeValidIndex]);  gives a compile error   ;O(

Why FreeAndNil at all? With setting a pointer variable to nil you signal to other pieces of your code that this object does not exist any more. But this is not necessary here at all since you free the entire list and thus it is impossible to access the individual pointers to these objects any more.  Do not confuse the pointers stored in the list with other variables pointing to the same object instance - they will not be nil'ed anyway even if you call FreeAndNil for MyList.Objects[..].

Simply .Free the object:

Code: Pascal  [Select][+][-]

1.   MyList.Objects[SomeValidIndex].Free;

[jcmontherock]

Handoko, you have right with your point:

FreeAndNil(MyList.Objects[SomeValidIndex]);  gives a compile error: Can't take the address of constant expressions.

MyObject := TMyClass(MyList.Objects[SomeValidIndex]);
FreeAndNil(MyObject);                                      gives no error

[Handoko]

Yes, it is not only gives no error it also runs perfectly without error.

But as @wp said, maybe you don't need to use FreeAndNil. I'm not interested into the debate of using FreeAndNil or not, but I only used it once in a program I wrote, and I really had the reason why FreeAndNil was needed. For all the other programs I wrote, using Free only, seem to run without any issue. So, I can sure to say 99% Free only is enough.

[BeniBela]

What class(es) would you suggest as an improvement on TStringList, and how are they better?

Classes are also a bad idea. They gives too much trouble with Free




If you mostly use the Text property, I would just suggest a  string.

For a list of strings, I would suggest array of string.

For a list of strings and objects, I would suggest array of record a: string; b: TYourObject; end  . And TYourObject should be an object or record, not a class.

For a sorted list of strings and objects used as map, I would suggest TDictionary

[MarkMLl]

I'm afraid that I'm very sceptical that this is good advice for a newcomer.

For a list of strings and objects, I would suggest array of record a: string; b: TYourObject; end  . And TYourObject should be an object or record, not a class.

OP has already said that he's using a TStringList, and is asking for advice in that context.

Muddying the water by introducing the obsolete Turbo Pascal object (if that is your intention), or by overlooking the fact that OP already knows that he needs an /instance/ of a class rather than the class itself, is not helpful.

MarkMLl

[Sieben]

But as @wp said, maybe you don't need to use FreeAndNil. I'm not interested into the debate of using FreeAndNil or not, but I only used it once in a program I wrote, and I really had the reason why FreeAndNil was needed. For all the other programs I wrote, using Free only, seem to run without any issue. So, I can sure to say 99% Free only is enough.

Most of the time because the vars just go out of scope, joining the quire invisible. But if they don't you'd better nil 'em.

[PascalDragon]

FreeAndNil(MyList.Objects[SomeValidIndex]);  gives a compile error   ;O(

Why FreeAndNil at all? With setting a pointer variable to nil you signal to other pieces of your code that this object does not exist any more. But this is not necessary here at all since you free the entire list and thus it is impossible to access the individual pointers to these objects any more.  Do not confuse the pointers stored in the list with other variables pointing to the same object instance - they will not be nil'ed anyway even if you call FreeAndNil for MyList.Objects[..].

Simply .Free the object:

Code: Pascal  [Select][+][-]

1.   MyList.Objects[SomeValidIndex].Free;

No, you should Nil it, cause otherwise there'll be a crash if OwnsObjects is set to True. Thus the correct one in this case is:

Code: Pascal  [Select][+][-]

1. MyList.Objects[SomeValidIndex].Free;
2. MyList.Objects[SomeValidIndex] := Nil;

Handoko, you have right with your point:

FreeAndNil(MyList.Objects[SomeValidIndex]);  gives a compile error: Can't take the address of constant expressions.

MyObject := TMyClass(MyList.Objects[SomeValidIndex]);
FreeAndNil(MyObject);                                      gives no error

This is rather useless. If you use a variable simply to use FreeAndNil then you have not understood what FreeAndNil does: MyObject will be Nil after that call, but MyList.Objects[SomeValidIndex] will not (and neither will any other reference that points to that object instance).

For a list of strings and objects, I would suggest array of record a: string; b: TYourObject; end  . And TYourObject should be an object or record, not a class.

Which would require users to write functions like IndexOf by themselves possibly introducing errors along the way. The container types exist for a reason, namely that one doesn't need to reinvent the wheel again and that one can concentrate on what is important: one's own code.

What class(es) would you suggest as an improvement on TStringList, and how are they better?

Any generic map or dictionary? Because they are type safe for all elements. But you know all that....

A common problem with descendants of classes like TFPGMap (TFPSList) is, that they have one or more public constructors and/or methods you really don't want any interfacing code to use. (Anything that handles plain pointers, for a start.) Not a problem if you're the sole developer, but not so good in a team or interface. And writing your own custom container class from scratch every time gets old fast.

Then use Generics.Collections.TDictionary if you're worried about that.

But as @wp said, maybe you don't need to use FreeAndNil. I'm not interested into the debate of using FreeAndNil or not, but I only used it once in a program I wrote, and I really had the reason why FreeAndNil was needed. For all the other programs I wrote, using Free only, seem to run without any issue. So, I can sure to say 99% Free only is enough.

Most of the time because the vars just go out of scope, joining the quire invisible. But if they don't you'd better nil 'em.

This is only true for managed types and record and object instances that are located on the stack (or are global variables). Class instances and any other manual allocations need to be freed explicitely as well.

[SymbolicFrank]

I'm afraid that I'm very sceptical that this is good advice for a newcomer.

For a list of strings and objects, I would suggest array of record a: string; b: TYourObject; end  . And TYourObject should be an object or record, not a class.

OP has already said that he's using a TStringList, and is asking for advice in that context.

Muddying the water by introducing the obsolete Turbo Pascal object (if that is your intention), or by overlooking the fact that OP already knows that he needs an /instance/ of a class rather than the class itself, is not helpful.

MarkMLl

Indeed. I would even go as far to advise only making and using classes, simply to keep it all simple and consistent. Learn to do one thing well.