Wer macht den Free?

[Mavarik]

Hallo!

Ich refrakturiere gerade alten Source-Code für einen Kollegen...

Die Frage ist: Wer gibt eine Instance wieder frei... (Gibt's sicher wieder ne tolle Regel)

Beispiel:

Delphi-Quellcode:

Procedure Foo; // Logisch...
var
  LListe : TStringList;
begin
  LListe := TStringList.Create;
  try
    // Whatever
  finally
    LListe.Free; 
  end;
end;

Procedure Bar(Const AListe : TStrings); 
begin
  // Whatever
  
  AListe.Free; // oder nicht?  (wenn kein Const dann klar!)
end;

Function Foo2(Const AParam1,AParam2 : String) : TStrings;
begin
  Result := TStringList.Create;
  Result.AddPair(AParam1,AParam2);
  // Whatever
  // Logischerweise kein Free
end;

Procedure Bar2;
begin
  Bar(Foo2('Value','42'));  
end;

Was ist wenn ich den Source von Bar nicht habe und in der Doc nix steht...

Angenommen Bar gibt die Instance nicht frei, dann ist es ein MemLeak!

Delphi-Quellcode:

Procedure Bar2;
var
  LStrings : TString;
begin
  LStrings := Foos2('Value','42');
  try
    Bar(LStrings);  // Dann hoffen wir mal Bar hat FreeAndNIL verwendet...
                    // Es sei den der Param ist ein Const dann geht FreeAndNIL nicht... 
  finally
    FreeAndNIL(LStrings);
  end;
end;

Besonders wenn man fremden Source liest, sieht man ggf. nicht ob man ein Object frei geben muss oder nicht.

Delphi-Quellcode:

Function Bar3(Const AListe : TStrings) : TStrings;
begin
// Den Source kenne ich nicht...  
end;

Function Bar4;
var
  LListe : TStrings;
begin
  LListe := Bar3(Foo2('Value','42'));
  // Ist LListe meine Instance von Foo2 oder eine neue?
  LListe := Foo2('Value','42');
  LListe := Bar3(LListe); // Hab ich jetzt ein MemLeak?
end;

Wie macht Ihr das?

Mavarik :stupid:

[Bernhard Geyer]

I.d.R. ist der der Ersteller auch für die Freigabe zuständig.
Hat man Funktionen bei denen das nicht der Fall ist, so ist der Funktions/Methodennamen entsprechend gewählt CreateMySuperDuperInstance(...)
Oder man baut seine Logik auf Interfaces auf. Dann stellt sich einen die Frage gar nicht.

[Mavarik]

Zitat:

Zitat von Bernhard Geyer (Beitrag 1361337)

I.d.R. ist der der Ersteller auch für die Freigabe zuständig.
Hat man Funktionen bei denen das nicht der Fall ist, so ist der Funktions/Methodennamen entsprechend gewählt CreateMySuperDuperInstance(...)
Oder man baut seine Logik auf Interfaces auf. Dann stellt sich einen die Frage gar nicht.

Klar mit Interfaces zählen die für einen mit... Bei eigenen Klassen kein Problem, aber was ist, wenn Du eine Procedure hast Du - wie in meinem Beispiel - als Parameter TStrings verlangt... Da kannst Du nicht so einfach IStrings übergeben...

[Stevie]

Gerade bei TStrings hab ich in unserem eigenen Sourcecode oft gesehen, dass hier ein TArray<string> genug gewesen wäre und sich somit die Frage gar nicht mehr stellt.

[Uwe Raabe]

Delphi-Quellcode:

Function Bar3(Const AListe : TStrings) : TStrings;
begin
// Den Source kenne ich nicht...  
end;

Function Bar4;
var
  LListe1 : TStrings;
  LListe2 : TStrings;
begin
  LListe1 := Foo2('Value','42');
  // unter der Annahme, daß Bar3 die Instanz nicht freigibt, was dem const ja widersprechen würde
  LListe2 := Bar3(LListe1);
  if LListe2 <> LListe2 then begin
    LListe2.Free;
  end;
  LListe1.Free;
end;

[Zacherl]

Ohne Quelltext ist sowas immer ein Ratespiel. Ich würde es bei Möglichkeit einfach mal ausprobieren. Eine generelle Faustregel wäre mir nicht bekannt, allerdings halte ich mich auch an das Prinzip, dass der Ersteller für das Freigeben verantwortlich ist. Kann sonst selbst im eigenen Code recht schnell ziemlich unübersichtlich werden.

Eine der Stellen, an der man sich die C++ "Const-ness" wünscht, mit der man nicht nur die Referenz/Pointer als

Delphi-Quellcode:

const

markieren kann, sondern tatsächlich auch das dahinterliegende Objekt.

[himitsu]

Zitat:

was dem const ja widersprechen würde

Jain.

FreeAndNil würde dem Const wiedersprechen, aber Free per se nicht.
Es wird ja nicht die Variable/Parameter verändert, sondern nur das, worauf sie zeigt. :roll:


Also der "Grundsatz" wurde ja schon genannt.
Am Besten gibt immer der etwas frei, der es auch erstellt hat.

Im Falle von solchen Results könnte die Klasse, wo die Get-Methode drin ist, eine GibFrei-Methode anbieten.
Alternativ kommt es auf die Dokumentation an.
Entweder die Get-Methode speichert intern den Zeiger und gibt es später frei, wenn die übergeordnete Klasse auch freigegeben wird, bzw. beim nächsten Aufruf der Get-Methode.
Oder man definiert es so, dass der "Caller" in soeinem Fall das freigeben muß. (ich würde das auch präferieren)

[Mavarik]

Zitat:

Zitat von Stevie (Beitrag 1361339)

Gerade bei TStrings hab ich in unserem eigenen Sourcecode oft gesehen, dass hier ein TArray<string> genug gewesen wäre und sich somit die Frage gar nicht mehr stellt.

ja so mache ich es auch...

Zitat:

Zitat von Zacherl (Beitrag 1361342)

Eine der Stellen, an der man sich die C++ "Const-ness" wünscht, mit der man nicht nur die Referenz/Pointer als

Delphi-Quellcode:

const

markieren kann, sondern tatsächlich auch das dahinterliegende Objekt.

Ja - der Kollege hat im ganzen Source FreeAndNIL genommen und wäre er dabei geblieben - alles gut - aber da viele der Proceduren mit CONST sind hat er in diesen Fällen

AListe.Free genommen... (Manchmal)

Ich muss mir also jeder Procedure einzeln ansehen und das in alle tiefen, ob jemand aufräumt... ätzend...

Da lobe ich mir doch die Interfaces...

Mavarik

[striderx]

"Ich refrakturiere gerade alten Source-Code für einen Kollegen..."

Das ist aber nicht sehr nett von dir ... :wink:

[Mavarik]

Zitat:

Zitat von striderx (Beitrag 1361362)

"Ich refrakturiere gerade alten Source-Code für einen Kollegen..."

Das ist aber nicht sehr nett von dir ... :wink:

Ja genau - ich breche den Code in Teile... :stupid:

[DeddyH]

Wegen genau dieser Problematik meide ich Funktionen, die Klasseninstanzen zurückgeben, welche erst innerhalb der Funktion erzeugt werden, wie der Teufel das Weihwasser. Etwas anders ist es bei Interfaces, da ist es dann eben ein TInterfacedObject, aber das muss den Aufrufer ja nicht interessieren. Außerdem schließt man somit die unnötige Verschwendung von Bits und Bytes (:mrgreen:) wie hier aus:

Delphi-Quellcode:

function TMyClass.GetNames: TStrings;
var
  List: TStringlist;
begin
  List := TStringlist.Create;
  //Liste beispielhaft befüllen
  List.Add('Heinz');
  List.Add('Hans');
  List.Add('August');
  Result := List;
end;

procedure TMyClass.VerarbeiteNames;
var
  i: integer;
begin
  for i := 0 to GetNames.Count - 1 do
    Stringverarbeitung(GetNames[i]);
end;

[bernau]

Zitat:

Zitat von DeddyH (Beitrag 1361374)

Wegen genau dieser Problematik meide ich Funktionen, die Klasseninstanzen zurückgeben, welche erst innerhalb der Funktion erzeugt werden, wie der Teufel das Weihwasser.

Die Problematik ist ja nicht das Instanzieren innerhalb einer Funktion, sondern das Freigeben.

Gegen das Instanzieren innerhalb einer Funktion spricht überhaupt nichts, wenn es im Funktionsnamen ersichtlich ist. Ich verwende öfters Funktionen, die eine Instanz erzeugen. Kleines Beispiel: Eine Stringlist ist bei mir in den meisten Fällen sortiert und lässt keine Duplikate zu. Ist normalerweise ein Dreizeiler.

Delphi-Quellcode:

sl:=TStringlist.create;
sl.sorted := True;
sl.duplicates := dupignore;

Dafür gibt es bei mir folgende Funktion (so ähnlich)

Delphi-Quellcode:

Function TStringlistSortedDupIgnore_Create;
begin
  result := TStringlist.create;
  result.sorted := True;
  result.duplicates := dupignore;
end;

Aufrufen muss ich dann nur noch

Delphi-Quellcode:

sl := TStringlistSortedDupIgnore_Create;

Aus drei Zeilen mach eine. Finde ich sehr praktisch.

[himitsu]

Zitat:

Zitat von bernau (Beitrag 1361399)

Aus drei Zeilen mach eine. Finde ich sehr praktisch.

Delphi-Quellcode:

SL := TStringlist.Create;  SL.Sorted := True;  SL.Duplicates := dupIgnore;

:oops:

Delphi-Quellcode:

type
  TStringListHelper = class helper for TStringList
    //constructor CreateDup;
    class function CreateDup: TStringList; static;
  end;

class function TStringListHelper.CreateDup: TStringList;
begin
  Result := TStringlist.create;
  Result.Sorted := True;
  Result.Duplicates := dupIgnore;
end;

Und dann auch nur noch

Delphi-Quellcode:

SL := TStringList.CreateDup;

, was Dank CodeInsight auch leichter zu finden ist.

Wäre nur toll, wenn Embarcadero es nach 11 Jahren endlich mal hinbekommt, dass man mehrere Helper an eine Klasse binden kann, ohne dort krankhaft mit Vererbung arbeiten zu müssen, was oftmal garnicht möglich ist.



Ja, statt Listen geben ich auch gern dynamische Arrays zurück, was aber nur gut nutzbar ist, wenn die Liste in der Funktion gefüllt und außerhalb nicht verändert wird.

[bernau]

Zitat:

Zitat von himitsu (Beitrag 1361419)

Delphi-Quellcode:

SL := TStringlist.Create;  SL.Sorted := True;  SL.Duplicates := dupIgnore;

:oops:

Und der Codeformatter macht daraus wieder 3 Zeilen. (OK. Ich habe deinen Smiley gesehen)

Zitat:

Zitat von himitsu (Beitrag 1361419)

Und dann auch nur noch

Delphi-Quellcode:

SL := TStringList.CreateDup;

, was Dank CodeInsight auch leichter zu finden ist.

Wäre nur toll, wenn Embarcadero es nach 11 Jahren endlich mal hinbekommt, dass man mehrere Helper an eine Klasse binden kann, ohne dort krankhaft mit Vererbung arbeiten zu müssen, was oftmal garnicht möglich ist.

Helper setzte ich aus dem o.g. Grund nicht ein. Wenn man mehrere Helper pro Klasse verwenden kann, dann werde ich es mir überlegen.

Die Codevervollständigung geht auch so prima. Einfach TStringlist ohne Punkt tippen und dir werden nach Ctrl-Leer alle Funktionen angezeigt.

[Uwe Raabe]

Oder du arbeitest mit Delphi Berlin und hast

Delphi-Quellcode:

    /// <summary>
    ///    This constructor creates new string list with specified Duplicates,
    ///    Sorted and CaseSensitive property values.
    /// </summary>
    constructor Create(Duplicates: TDuplicates; Sorted: Boolean; CaseSensitive: Boolean); overload;

[bernau]

Zitat:

Zitat von Uwe Raabe (Beitrag 1361428)

Oder du arbeitest mit Delphi Berlin und hast

Delphi-Quellcode:

    /// <summary>
    ///    This constructor creates new string list with specified Duplicates,
    ///    Sorted and CaseSensitive property values.
    /// </summary>
    constructor Create(Duplicates: TDuplicates; Sorted: Boolean; CaseSensitive: Boolean); overload;

In welcher Klasse steht der Constuctor? Wenn dann in der "Ableitung" von TStringlist. Oder?
Dazu müsste aber der Constructor von TStringlist auch als Overload deklariert sein. Oder?
Kurz in die RTL (Berlin) geschaut.... Tatsächlich, das Create der TStringlist ist mit overload deklariert. Insgesamt gibt es Für die Stringlist fünf verschiedene Constructoren.

Man, man. Wenn man mal 2 Tage nicht in die RTL schaut ;-)

Seit wann ist das?

[Uwe Raabe]

Zitat:

Zitat von bernau (Beitrag 1361450)

Seit wann ist das?

Seit Delphi Berlin - ob erst mit dem letzten Update kann ich nicht sagen.

[sko1]

Das mit dem überladenen Construktor vom Create hätte ich ohne Euch nie gemerkt!

Genau so das TMemo.LoadFromFile(filename,Encoding) ...

Warum zeigt die Codevervollständigung nicht an dass es überladene Versionen des Create oder LoadFromFile gibt???

Mein Berlin 10.1 Update 2 tut dies jedenfalls nicht ;-(

Ciao
Stefan

[Uwe Raabe]

Zitat:

Zitat von sko1 (Beitrag 1361977)

Warum zeigt die Codevervollständigung nicht an dass es überladene Versionen des Create oder LoadFromFile gibt???

Mein Berlin 10.1 Update 2 tut dies jedenfalls nicht ;-(

Doch! Du musst nur erst die Methode auswählen. Wenn du dann die Klammer öffnest, bekommst du alle Überladungen angezeigt.