Forum > General

Smart pointers revisited. Let me know what you think.

<< < (12/15) > >>

SymbolicFrank:

--- Quote from: Thaddy on January 26, 2022, 12:33:19 pm ---FreeAndNil almost always simply hides bad programming. You should almost never need it in a well written program. The fpc/lazarus community seems to favor it, The Delphi community seems to use it with more caution.
--- End quote ---

Well, it depends. Like, if you want the intersection of two StringLists:


--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function Both(List1, List2: TStringList): TStringList;
Obvious and easy, right? But now there are three objects that have to be freed at some point. It's clear the caller has to do that, right? But that's not where it is created. So, inserting the matching Free immediately afterwards typing the Create doesn't work.


--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function Both(List1, List2: string): string;
How about this? Use the TStringList.Text property. But that requires strings without line breaks. And there is more overhead.


--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function Both(List1, List2: TStringArray): TStringArray;
How about this? Still overhead, but at least you don't have to Free them.


--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function Both(List1, List2: TStringList; out These: TStringList): Boolean;
Many people think this is the way to do it. That way, the caller has to Create and Free all of them. I sometimes do this, if it is important to return a clear status, but I really don't like it. Useful in C++ style where everything generates an exception, but not in Borland style, where nil or Count = 0 are used.

If you have a class that has a property that is another class, it should be obvious that you shouldn't free it outside the encapsulating class. But you could. And it is not easy to make a copy, so you keep passing around pointers to that same class instance, or make more of the encapsulating one. Many also allow you to assign those property classes. So when do you free what if you assign the property of one to another?


--- Quote ---It is interesting to know that Delphi dropped ARC, which is a form of global automatic memory management that does not rely on garbage collection.

--- End quote ---

I vastly prefer refcounting to garbage collection, because it keeps memory usage in check and doesn't periodically freeze your application. But I'll see if I can use the TInterfacedObject.

Warfley:
Well, this is actually quite easy, when you have a function that returns a new object you to handle the function call as create, but free the Result in error case in the function

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function Both(List1, List2: TStringList): TStringList;begin  ...  Result := TStringList.Create;  try    ...  except    Result.Free;    raise;  end;end; // usage:  UnionList := Both(ListA, ListB);  try    ...  finally    UnionList.Free;  end;
That said, there shouldnt't be a reason one has to care for this stuff in the first place. This is very easy to get wrong, and automatic memory management is completely fine for most cases. I wish we could override the  ^ operator, this  would allow to create smartpointers accessible with the already existing pointer syntax

Bogen85:
Thaddy, your smart pointer proof of concept is very cool.

I tried it with a few of my existing object pascal classes that I've written, and it appears to work very well and work as expected.

The destructors don't appear to be called when smart pointers are used in a program's begin ... end. block but they do get called when used in a separate procedure, which is fine.

However, I've only gotten classes to work that don't have arguments in their constructor.
I got around this by making a generic class that wraps one pointer and frees it in it's destructor.

But I'd prefer to have the smart pointer record handle that.

Regardless though, this is very promising (for someone coming from C++ to free pascal).

EDIT: (Well, coming back to Pascal after not using it for a couple decades, and extensively using several other programming languages in the interim)

ASerge:

--- Quote from: Thaddy on January 25, 2022, 06:02:54 pm ---Well, let's see what ASerge comes up with. This is still relevant and - I think you all agree -  a total joy to explore.

--- End quote ---
Sorry, I somehow missed this conversation.
The problem with sudden double release is easily solved - I forgot to implement the AddRef method.
So the updated unit

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---unit smartptrs; {$MODE OBJFPC}{$MODESWITCH ADVANCEDRECORDS} interface type  generic TAuto<T:class> = record  strict private type    TRef = record Item: T; RefCount: LongInt; end;    PRef = ^TRef;  strict private    FRef: PRef;    class operator AddRef(var Self: TAuto);    class operator Copy(constref Src: TAuto; var Dst: TAuto);    class operator Finalize(var Self: TAuto); inline;    class operator Initialize(var Self: TAuto); inline;    function GetInstance: T; inline;    procedure Release;    procedure SetInstance(const Value: T);  public    class operator Explicit(constref From: TAuto): T; inline;    property Instance: T read GetInstance write SetInstance;  end; implementation class operator TAuto.AddRef(var Self: TAuto);begin  if Assigned(Self.FRef) then    InterlockedIncrement(Self.FRef^.RefCount);end; class operator TAuto.Copy(constref Src: TAuto; var Dst: TAuto);begin  Dst.Release;  if Assigned(Src.FRef) then    InterlockedIncrement(Src.FRef^.RefCount);  Dst.FRef := Src.FRef;end; class operator TAuto.Explicit(constref From: TAuto): T;begin  Result := From.Instance;end; class operator TAuto.Finalize(var Self: TAuto);begin  Self.Release;end; class operator TAuto.Initialize(var Self: TAuto);begin  Self.FRef := nil;end; function TAuto.GetInstance: T;begin  Result := nil;  if Assigned(FRef) then    Result := FRef^.Item;end; procedure TAuto.Release;begin  if Assigned(FRef) then  begin    if InterlockedDecrement(FRef^.RefCount) = 0 then    begin      FRef^.Item.Free;      Dispose(FRef);    end;    FRef := nil;  end;end; procedure TAuto.SetInstance(const Value: T);begin  if Assigned(FRef) then  begin    if Value = FRef^.Item then      Exit;    Release;  end;  if Assigned(Value) then  begin    New(FRef);    FRef^.Item := Value;    FRef^.RefCount := 1;  end;end; end.
and the updated Test

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---{$APPTYPE CONSOLE}{$IFDEF FPC}{$MODE OBJFPC}{$ENDIF} uses smartptrs; type  TTest = class  private    FValue: Integer;  public    constructor Create(AValue: Integer); overload;    property Value: Integer read FValue write FValue;  end; constructor TTest.Create(AValue: Integer);begin  FValue := AValue;end; type  TAutoTest = specialize TAuto<TTest>;var  GlobRef: TAutoTest; procedure TestCopyOnWrite;var  LocalRef: TAutoTest;begin  WriteLn('> TestCopyOnWrite');  LocalRef.Instance := TTest.Create(-1);  WriteLn('TTest(LocalRef).Value = ', TTest(LocalRef).Value);  TTest(LocalRef).Value := 101;  WriteLn('TTest(LocalRef).Value = ', TTest(LocalRef).Value);  Writeln('Copy Local to Global and release it');  GlobRef := LocalRef;  // TTest(GlobRef).Value = 101  LocalRef.Instance := nil; //early release  Writeln('Create new Local');  LocalRef.Instance := TTest.Create(0);  WriteLn('TTest(LocalRef).Value = ', TTest(LocalRef).Value);  WriteLn('< TestCopyOnWrite');end; procedure TestChangeInternalField(R: TAutoTest);begin  WriteLn('> TestChangeInternalField');  R.Instance.Value := -2;  WriteLn('< TestChangeInternalField');end; begin  GlobRef.Instance := TTest.Create(-1);  TestChangeInternalField(GlobRef);  WriteLn;  WriteLn('TTest(GlobRef).Value = ', TTest(GlobRef).Value);  WriteLn;  TestCopyOnWrite;  WriteLn;  WriteLn('TTest(GlobRef).Value = ', TTest(GlobRef).Value);  WriteLn;  ReadLn;end.

Bogen85:

--- Quote from: ASerge on September 25, 2022, 02:18:21 pm ---Sorry, I somehow missed this conversation.
The problem with sudden double release is easily solved - I forgot to implement the AddRef method.
So the updated unit...

--- End quote ---

Thanks ASerge, I believe your updated unit addresses some of my questions as well. I'll try to use it.

Navigation

[0] Message Index

[#] Next page

[*] Previous page

Go to full version