Recent

Author Topic: Proper way to free an object returned in a function  (Read 3002 times)

rwebb616

  • Full Member
  • ***
  • Posts: 133
Proper way to free an object returned in a function
« on: May 18, 2021, 04:56:27 am »
I have a function that returns a TStringList:
Code: Pascal  [Select][+][-]
  1. function TData.GetCategories: TStringList;
  2. var
  3.   i : integer;
  4. begin
  5.   result := TStringlist.Create;
  6.   result.Sorted:=true;
  7.   result.Duplicates:=dupIgnore;
  8.   for i:=0 to Words.Count - 1 do
  9.     result.Add(Tword(words[i]).Category);
  10. end;

I call it as such:
Code: Pascal  [Select][+][-]
  1. listbox1.items := gamedata.getcategories

heaptrc is complaining about getcategories at the line:
Code: Pascal  [Select][+][-]
  1. result.Add(Tword(words[i]).Category);

First, is it the Tstringlist assigned to Result that is the issue?  and if so, how would I go about freeing it?


Handoko

  • Hero Member
  • *****
  • Posts: 5131
  • My goal: build my own game engine using Lazarus
Re: Proper way to free an object returned in a function
« Reply #1 on: May 18, 2021, 05:23:31 am »
I usually will avoid creating new object as the function result. Instead I use
procedure SomeProcess(out Data: TData);

I remember there were a similar topic some months ago, that discussion is worth to read.
« Last Edit: May 18, 2021, 05:28:59 am by Handoko »

Leledumbo

  • Hero Member
  • *****
  • Posts: 8746
  • Programming + Glam Metal + Tae Kwon Do = Me
Re: Proper way to free an object returned in a function
« Reply #2 on: May 18, 2021, 05:25:18 am »
I call it as such:
Code: Pascal  [Select][+][-]
  1. listbox1.items := gamedata.getcategories
...
First, is it the Tstringlist assigned to Result that is the issue?  and if so, how would I go about freeing it?
Bad idea, use ListBox1.Items.Assign instead, assign the function result to a local variable first then free it after calling .Assign:
Code: Pascal  [Select][+][-]
  1. lcats := gamedata.getcategories;
  2. with lcats do
  3.   try
  4.     listbox1.items.assign(lcats);
  5.   finally
  6.     lcats.Free;
  7.   end;
  8.  
heaptrc is complaining about getcategories at the line:
Code: Pascal  [Select][+][-]
  1. result.Add(Tword(words[i]).Category);
Need to know what TWord.Category is first. If it's also a function, including property mapped one, then it's the same thing as above.

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: Proper way to free an object returned in a function
« Reply #3 on: May 18, 2021, 05:26:37 am »
This assignment:
Code: Pascal  [Select][+][-]
  1.     listbox1.items := gamedata.getcategories;

Calls the property setter:
Code: Pascal  [Select][+][-]
  1. procedure TCustomListBox.SetItems(Value: TStrings);
  2. begin
  3.   if (Value <> FItems) then
  4.   begin
  5.     LockSelectionChange;
  6.     FItems.Assign(Value);
  7.     UnlockSelectionChange;
  8.   end;
  9. end;

As you can see, it simply copies the items from the TStringList. Simply call Free after the assignment:
Code: Pascal  [Select][+][-]
  1.     categories:=gamedata.getcategories;
  2.     listbox1.items := categories;
  3.     categories.Free;

A better way is to pass listbox1.items to gamedata.fillcategories.

eny

  • Hero Member
  • *****
  • Posts: 1634
Re: Proper way to free an object returned in a function
« Reply #4 on: May 21, 2021, 01:55:48 pm »
A better way is to pass listbox1.items to gamedata.fillcategories.
+1
All posts based on: Win10 (Win64); Lazarus 2.0.10 'stable' (x64) unless specified otherwise...

Warfley

  • Hero Member
  • *****
  • Posts: 1499
Re: Proper way to free an object returned in a function
« Reply #5 on: May 21, 2021, 02:41:20 pm »
Having classes as return types for functions is a bad idea, because it can make things very complicated.

For example:
Code: Pascal  [Select][+][-]
  1. function foo: TSomeClass;
  2. begin
  3.   Result := TSomeClass.Create;
  4.   Result.DoSomething;
  5. end;

What happens if that DoSomething throws an exception? Well the instance reference is lost. So what you need to do is to always have a try-except block in such a function:
Code: Pascal  [Select][+][-]
  1. function foo: TSomeClass;
  2. begin
  3.   Result := TSomeClass.Create;
  4.   try
  5.     Result.DoSomething;
  6.   except on e: Exception do
  7.     FreeAndNil(Result);
  8.     raise e;
  9.   end;
  10. end;

If you simply pass that instance as a parameter:
Code: Pascal  [Select][+][-]
  1. procedure foo(inst: TSomeClass);
  2. begin
  3.   inst.DoSomething;
  4. end;
This is not required, as you typically have a try-finally block on the callsite anyway:
Code: Pascal  [Select][+][-]
  1. someInstance := TSomeClass.Create;
  2. try
  3.   foo(someInstance);
  4.   ...
  5. finally
  6.   someInstance.Free;
  7. end;

So giving the instance as parameter makes it much easier because you don't need a try-except block, which is required if the instance is created by the function, and more importantly, it is also cleaner because creation and destruction happens in the same block/function

 

TinyPortal © 2005-2018