Reputation:
I have a delphi function that returns a TStringList, but when I return a value and try to use it I get a Access Violation Error i.e
myStringList := FuncStringList();
myStringList.Items.Count // <-- This causes an access violation
// function FuncStringList
function FuncStringList:TStringList;
var
vStrList:TStringList;
begin
vStrList := TStringList.Create;
...
// Fill the vStrList
Result := vStrList
vStrList.Free; //<- when i free here, this function will cause AccessViolation
end;
How can I return the TStringList and still free it in the local function?
Upvotes: 14
Views: 14521
Reputation: 21221
Simple answer (with examples):
When you do
Result := vStrList
you assign vStrList to Result. At this moment vStrList and Result ARE THE SAME THING! So, in the next line of code, when you free vStrList, you free also Result (well, this is not TECHNICALLY accurate but I used it to keep the explanation simple). This is why you get an AV when you try to use the result of the function. The Result was destroyed when you freed vStrList.
So, this will solve your problem:
function FuncStringList:TStringList;
begin
Result := TStringList.Create;
// Do stuff with Result
// never free (here, in this function) the Result
end;
The caller of FuncStringList will HAVE TO free "Result".
You call it like this:
myStringList := FuncStringList;
try
myStringList.Items.Count
finally
FreeAndNil(myStringList); <------------- NOW you can free "Result"
end;
.
Another example:
function makelist: tstringlist;
begin
result := Tstringlist.create;
result.add('1');
result.add('2');
end;
procedure TForm1.Button_GOOD_Click(Sender: TObject);
var list : tstringlist;
begin
list := makelist;
DoStuff(list);
list.free; //ok
end;
procedure TForm1.Button_BAD_Click(Sender: TObject);
begin
listbox1.items.Assign(makelist); // <---- memory leak here because you forgot to free
end;
I put this note here before anyone will start 'picking' on my explanation. I used some 'shortcuts' in my explanation in order to avoid complex concepts (such as pointer assignment) keep things very simple. @gath asked a basic question which means that he is still learning the basics of programming.
Upvotes: 4
Reputation: 31
Either as Out variable.
function GetList(Parameter1: string; out ResultList: TStringList): boolean;
begin
// either
if not Assigned(ResultList) then
raise Exception.Create('Out variable is not declared.');
// or
Result := False;
if not Assigned(ResultList) then
Exit;
ResultList.Clear;
ResultList.Add('Line1');
ResultList.Add('Line2');
//...
Result := True;
end;
Or as string.
function GetList(Parameter1: string): string;
var
slList: TStringList;
begin
slList := TStringList.Create;
try
slList.Clear;
slList.Add('Line1');
slList.Add('Line2');
//...
Result := slList.Text;
finally
slList.Free;
end;
end;
.
procedure Main;
var
slList: TStringList;
begin
slList := TStringList.Create;
try
// either
GetList(Parameter1, slList);
// or
slList.Text := GetList(Parameter1);
// process slList...
finally
slList.Free;
end;
end;
Upvotes: 0
Reputation: 1388
both are reffering to same memory,if you free it both will get freed.......
Upvotes: 0
Reputation: 3820
A policy I have with such situations is to pass the stringlist content through the text property and just pass the string returned to the function. This way there's no need to discuss who release who. Of course, you have to do a little more coding, but it's safer. The example is an adaptation of the Ken White's one :
var
SL: TStringList;
Aux: String;
begin
SL := TStringList.Create;
try
SL.Text := ProcToFillStringList;
//Do something with populated list
finally
SL.Free;
end;
end;
// It receives a default param, in the case you have to deal with
// StringList with some previous content
function ProcToFillStringList(SListContent: String = ''):String;
// Do the stuff you need to do with the content
end;
An exception is when all you have is the object and there's no way to retrieve the content on it through a safe type (in this case, strings); then I follow Ken White's idea.
Upvotes: 0
Reputation: 913
Another possibility is to use a dynamic array instead of a TStringList. Since arrays are reference counted, you will never have to worry about freeing it.
Upvotes: 0
Reputation: 125748
As Smasher said, you can't free it; the code calling the function that returns the object is responsible for destroying it.
This is bad code design, by the way, as it makes it confusing as to who allocates and frees. A much better way to do it would be to have the caller create the object and pass it in to the function. That way, the code that creates it also frees it. Something like this:
var
SL: TStringList;
begin
SL := TStringList.Create;
try
ProcToFillStringList(SL);
//Do something with populated list
finally
SL.Free;
end;
end;
// Note I've made the parameter a TStrings and not a TStringList. This allows
// passing a TMemo.Lines or a TListBox or TComboBox Items as well.
procedure ProcToFillStringList(const SList: TStrings);
// Do whatever populates the list with SList.Add()
end;
Now there's no confusion over who does what - the same code that creates the object is responsible for freeing it. And the code, IMO, is much clearer to read and maintain.
Upvotes: 31
Reputation: 16620
How can i return the TStringList and still free it in the local function?
You can't. If you free it in the local function, you can't use the return value. Result and vStrList point to the same TStringList object in memory. TStringList is a class and
Result := vStrList
does therefore not copy the string list, but only copies the reference.
So, instead you should free the string list in the calling context after you're done working with it or pass the string list as a parameter to your function like this
procedure FuncStringList (StringList : TStringList);
and let the calling code create and free the string list. As pointed out by the other answers, this is the preferable way, since it makes ownership very clear.
Upvotes: 27
Reputation: 84650
Simple answer: you can't. Why are you trying to? Is it because you've learned that you need to free every object you create in the same function in which they're created? That's generally correct, but not always, and this is one of the exceptions to the rule. A better way to put it is that every object must be freed by its owner.
If you have a function that generates an object, like this one, but then passes it on to another function, it doesn't take ownership of the object. Remove the call to free and document it, so you (and anyone else who uses this function) will realize that it creates a new object that the code that calls it has to take ownership of.
Upvotes: 6
Reputation: 24938
You simply cannot free something and then expect to reference it later. That is the wrong way. You have two basic options:
The first option seems simpler, keeps the interface to the function smaller, etc. The second option makes usage less error prone because it's intuitive to the caller that it's responsible for managing the object.
Upvotes: 2
Reputation: 2207
Don't free the object before you are done invoking the methods on it. You are currently invoking the Count method on a destroyed object, hence the error.
Why don't you create the string list in the calling function instead, and pass its reference to the method that fills it? Or make the string list a member of a class, and free it when you free the class that owns it?
Upvotes: 0