Board index » delphi » Functions that return TStringLists

Functions that return TStringLists

I have a function that parses a delimited string and returns a
TStringList. The Result is created in the function.  I just realized
that this function is leaking depending on how I used it.  Eg:

This leaks:
        MyListBox.Items := ParseString(MyString);

But this doesn't:
        TmpStringList := ParseString(MyString);
        MyListBox.Items := TmpStringList;
        TmpStringList.Free;

I think this means that MyListBox.Items := ParseString(MyString); is
actually copying the my function's TStringList result into it's
another TStringList rather than just pointing to the one created in my
function?  Using a temp TStringList seems cludgy.  Can I do what I
want in one statement?

Thanks,
Steve

 

Re:Functions that return TStringLists


Hi Stephen!

Quote
> MyListBox.Items := ParseString(MyString);

>But this doesn't:
> TmpStringList := ParseString(MyString);
> MyListBox.Items := TmpStringList;
> TmpStringList.Free;

try:
MyListBox.Items.Assign(ParseString(MyString));

Re:Functions that return TStringLists


Quote
Stephen Smith <smi...@removethis.imagesoftcorp.com> wrote:
> I have a function that parses a delimited string and returns a
> TStringList. The Result is created in the function.  I just realized
> that this function is leaking depending on how I used it.  Eg:
> This leaks:
>    MyListBox.Items := ParseString(MyString);
> But this doesn't:
>    TmpStringList := ParseString(MyString);
>    MyListBox.Items := TmpStringList;
>    TmpStringList.Free;

That's a problem with properties - they hide what really happens when you
write MyListBox.Items:=TmpStringList, in fact it does a
MyListBox.Items.Assign(TmpStringList) and therefore don't free the
Stringlist. But if they would work the other way round - inserting the pointer
directly either the original value of Mylistbox.items won't be freed or you
have two components using the same TStringlist
(Listbox1.items:=listbox2.items) and you're on the highway to a GPF.

Quote
> function?  Using a temp TStringList seems cludgy.  Can I do what I
> want in one statement?

AFAIK no. I normally do a

TmpStringlist:=NIL;
try
  TmpStringlist:=Parsestring(Mystring);
  mylistbox.items:=tmpstringlist;
finally
  tmpstringlist.free;
  end;

for occasions like this. Normally you need to write the free for each object
you create yourself (and which has no owner for the windowed stuff).

Bye,
  Andy

--
Andreas H"orstemeier                       | "We are not interested in
email: a...@scp.de                         | little green men, but in little
       a...@hoerstemeier.de                | green women."
www:   http://www.westend.de/~hoerstemeier | (Edwin "Buzz" Aldrin)
fido:  2:2452/108     ICQ: 1661968         |

Re:Functions that return TStringLists


In article <34dabab8.19698...@news.mindspring.com>,
smi...@removethis.imagesoftcorp.com says...

Quote
> I have a function that parses a delimited string and returns a
> TStringList. The Result is created in the function.  I just realized
> that this function is leaking depending on how I used it.  

Yep. That's the design you've made. Basically, you have a problem
of ownership. Who/what owns the string list? The owner is the
thing (object/procedure/whatever) that's responsible for deleting
the object. A good rule is to have the owner also create the
object, or at least that they're at the same 'level' in the code.
The way you've coded, the ParseString creates the object, but the
user of ParseString is the owner. This will quite often cause
problems. Especially when somebody else is using your ParseString
routine...

One thing you could do is to make an object deriving from
TStringList (or TStrings) containing your parse function. That
way the ownership of the list is more clear for someone reading
the code.

M.

--
Author of several unknown utilities for DOS and Windows.
http://www.delfidata.no/users/~martin
mailto:martin.lars...@delfidata.no.YYY (remove last four chars.)

Re:Functions that return TStringLists


Try
mylistbox.items.assign(parsestring(mystring)); That work.

Quote
On Fri, 6 Feb 1998, Stephen Smith wrote:
> I have a function that parses a delimited string and returns a
> TStringList. The Result is created in the function.  I just realized
> that this function is leaking depending on how I used it.  Eg:

> This leaks:
>    MyListBox.Items := ParseString(MyString);

Re:Functions that return TStringLists


Steve -

I agree with what Andy and Martin said.  Here, we never have a function return a
new TStringList.  If it returns one, then you don't need to free it.  I would
implement your ParseString method as a procedure with the string to parse and
the TStringList to parse it into as parameters.  Then the method calling
ParseString create the TStringList and can free it when it is done.  Optionally,
you could pass in the string and the list box you want the items put in to.
Then ParseString could create the TStringList, fill it, assign it to the list
box, and finally free it.

--
Clayton Neff
Software Project Leader
The Personal Marketing Company, Inc.
11843 West 83rd Terrace
Lenexa, KS  66214
(913)492-0322
clay...@tpmco.com

Re:Functions that return TStringLists


In article <Pine.HPP.3.91.980206075229.27747A-
100...@fhs.csu.McMaster.CA>, no...@fhs.csu.McMaster.CA says...

Quote
> Try
> mylistbox.items.assign(parsestring(mystring)); That work.

No it won't.
You're just doing the same thing TCustomListBox.SetItems is
doing. So there's no difference between MyListbox.Items :=
ParseString(MyString) and the code above.

M.

--
Author of several unknown utilities for DOS and Windows.
http://www.delfidata.no/users/~martin
mailto:martin.lars...@delfidata.no.YYY (remove last four chars.)

Re:Functions that return TStringLists


What you could do is move it to a procedure...and one of the Parameters
could be a StringList...and just pass the correct StringList...that way you
don't have to create a stringlist in your function or procedure...which is a
no no...

ParseString(MyString:string, MyListBox.Items);

Rick Peterson

Quote
Stephen Smith wrote in message <34dabab8.19698...@news.mindspring.com>...
>I have a function that parses a delimited string and returns a
>TStringList. The Result is created in the function.  I just realized
>that this function is leaking depending on how I used it.  Eg:

>This leaks:
> MyListBox.Items := ParseString(MyString);

>But this doesn't:
> TmpStringList := ParseString(MyString);
> MyListBox.Items := TmpStringList;
> TmpStringList.Free;

>I think this means that MyListBox.Items := ParseString(MyString); is
>actually copying the my function's TStringList result into it's
>another TStringList rather than just pointing to the one created in my
>function?  Using a temp TStringList seems cludgy.  Can I do what I
>want in one statement?

>Thanks,
>Steve

Re:Functions that return TStringLists


This is worth a try:

MyListBox.Items^ := @ParseString(MyString);

Guenther S{*word*203}<Guenther.So...@infonova.at> wrote in article
<6bei6b$122...@garion.telecom.at>...

Quote
> Hi Stephen!
> > MyListBox.Items := ParseString(MyString);

> >But this doesn't:
> > TmpStringList := ParseString(MyString);
> > MyListBox.Items := TmpStringList;
> > TmpStringList.Free;

> try:
> MyListBox.Items.Assign(ParseString(MyString));

Re:Functions that return TStringLists


That was rubbish, sorry.
I forgot you will need to get rid of any TStrings already attached
to your ListBox. Maybe:

if MyListBox.Items<>nil then MyListBox.Items.Free;
MyListBox.Items^ := @ParseString(MyString);

Andrew Hill <a.h...@cairnweb.com> wrote in article
<01bd3335$c47495a0$47495...@bn38.businessdial.net>...

Quote
> This is worth a try:

> MyListBox.Items^ := @ParseString(MyString);

> Guenther S{*word*203}<Guenther.So...@infonova.at> wrote in article
> <6bei6b$122...@garion.telecom.at>...
> > Hi Stephen!
> > > MyListBox.Items := ParseString(MyString);

> > >But this doesn't:
> > > TmpStringList := ParseString(MyString);
> > > MyListBox.Items := TmpStringList;
> > > TmpStringList.Free;

> > try:
> > MyListBox.Items.Assign(ParseString(MyString));

Re:Functions that return TStringLists


Correct me if I'm wrong but isn't a TStringList a special datatype that
holds a list of strings i.e. more than one string.  And isn't a TMemo a
simpler version of the same thing using TStringList to dereference it's
component lines?  Consequently, to keep things simple, why not pack a TMemo
component with the TSringList then pass the memo reference/pointer to the
procedure that can then dereference the constituent TStringList for
processing?  I'm sure there's probably a more elegant way to do it but I
believe in keeping it simple in passing.

Kinda like boxing up your toys and sending them somewhere with a return
address?
--

=========================================
bgrav...@vha.com
codi...@airmail.net
=========================================

Rick Peterson <rickpet@"airmail.net"> wrote in article
<EDD342BB6E2F85A0.3E1D4D7D75E5BE06.AC6F7A3FD68A8...@library-proxy.airnews.ne
t>...

Quote
> What you could do is move it to a procedure...and one of the Parameters
> could be a StringList...and just pass the correct StringList...that way
you
> don't have to create a stringlist in your function or procedure...which
is a
> no no...

> ParseString(MyString:string, MyListBox.Items);

> Rick Peterson

> Stephen Smith wrote in message <34dabab8.19698...@news.mindspring.com>...
> >I have a function that parses a delimited string and returns a
> >TStringList. The Result is created in the function.  I just realized
> >that this function is leaking depending on how I used it.  Eg:

> >This leaks:
> > MyListBox.Items := ParseString(MyString);

> >But this doesn't:
> > TmpStringList := ParseString(MyString);
> > MyListBox.Items := TmpStringList;
> > TmpStringList.Free;

> >I think this means that MyListBox.Items := ParseString(MyString); is
> >actually copying the my function's TStringList result into it's
> >another TStringList rather than just pointing to the one created in my
> >function?  Using a temp TStringList seems cludgy.  Can I do what I
> >want in one statement?

> >Thanks,
> >Steve

Re:Functions that return TStringLists


You could pass a TStrings...so that you could pass all types of List...ie
Memo, ComboBox
kinda like the way the VCL does it...

Rick Peterson...

Quote
VHA PC Development wrote in message

<01bd333f$065609a0$a6790...@GLayfiel.Corp.vha.com>...
Quote
>Correct me if I'm wrong but isn't a TStringList a special datatype that
>holds a list of strings i.e. more than one string.  And isn't a TMemo a
>simpler version of the same thing using TStringList to dereference it's
>component lines?  Consequently, to keep things simple, why not pack a TMemo
>component with the TSringList then pass the memo reference/pointer to the
>procedure that can then dereference the constituent TStringList for
>processing?  I'm sure there's probably a more elegant way to do it but I
>believe in keeping it simple in passing.

>Kinda like boxing up your toys and sending them somewhere with a return
>address?
>--

>=========================================
>bgrav...@vha.com
>codi...@airmail.net
>=========================================

>Rick Peterson <rickpet@"airmail.net"> wrote in article
><EDD342BB6E2F85A0.3E1D4D7D75E5BE06.AC6F7A3FD68A8...@library-proxy.airnews.n
e
>t>...
>> What you could do is move it to a procedure...and one of the Parameters
>> could be a StringList...and just pass the correct StringList...that way
>you
>> don't have to create a stringlist in your function or procedure...which
>is a
>> no no...

>> ParseString(MyString:string, MyListBox.Items);

>> Rick Peterson

>> Stephen Smith wrote in message <34dabab8.19698...@news.mindspring.com>...
>> >I have a function that parses a delimited string and returns a
>> >TStringList. The Result is created in the function.  I just realized
>> >that this function is leaking depending on how I used it.  Eg:

>> >This leaks:
>> > MyListBox.Items := ParseString(MyString);

>> >But this doesn't:
>> > TmpStringList := ParseString(MyString);
>> > MyListBox.Items := TmpStringList;
>> > TmpStringList.Free;

>> >I think this means that MyListBox.Items := ParseString(MyString); is
>> >actually copying the my function's TStringList result into it's
>> >another TStringList rather than just pointing to the one created in my
>> >function?  Using a temp TStringList seems cludgy.  Can I do what I
>> >want in one statement?

>> >Thanks,
>> >Steve

Re:Functions that return TStringLists


Quote
Stephen Smith wrote:

> I have a function that parses a delimited string and returns a
> TStringList. The Result is created in the function.  I just realized
> that this function is leaking depending on how I used it.  Eg:

> This leaks:
>         MyListBox.Items := ParseString(MyString);

> But this doesn't:
>         TmpStringList := ParseString(MyString);
>         MyListBox.Items := TmpStringList;
>         TmpStringList.Free;

> I think this means that MyListBox.Items := ParseString(MyString); is
> actually copying the my function's TStringList result into it's
> another TStringList rather than just pointing to the one created in my
> function?  Using a temp TStringList seems cludgy.  Can I do what I
> want in one statement?

> Thanks,
> Steve

The code below shows my approach to the problem. TForm1.ListBox1DblClick
shows how I handle the parse result (I use it to test my 'parse_rules').
Function parse creates an instance of TStringLIst.

I think I understand the problems associated with the creation and
deletion of objects, but apparently I don't. Based on my interpretation
of the responses to the original question, my approach is not considered
a 'good' one.

I thought that since parse is a (private) field of Form1 objects created
by parse would belong to Form1 and could properly be freed anyplace
within Form1.

Using the 'procedure' solution would give me, assuming plst was inserted
during the loading of ListBox1 (I have not tried this, so not sure if
correct):

  ListBox1.items.objects[i].create;
  parse (edit1.text, rule, ListBox1.items.objects[i]);

where parse is now a procedure.

Would someone please enlighten me on the pitfalls of my approach and the
benefits of the procedure solution.

Question for Steve: How did you discover that your original approach was
leaking?

thanks,
bob mackey

=============================

code fragment:

function parse (const line, parse_rule : string) : TStringList;
{parse line according to parse_rule and return the results in a
TStringList}

procedure TForm1.ListBox1DblClick(Sender: TObject);
var i, j : integer;
    plst : TStringList;
begin
  i := ListBox1.itemindex; {the line to be parsed}
  edit1.text := ListBox1.items[i]; {save the line to be parsed}
  ListBox1.items.delete(i);
  { this works, does not need plst
  ListBox1.items.insertobject (i, edit1.text, parse (edit1.text, rule));

Quote
}

  { alternate approach
      inserting plst shud be done when initially loading the ListBox,
      which precludes using LoadFromFile
      if plst inserted during intialization then .delete(i) above not
needed}
  plst := nil;
  ListBox1.items.insertobject (i, edit1.text, plst);
  ListBox1.items.objects[i] := parse (edit1.text, rule);
   { now show the parse result}
  memo2.lines.clear;
  memo2.lines.add ('the parsed line:');
  with ListBox1.items.objects[i] as TStringList do
    for j := 0 to count-1 do memo2.lines.add (strings[j]);
end;

=============================

Re:Functions that return TStringLists


On 6 Feb 1998 07:36:02 -0700, cn...@primenet.com (Clayton Neff) wrote:

Quote
>I agree with what Andy and Martin said.  Here, we never have a function return a
>new TStringList.  If it returns one, then you don't need to free it.  I would
>implement your ParseString method as a procedure with the string to parse and
>the TStringList to parse it into as parameters.  Then the method calling
>ParseString create the TStringList and can free it when it is done.

That is the best solution, but make sure the string list parameter has
type TStrings, not TStringList. That lets you pass any string list,
including a memo's Lines property, as the argument.
--
Ray Lischner (http://www.tempest-sw.com/)
Author of "Hidden Paths of Delphi 3: Experts, Wizards, and the Open Tools API"

Re:Functions that return TStringLists


Steve

I would agree with some of what Clayton said.  I also recommend that called
routines take as a parameter a TStringList rather than creating a new one
on the stack for code maintenance purposes.  In either case the calling
block of code is still responsible for freeing the memory and I do allow it
when it makes sense.

However regarding the your real question, filling a Tlistbox.items member.
First this is a TSrings object not a TStringList object, and second it is a
constant value. This is why I presume clayton recommends passing in the
TListBox itself, creating a TStringList, populating it, assigning it to
ListBox.Items, and freeing it. But there is one extra step here than is
necessay.  I have had developers try to do the the folowing:
create a procedure like
        procedure populateList( var list ; TstringList);
and then try to call it with someting like
        poulateList( ListBox1.Items);
Well, this fails to complile because, again, ListBox1.Items is a constant
value. But that is OK, think of it as a pointer to a pointer, or a typed
constant, you can't change the thing itself but you can work on what it
points to.  This means that all that was necessay was to change the
procedure decleration to:
        procedure populateList( const list ; Tstrings);

Finally, I also agree with Clayton that the best approach would be to pass
both the string to pase and the TStrings object into your procedure.  I
have ATTACHED a small example program that does just this, I assume that
the string to be parsed is made up of substrings seperated by ';'.  The
actual populate function in it is:

function TForm1.PopulateList( const FNames : String; const list :Tstrings):
Integer;
var
   tmpNames : String;
   tmpStr   : String;
   count    : integer;
begin
     tmpNames := FNames;
     count := 0;
     repeat
           tmpStr := nextFName( tmpNames );
           if tmpStr <> '' then
              begin
              list.Add( tmpStr );
              inc(count);
              end;
     until tmpStr = '';
     result := count;
end;

Hope this helps;

Andy
        atjen...@bellatlantic.net

Clayton Neff <cn...@primenet.com> wrote in article
<34dc1e90.3579...@news.primenet.com>...

Quote
> Steve -

> I agree with what Andy and Martin said.  Here, we never have a function
return a
> new TStringList.  If it returns one, then you don't need to free it.  I
would
> implement your ParseString method as a procedure with the string to parse
and
> the TStringList to parse it into as parameters.  Then the method calling
> ParseString create the TStringList and can free it when it is done.
Optionally,
> you could pass in the string and the list box you want the items put in
to.
> Then ParseString could create the TStringList, fill it, assign it to the
list
> box, and finally free it.

> --
> Clayton Neff
> Software Project Leader
> The Personal Marketing Company, Inc.
> 11843 West 83rd Terrace
> Lenexa, KS  66214
> (913)492-0322
> clay...@tpmco.com

  Unit1.pas
1K Download

  Unit1.dfm
< 1K Download

  lists.dpr
< 1K Download
Go to page: [1] [2]

Other Threads