[Delphi] Functie met TStringlist als result

[Verwijderd]

Kort vraagje:

Ik heb in een kleine applicatie die ik momenteel aan het maken ben een functie waarbij ik graag een TStringlist als result terug geef. Nou heb ik altijd geleerd dat objecten die je aanmaakt je ook weer vrj moet geven; na iedere create volgt vroeg of laat een free. Echter, hoe geef ik een functie result vrij??

Enige monitoring leert mij trouwens dat het geheugen gebruik van de applicatie gestaag oploopt. Zou dit kunnen komen door het feit dat ik deze functie aan blijf roepen?

[MaTriCX]

Je kunt proberen om de TStringlist mee te geven als variabel argument (var lijst: TStringlist).
Op deze manier kun je buiten de functie om de Stringlist aanmaken en vrijgeven en kun je als resultaat van de functie een int meegeven (0 / 1 in geval van gelukt of niet gelukt).

[Verwijderd]

Hmm, is een optie. Ik ga er even mee stoeien.. Kijken of het werkt.

[CyeZ]

Een andere mogelijkheid is om de functie aanroep in de plaats van de Create te gebruiken in je aanroepende functie, aangezien je de stringlist al je je functie creeerd.

Delphi:

Function geefStringlist: TStringlist;
begin
  Result := TStringlist.Create;
end;

Procedure gebruikStringlist;
var
  oStringlist: TStringlist;
begin
  oStringlist := geefStringlist;
  try
    {...}
  finally
    if Assigned(oStringlist) then begin
      FreeAndNil(oStringList);
    end;
  end;
end;

[Verwijderd]

De standaard manier in Delphi is om de lijst door de aanroepende kant te laten beheren, en 'm mee te geven als parameter aan de procedure of functie.
(De parameter hoeft niet als 'var' doorgegeven te worden, omdat objecten toch al altijd by reference worden doorgegeven)

Delphi:

procedure DoeIetsMetLijst(AList: TStrings);
begin
  AList.BeginUpdate;
  try
    AList.Clear;
    // doe iets met AList
  finally
    AList.EndUpdate;
  end;
end;

Voordeel hiervan is dat je 'm ook direct kunt gebruiken op items van een listbox, lines van een memo, en iedere andere TStrings afgeleide.

[LordLarry]

En met de methode van afterlife is het duidelijk wie m vrij moet geven, namelijk degene die m creeerd. DoeIetsMetLijst creeerd niets en hoeft dus ook niets vrij te geven.

[Verwijderd]

Echter, hoe geef ik een functie result vrij??

Wanneer je die functie (laten we 'm HaalLijst noemen) toch een TStringList wilt laten aanmaken, dan is 't ook vrij simpel:

Delphi:

with HaalLijst do try
  // doe leuke dingen met de stringlist die je teruggekregen hebt
finally
  Free;
end;

Als die 'with' structuur niet past in je code, dan kun je het resultaat van HaalLijst natuurlijk ook aan een locale variabele toekennen, en die aan 't eind free-en.
Maar mijn voorkeur gaat met stip uit naar m'n eerdere oplossing. Een functie hoort in principe geen nieuw aangemaakt object terug te geven, dat is de taak van de constructor van z'n class.

Soms kom je er niet onderuit, en soms is 't zelfs heel handig (bv. een CreateQuery functie die een TADOQuery aanmaakt en ook maar even de connection zet, dan is een afgeleide subclass die hetzelfde doet in z'n constructor doodgewoon overkill), maar ik probeer het toch zoveel mogelijk te beperken.

[Verwijderd]

Verwijderd schreef op donderdag 31 maart 2005 @ 16:57:
Kort vraagje:

Ik heb in een kleine applicatie die ik momenteel aan het maken ben een functie waarbij ik graag een TStringlist als result terug geef. Nou heb ik altijd geleerd dat objecten die je aanmaakt je ook weer vrj moet geven; na iedere create volgt vroeg of laat een free. Echter, hoe geef ik een functie result vrij??

Eh?

Je geeft het object toch gewoon vrij in de aanroepende code? (dus, de code die de referentie naar de TStringList beheert)

Delphi:

MyList := FunctieDieStringListUitPoept(EenParameter);
try
  // ... Do stuff
finally
  MyList.Free;
end;

Enige monitoring leert mij trouwens dat het geheugen gebruik van de applicatie gestaag oploopt. Zou dit kunnen komen door het feit dat ik deze functie aan blijf roepen?

Als je een memory leak hebt zou het best eens kunnen dat daardoor je geheugengebruik oploopt

[Tomatoman]

Verwijderd schreef op vrijdag 01 april 2005 @ 01:38:
Als je een memory leak hebt zou het best eens kunnen dat daardoor je geheugengebruik oploopt

Goed punt heb je daar. De correcte code wordt dan ook - heel onlogisch - zoiets als...

... en terwijl ik dit zit te typen constateer ik dat een falende aanroep van FunctieDieStringListUitPoept per definitie een geheugenlek kan veroorzaken. Stel dat FunctieDieStringListUitPoept er als volgt uitziet:

Delphi:

function FunctieDieStringListUitPoept: TStrings;
begin
  Result := TStringList.Create;
  Result.Strings[-1] := 'Dit gaat niet goed'; // veroorzaakt een exception
end;

Wat je ook doet met exception blocks rond code die FunctieDieStringListUitPoept aanroept, een geheugenlek als gevolg van een TStringList die nooit meer wordt vernietigd is niet te voorkomen.

[ Voor 4% gewijzigd door Tomatoman op 01-04-2005 02:03 ]

[CyeZ]

tomatoman schreef op vrijdag 01 april 2005 @ 02:03:
[...]
Goed punt heb je daar. De correcte code wordt dan ook - heel onlogisch - zoiets als...

... en terwijl ik dit zit te typen constateer ik dat een falende aanroep van FunctieDieStringListUitPoept per definitie een geheugenlek kan veroorzaken. Stel dat FunctieDieStringListUitPoept er als volgt uitziet:

Delphi:

function FunctieDieStringListUitPoept: TStrings;
begin
  Result := TStringList.Create;
  Result.Strings[-1] := 'Dit gaat niet goed'; // veroorzaakt een exception
end;

Wat je ook doet met exception blocks rond code die FunctieDieStringListUitPoept aanroept, een geheugenlek als gevolg van een TStringList die nooit meer wordt vernietigd is niet te voorkomen.

Dat kun je toch best op de volgende manier verhelpen?

Delphi:

function FunctieDieStringListUitPoept: TStrings;
begin
  Result := TStringList.Create;
  try
    Result.Strings[-1] := 'Dit gaat niet goed'; // veroorzaakt een exception
  except
    if Assigned(Result) then begin
      Result.Free;
    end;
  end;
end;

Oftewel, als je een functie op deze manier op wilt bouwen moet je ook zorgen dat deze functie zelf het object weer opruimt als er een exceptie optreed.Daarna zou je eventueel diezelfde exceptie weer opnieuw kunnen raisen zodat de aanroepende functie hem voor z'n kiezen krijgt. Het result object is dan in ieder geval op opgeruimt.

[Verwijderd]

Als een functie een object terug geeft is de ontvanger van het object verantwoordelijk voor het opruimen daarvan. Deze aanpak geldt in het alagemeen voor functies die "zaken" teruggeven, zoals bv system resources of COM-objecten

Aangezien alle Delphi-objecten heap-based zijn, moet je ze dan idd altijd zelf naderhand free-en. File-handles moet je zelf vrijgeven na gebruik. COM-objecten kan je niet expliciet free-en, maar je geeft wel met obj.Release() aan dat jij het object niet meer nodig hebt. En zo voorts.

[Verwijderd]

tomatoman schreef op vrijdag 01 april 2005 @ 02:03:
[...]
... en terwijl ik dit zit te typen constateer ik dat een falende aanroep van FunctieDieStringListUitPoept per definitie een geheugenlek kan veroorzaken. Stel dat FunctieDieStringListUitPoept er als volgt uitziet:

Wat je ook doet met exception blocks rond code die FunctieDieStringListUitPoept aanroept, een geheugenlek als gevolg van een TStringList die nooit meer wordt vernietigd is niet te voorkomen.

Helemaal waar, natuurlijk. Op de inhoud van de functie was ik verder nog niet ingegaan, maar dat zal ik nu doen.

CyeZ schreef op vrijdag 01 april 2005 @ 05:04:

Delphi:

function FunctieDieStringListUitPoept: TStrings;
begin
  Result := TStringList.Create;
  try
    Result.Strings[-1] := 'Dit gaat niet goed'; // veroorzaakt een exception
  except
    if Assigned(Result) then begin
      Result.Free;
    end;
  end;
end;

Bijna goed. De test op Assigned(Result) is eigenlijk niet nodig (is toch altijd waar), en de exception verdwijnt nu, dus weet de aanroeper niet meer dat er iets mis ging (hij krijgt zelfs een dangling pointer terug -- de waarde in Result is nu bogus -- dus crasht met een AccessViolation zodra hij het object probeert aan te spreken).

Delphi:

function FunctieDieStringListUitPoep : TStrings;
begin
  Result := TStringList.Create;
  try
    // ... code die een exception kan geven
  except
    Result.Free;
    raise;
  end;
end;

De bedoeling is natuurlijk dat het alloceren van de stringlist in FunctieDieStringListUitPoept, bij gebrek aan een beter woord, "atomair" plaatsvindt^[1]. Dat wil zeggen dat die functie wel de garantie moet geven dat als hij

• zonder exception afloopt, de stringlist gealloceerd is; en als hij
• met exception afloopt, de stringlist niet gealloceerd is.

En het werkt op precies dezelfde manier als de functie bijvoorbeeld meerdere resources tegelijk moet alloceren: aan het eind van de functie zijn alle resources tegelijk en succesvol gealloceerd, of als er iets mis ging (exception) zijn ze allemaal niet gealloceerd (of weer vrijgegeven).

Hierbij treedt iets op wat ik "assumption of responsibility" zou willen noemen: op elk moment in de levensduur van een resource (object/stuk geheugen/filehandle, whatever), heeft één routine de verantwoordelijk voor deze resource. Zolang deze routine verantwoordelijk is, dient hij ervoor te zorgen dat de resource niet gelekt wordt, maar vrijgegeven wordt wanneer dat toepasselijk is. Een routine kan wel verantwoordelijkheid over het object overdragen, door het aan een andere routine over te dragen -- middels een aanroep naar een andere routine, of door het teruggeven van de resource als functieresultaat. Daarna dient de volgende routine in de ketting dus te zorgen dat de resource tzt. weer vrijgegeven wordt. (Hierbij dient de aangeroepen routine natuurlijk wel te wéten dat hij verantwoordelijk wordt, anders is er geen routine meer verantwoordelijk en zal de resource onherroepelijk lekken. De aangeroepen routine moet dus de verantwoordelijkheid aannemen).

Op deze manier heb je duidelijkheid welke resource door wie vrij moet worden gegeven, en voorkom je geheugenlekken.


_{[1] Niet echt de goede term; heeft iemand een betere...?}

[JozyDaPozy]

Ik doe dit soort dingen altijd met een var. Is echt het makkelijkst..
stukje voorbeeldcode:

Delphi:

function geef_bestands_lijst(pad : string; zet_pad_ervoor : Boolean; var lijst : TStringList; moeten_groter_dan_0_bytes_zijn : boolean = false) : Integer;
var
    sr : tsearchrec;
    padstring : string;
begin
    padstring := extractfilepath(pad);
    lijst.Clear;
    if findfirst(pad,faanyfile,sr) = 0 then
    begin
        repeat
            if (sr.name <> '.') and (sr.name <> '..') then
            begin
                if (sr.Attr and faDirectory) > 0 then
                    continue;

                if moeten_groter_dan_0_bytes_zijn then
                begin
                    if sr.Size <= 0 then
                        continue;
                end;

                if zet_pad_ervoor then
                    lijst.Add(padstring + sr.name)
                else
                    lijst.Add(sr.Name);
            end;
        until findnext(sr) <> 0;
    end;
    lijst.Sort;

    FindClose(sr);

    Result := lijst.Count;
end;

En zoiets roep je dan aan als volgt:

Delphi:

var
    bestands_lijst : tstringlist;
    i : integer;
begin
    bestands_lijst := TstringList.create;
    geef_bestands_lijst('c:\*.*',true,bestands_lijst);
    for i := 0 to bestands_lijst.count-1 do
    begin
        // blabla
    end;
    bestands_lijst.free;
end;

[Verwijderd]

Verwijderd schreef op vrijdag 01 april 2005 @ 12:45:
[...]
"atomair" plaatsvindt^[1]
[1] Niet echt de goede term; heeft iemand een betere...? :)[/sub]

Wat vind je van "transactioneel" ?

[Verwijderd]

JozyDaPozy, ik heb je functie een klein beetje verspijkerd...

Delphi:

function geef_bestands_lijst(pad: string; zet_pad_ervoor: Boolean; lijst: TStrings; 
  moeten_groter_dan_0_bytes_zijn: boolean = false) : Integer; 
var 
  sr: TSearchRec; 
  padstring: string; 
begin
  lijst.BeginUpdate;
  try
    padstring := ExtractFilePath(pad); 
    lijst.Clear; 
    if FindFirst(pad, faAnyFile, sr) = 0 then 
    begin 
      repeat 
        if ((sr.Attr and faDirectory) = 0) {ignore directories} and
           ((not moeten_groter_dan_0_bytes_zijn) or (sr.Size > 0)) then
        begin
          if zet_pad_ervoor then 
            lijst.Add(padstring + sr.name) 
          else 
            lijst.Add(sr.Name); 
        end; 
      until FindNext(sr) <> 0; 
    end; 
    lijst.Sort;
  finally
    lijst.EndUpdate;
  end;
  FindClose(sr); 
  Result := lijst.Count; 
end;

Ik zag die 'continue's staan, en dacht dat moet mooier kunnen.
IMHO is Continue bijna net zo erg als Goto...

O ja, probeer er een gewoonte van te maken om try/finally blokken te gebruiken, waarbij je in het finally-stuk de aangemaakte objecten opruimt. Dit voorkomt memory leaks wanneer er onderweg iets mis is gegaan.

[Robbemans]

Rule of thumb:

GEEF NOOIT EEN OBJECTINSTANTIE TERUG ALS FUNCTIERESULTAAT

Ik kan het niet vaak genoeg herhalen, want het levert vaker problemen op dan dat je er gemak van hebt.

Ook het meegeven van een object als VAR parameter is niet aan te raden, omdat je naar mijn idee niet het object wil veranderen, maar de eigenschappen ervan.

Zoals al gezegd is een aanroep als volgt het beste:

procedure MyProc(AItems: TStrings);

of, als je moet weten dat er iets gewijzigd is aan de lijst, dan is het wellicht handig er toch een functie van te maken en een boolean terug te geven.

[Verwijderd]

Robbemans schreef op vrijdag 01 april 2005 @ 14:23:
Rule of thumb:

GEEF NOOIT EEN OBJECTINSTANTIE TERUG ALS FUNCTIERESULTAAT

Van de ene kant ben ik het daarmee eens, van de andere niet.

Over het algemeen vind ik het logischer (en fijner), als een functie die een lijst teruggeeft (of wat dan ook), dit object aanmaakt en aan mij geeft.

Van de andere kant levert het in non-garbage collected talen wel weer uitzonderingsgevallen op waar je rekening mee moet houden (zoals hierboven beschreven).

Ik vind het een lastige keuze. En aangezien ik een eigenwijze donder ben, blijf ik het stug op de eerste manier doen .

Verwijderd schreef op vrijdag 01 april 2005 @ 14:04:
Wat vind je van "transactioneel" ?

Perfect! Bedankt voor deze correctie .

[Tomatoman]

Verwijderd schreef op vrijdag 01 april 2005 @ 14:14:
JozyDaPozy, ik heb je functie een klein beetje verspijkerd...

Delphi:

{ code met FindFirst/FindNext/FindClose, inclusief een bug }

Deze code bevat dezelfde fout als de code van JozyDaPozy, namelijk verkeerd gebruik van de FindXXX functies. FindClose dien je alleen uit te voeren als FindFirst de waarde 0 retourneert:

Delphi:

  if FindFirst(MijnSearchRec) = 0 then
    try
      repeat
        { doe iets met MijnSearchRec }
      until FindNext(MijnSearchRec) <> 0;
    finally
      FindClose(MijnSearchRec);
    end;

De analogie van FindFirst met een functie die een object instantieert en retourneert is groot. Ook daarbij hangt het vervolg af van de vraag of de functie succesvol is geweest. Bij FindFirst doe je dat door te controleren of het functieresultaat ongelijk is aan 0. Bij de in eerdere posts genoemde FunctieDieStringListUitPoept moet je iets soortgelijks verzinnen, bijvoorbeeld door nil te retourneren als de functie faalt. Dit gedrag beschrijf je vervolgens netjes in de documentatie, want het blijkt niet direct uit de functiedeclaratie.

De oplossing om nil te retourneren als het creëren van de TStringList mislukt schept direct een volgend probleem, namelijk dat je geen exception kunt afhandelen die inzichtelijk maakt wat er precies is foutgegaan. Dat is niet ideaal. Wil je toch toestaan dat FunctieDieStringListUitPoept een exception kan genereren en bovendien geen geheugenlek kan veroorzaken (OneOfBorg noemt de functie atomair), dan kan dat heel eenvoudig:

Delphi:

function FunctieDieStringlistUitPoept: TStrings;
begin 
  Result := TStringList.Create;
  try
    { hier doe je wat dingen die een exception kunnen opleveren }
  except
    Result.Free; // geheugenlek voorkomen als de functie faalt
    raise;       // niet verdoezelen waarom de functie faalt
  end;
end;

var
  S: TStrings;
begin
  S := FunctieDieStringlistUitPoept;
  try
    { doe iets spannends met S }
  finally
    S.Free;
  end;
end.

Deze code voldoet niet aan de 'assumption of responsibility' (mooi verwoord OneOfBorg ) en is ook enigszins lastig te begrijpen. In eerste instantie lijkt het net of S niet geïnstantieerd wordt. Je moet in de documentatie of de implementatie van FunctieDieStringlistUitPoept duiken om te begrijpen wat er nou eigenlijk gebeurt.

Technisch is deze oplossing à la FindFirst/FindNext/FindClose prima, maar doordat hij afwijkt van wat een Delphiprogrammeur gewend is, werkt hij bugs in de hand. Ook om die reden vind ik het geen goede omlossing.

[ Voor 15% gewijzigd door Tomatoman op 02-04-2005 01:28 ]

[Antonius]

Als het iets heel algemeens is wat die functie doet, zou je kunnen erven van TStringList (of een verwante klasse) en deze nieuwe klasse een speciale constructor geven die doet wat je anders in FunctieDieStringlistUitpoept zou stoppen. Zit in een heel andere richting dan alle voorgaande opties, maar kan wellicht ook een oplossing zijn. Ligt er m.i. nogal aan of het iets heel generieks is wat je doet, of juist iets wat vrij nauw verweven is met andere delen van je programma. In het laatste geval zou ik deze optie zeker niet kiezen, maar als je iets tamelijk generieks doet wat los staat van de rest van je programma, dan kan het een aardige OO oplossing zijn.

Dat daarna TMarkVMStringList.Create(StopDitErin) een try-finally-end nodig heeft, zou iedere Delphi programmeur moeten snappen.

[Verwijderd]

tomatoman schreef op zaterdag 02 april 2005 @ 01:23:
Deze code bevat dezelfde fout als de code van JozyDaPozy, namelijk verkeerd gebruik van de FindXXX functies. FindClose dien je alleen uit te voeren als FindFirst de waarde 0 retourneert:

Uit SysUtils.pas van Delphi 6:

Delphi:

{ FindFirst searches the directory given by Path for the first entry that
  matches the filename given by Path and the attributes given by Attr. The
  result is returned in the search record given by SearchRec. The return
  value is zero if the function was successful. Otherwise the return value
  is a system error code. After calling FindFirst, always call FindClose.
  FindFirst is typically used with FindNext and FindClose as follows:

    Result := FindFirst(Path, Attr, SearchRec);
    while Result = 0 do
    begin
      ProcessSearchRec(SearchRec);
      Result := FindNext(SearchRec);
    end;
    FindClose(SearchRec);

  where ProcessSearchRec represents user-defined code that processes the
  information in a search record. }

FindClose aanroepen wanneer FindFirst geen resultaat heeft opgeleverd kan dus geen kwaad. De FindClose wrapper van Delphi vangt dat netjes af:
"if F.FindHandle <> INVALID_HANDLE_VALUE then..."

[Tomatoman]

Verwijderd schreef op zondag 03 april 2005 @ 00:51:
[...]
FindClose aanroepen wanneer FindFirst geen resultaat heeft opgeleverd kan dus geen kwaad.

Dankjewel, dat wist ik niet.