TExtendedStringList and OwnsObjects

[Sieben]

Does anyone know why TExtendedStringList introduces it's own option esoFreeObjectsOnDelete instead of just using it's ancestors OwnsObjects property? It seems to serve exactly the same purpose while the original TStringList property is completely ignored.

[jamie]

No it does not do the same thing..

Its a class used specifically just to store some unknown created objects/Classes etc  and that is what's used for, to manage those.

 It looks like something that was specific and I don't see any use of it..

[Sieben]

It - TExtendedStringList - is used in TCustomListBox to store the Selected property of the items. For that purpose it stores the Object plus the additional property in a custom record. Now when an item is deleted or the list is cleared or freed it provides the option to free it's objects as well - just like TStringlist does with the difference that TStringList already has OwnsObjects property to manage that. It's an interesting class for it provides the oppurtunity of storing rather simple data types 'besides' the object when creating an extra object for that purpose would be kind of overkill.

[jamie]

its a memory fragmentation hog, its poorly thought out.

Its doing double allocations..

I don't know, I would of gone some other route..

[Sieben]

It seems to have some minor flaws indeed, but I can see no double allocations so far. Would you mind to have another look at it?

[jamie]

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

1. procedure TExtendedStringList.FreeRecord(Index: integer);
2. var
3.   OldRecord: pointer;
4.   OldObject: TObject;
5. begin
6.   OldRecord:=inherited GetObject(Index);
7.   if OldRecord<>nil then begin
8.     if (esoFreeObjectsOnDelete in Options) then begin
9.       OldObject:=Objects[Index];
10.       if OldObject<>nil then begin
11.         OldObject.Free;
12.       end;
13.     end;
14.     FreeMem(OldRecord);
15.     inherited PutObject(Index,nil);
16.   end;
17. end;      
18.                                
19.  

This is from Laz 2.0.4
Unless they have changed something it is getting Record and A Object from the same index.
you should notice they are calling the inherited GetObject and Also Objects

• which comes from the same location.

 Its just my observation at first glance.

It just does not look right to me and that isn't how I would code this...
oh well

[Sieben]

But instead of the object there is that record at this position, and the first 'field' of the record is a pointer reserved for the object. The first call - GetObject(Index) - calls the inherited method whereas the second - Objects[Index] - calls it's own overriden method and fetches the object from the record.

Edit: this is the record it uses in TCustomListBox:

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

1. type
2.   TCustomListBoxItemRecord = record
3.     TheObject: TObject;
4.     Selected: Boolean;
5.   end;
6.   PCustomListBoxItemRecord = ^TCustomListBoxItemRecord;
7.  
8.  

[howardpc]

Does anyone know why TExtendedStringList introduces it's own option esoFreeObjectsOnDelete instead of just using it's ancestors OwnsObjects property? It seems to serve exactly the same purpose while the original TStringList property is completely ignored.

It is because TExtendedStringList is cleverly designed not only to hold TObjects in its Objects property, but can also hold a reference to records.
Records cannot be freed.
Consequently the inherited FreeObjects property was inadequate.
This class needed extending for the case where the list contains records, when the option esoClearRecordsOnCreate is the appropriate alternative to esoFreeObjectsOnDelete that applies when objects (rather than records) are being contained.

[Sieben]

Yes. But at that very place where esoFreeObjectsOnDelete is asked for you could as well ask for OwnsObjects.

[jamie]

I guess you are not seeing what I am.

oh well, I won't go any future on this because I know what It will turn into but one day when I decide to use this code in the TlistBox, I am sure I will remember this and be back at this code when things start to go left field.

[Sieben]

I guess you are not seeing what I am.

Now, what do you see? The last line of FreeRecord is

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

1.     inherited PutObject(Index,nil);

so by the time when inherited Delete, Clear or Destroy of the ancestor ist called it's 'Objects' are gone already, the pointer is nil and nothing would be tried to be freed despite it's OwnsOnjects property being true. I guess that's what howardpc wanted to point at as well, but it's idle fear.

[Sieben]

Now what I'm trying to get at is a proposition as follows:

- rewrite TExtendedStringList to a new class TRecordStringList that corrects some minor flaws - for instance a CheckIndex as first line of CreateRecord so that no memory is allocated for an index that might not exist - and at the same time introducing a new ancestor class TCustomRecordStringList that can be used to descend lists that for example might look like this:

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

1.   TCheckTreeList = class(TCustomRecordStringList)
2.   private
3.     function GetCheckMode(Index: Integer): TCheckTreeMode;
4.     function GetCheckState(Index: Integer): TCheckTreeState;
5.     procedure SetCheckMode(Index: Integer; AValue: TCheckTreeMode);
6.     procedure SetCheckState(Index: Integer; AValue: TCheckTreeState);
7.   public
8.     constructor Create; reintroduce;
9.     property CheckMode[Index: Integer]: TCheckTreeMode read GetCheckMode write SetCheckMode;
10.     property CheckState[Index: Integer]: TCheckTreeState read GetCheckState write SetCheckState;
11.   end;
12.  

without confusing or tempting the user (ie application developer) with the record properties or methods and without having to create a TObject descendant for those indexed properties.

- add RecordStrings to LazUtils and replace TExtendedStringList within TCustomListBox with TRecordStringList

- leave ExtendedStrings in LazUtils in case there is code out there that relies on the current layout of TExtendedStringList, but eventually mark it as deprecated

Where and how to make a propostion like that? Maybe Bart or someone else more involved with the interiors of Lazarus might advise...?

I'm including recordstrings.pas and a recordstrings.diff created with rev 63949 so it can be tested with trunk.