Recent

Author Topic: TExtendedStringList and OwnsObjects  (Read 656 times)

Sieben

  • Full Member
  • ***
  • Posts: 142
TExtendedStringList and OwnsObjects
« on: October 03, 2020, 07:49:21 pm »
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.
Lazarus 2.0.10, FPC 3.2.0, .deb install on Ubuntu Xenial 32

jamie

  • Hero Member
  • *****
  • Posts: 3797
Re: TExtendedStringList and OwnsObjects
« Reply #1 on: October 03, 2020, 10:04:50 pm »
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..
The only true wisdom is knowing you know nothing

Sieben

  • Full Member
  • ***
  • Posts: 142
Re: TExtendedStringList and OwnsObjects
« Reply #2 on: October 03, 2020, 10:19:50 pm »
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.
Lazarus 2.0.10, FPC 3.2.0, .deb install on Ubuntu Xenial 32

jamie

  • Hero Member
  • *****
  • Posts: 3797
Re: TExtendedStringList and OwnsObjects
« Reply #3 on: October 03, 2020, 10:22:03 pm »
its a memory fragmentation hog, its poorly thought out.

Its doing double allocations..

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

The only true wisdom is knowing you know nothing

Sieben

  • Full Member
  • ***
  • Posts: 142
Re: TExtendedStringList and OwnsObjects
« Reply #4 on: October 03, 2020, 10:27:04 pm »
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?
Lazarus 2.0.10, FPC 3.2.0, .deb install on Ubuntu Xenial 32

jamie

  • Hero Member
  • *****
  • Posts: 3797
Re: TExtendedStringList and OwnsObjects
« Reply #5 on: October 03, 2020, 10:42:12 pm »
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

The only true wisdom is knowing you know nothing

Sieben

  • Full Member
  • ***
  • Posts: 142
Re: TExtendedStringList and OwnsObjects
« Reply #6 on: October 03, 2020, 10:47:45 pm »
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.  
« Last Edit: October 03, 2020, 10:50:51 pm by Sieben »
Lazarus 2.0.10, FPC 3.2.0, .deb install on Ubuntu Xenial 32

howardpc

  • Hero Member
  • *****
  • Posts: 3583
Re: TExtendedStringList and OwnsObjects
« Reply #7 on: October 03, 2020, 10:52:43 pm »
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

  • Full Member
  • ***
  • Posts: 142
Re: TExtendedStringList and OwnsObjects
« Reply #8 on: October 03, 2020, 11:03:14 pm »
Yes. But at that very place where esoFreeObjectsOnDelete is asked for you could as well ask for OwnsObjects.
Lazarus 2.0.10, FPC 3.2.0, .deb install on Ubuntu Xenial 32

jamie

  • Hero Member
  • *****
  • Posts: 3797
Re: TExtendedStringList and OwnsObjects
« Reply #9 on: October 04, 2020, 01:52:12 am »
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.
The only true wisdom is knowing you know nothing

Sieben

  • Full Member
  • ***
  • Posts: 142
Re: TExtendedStringList and OwnsObjects
« Reply #10 on: October 04, 2020, 11:27:01 am »
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.
Lazarus 2.0.10, FPC 3.2.0, .deb install on Ubuntu Xenial 32

Sieben

  • Full Member
  • ***
  • Posts: 142
Re: TExtendedStringList and OwnsObjects
« Reply #11 on: October 04, 2020, 04:02:00 pm »
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.
« Last Edit: October 04, 2020, 04:55:46 pm by Sieben »
Lazarus 2.0.10, FPC 3.2.0, .deb install on Ubuntu Xenial 32

 

TinyPortal © 2005-2018