Board index » delphi » Function that returns a TStringList

Function that returns a TStringList

Hi,

How do you go about writing a function that returns a TStringList?  I'm
using the code below but I'm sure it's leaking memory.  I don't want a
procedure that populates a passed TStringList, I want a function.  This is
so I can use it in this context:

ShowMessage(Explode('|','Bob|Brown|New Zealand')[0]);

function Explode(delimiter:string; source:string) : TStringList;
(*
// Name : Explode
// Purpose : each of which is a substring of string formed by splitting it
on
// boundaries formed by the string delim. It does not support PHP's
// 'limit' feature.
// Date : 12 Feb 2001
// Comments : Based on PHP's Explode function (http://www.php.net/explode).
// Returns an array of strings, each of which is a substring of
// string formed by splitting it on boundaries formed by the string //
delimiter.
*)
var
  c : word;
begin
  Result:=TStringList.Create;
  c:=0;
  while source<>'' do
  begin
    if Pos(delimiter,source)>0 then
    begin
      Result.Add(Copy(Source,1,Pos(delimiter,source)-1));
      Delete(Source,1,Length(Result[c])+Length(delimiter));
    end
    else
    begin
      Result.Add(Source);
      Source:='';
    end;
    inc(c);
  end;
end;

 

Re:Function that returns a TStringList


On Thu, 9 Aug 2001 10:03:13 +1200, "Bob M Brown"

Quote
<bob.br...@opus.co.nz> wrote:
>Hi,

>How do you go about writing a function that returns a TStringList?  I'm
>using the code below but I'm sure it's leaking memory.  I don't want a
>procedure that populates a passed TStringList, I want a function.  This is
>so I can use it in this context:

If you instantiate and return an instance of a class from a function
like this, then the calling code becomes responsible for its
destruction since the function has no more association with it.

Calling the function as you show will return an anonymous instance
that is never freed, and will indeed leak memory.

I'd suggest either using an intermediate variable to contain the
instance:

  var
    ExpList : TStringList;
  begin
  ...
    ExpList :=  Explode('|','Bob|Brown|New Zealand');
    ShowMessage(ExpList[0]);
    ExpList.Free;

or changing the function's approach to return the particular string by
passing the index as a variable:

  function Explode(delimiter:string; source:string;
    index: Integer) : string;

called like so:

  ShowMessage(Explode('|','Bob|Brown|New Zealand', 0));

You could also do something with creating and destroying a global
stringlist that Explode just keeps re-using.

The gist of this is if you create the instance you must account for
destroying it in some fashion.

Stephen Posey
slpo...@concentric.net

Re:Function that returns a TStringList


Another (widely used) option is to make the caller completely responsible
for the list by adding the list to the function parameters, e.g.

(I also added a faster Explode procedure)

var
  List: TStrings;
begin
  List := TStringList.Create;
  try
    Explode('|', 'Bob|Brown|New Zealand', List);
    ShowMessage(List[0]);
  finally
    List.Free;
  end;
end;

procedure Explode(const Delimiter: Char; const Source: String; const List:
TStrings);
var
  Len, i, j: Integer;
begin
  Len := Length(Source);
  i := 1;
  while i <= Len do
  begin
    j := i;
    while (i <= Len) and (Source[i] <> Delimiter) do
      Inc(i);
    List.Add(Copy(Source, j, i - j);
    if i <= Len then
      Inc(i);
  end;
end;

Quote
"Stephen Posey" <slpo...@concentric.net> wrote in message

news:3b7327dc.14409365@news.concentric.net...
Quote
> On Thu, 9 Aug 2001 10:03:13 +1200, "Bob M Brown"
> <bob.br...@opus.co.nz> wrote:

> >Hi,

> >How do you go about writing a function that returns a TStringList?  I'm
> >using the code below but I'm sure it's leaking memory.  I don't want a
> >procedure that populates a passed TStringList, I want a function.  This
is
> >so I can use it in this context:

> If you instantiate and return an instance of a class from a function
> like this, then the calling code becomes responsible for its
> destruction since the function has no more association with it.

> Calling the function as you show will return an anonymous instance
> that is never freed, and will indeed leak memory.

> I'd suggest either using an intermediate variable to contain the
> instance:

>   var
>     ExpList : TStringList;
>   begin
>   ...
>     ExpList :=  Explode('|','Bob|Brown|New Zealand');
>     ShowMessage(ExpList[0]);
>     ExpList.Free;

> or changing the function's approach to return the particular string by
> passing the index as a variable:

>   function Explode(delimiter:string; source:string;
>     index: Integer) : string;

> called like so:

>   ShowMessage(Explode('|','Bob|Brown|New Zealand', 0));

> You could also do something with creating and destroying a global
> stringlist that Explode just keeps re-using.

> The gist of this is if you create the instance you must account for
> destroying it in some fashion.

> Stephen Posey
> slpo...@concentric.net

Re:Function that returns a TStringList


All before said is absolutely true.
I only want to add an alternative way:

with Explode(...) do begin
    if Count = 0 then ....
    ....
    Free();
end;

But caution! If you get an exception in your with..end block, the Free() may
never reached!

Thorsten Claus

Re:Function that returns a TStringList


"Bob M Brown" <bob.br...@opus.co.nz> skrev i melding
news:9kscv5$la8$1@com.opus.co.nz...

Quote
> Hi,

> How do you go about writing a function that returns a TStringList?  I'm
> using the code below but I'm sure it's leaking memory.  I don't want a
> procedure that populates a passed TStringList, I want a function.  This is
> so I can use it in this context:

Warning: Asking for a function that creates an object should rather be posted
here:

comp.lang.pascal.delphi.functioncreatingclassthatmustbedestroyedbycaller.misc

...here it's very likely to cause a loooong thread.

--
Bjoerge Saether
Consultant / Developer
http://www.itte.no
Asker, Norway
bjorge@takethisaway_itte.no (remve the obvious)

Re:Function that returns a TStringList


That is because you should use:

List := Explode(...);
try
  if List.Count > 0 then
    ...
finally
  List.Free;
end;

Quote
"Thorsten Claus" <Thorsten.Cl...@cosynus.de> wrote in message

news:9ktics$6i1fa$1@ID-54951.news.dfncis.de...
Quote
> All before said is absolutely true.
> I only want to add an alternative way:

> with Explode(...) do begin
>     if Count = 0 then ....
>     ....
>     Free();
> end;

> But caution! If you get an exception in your with..end block, the Free()
may
> never reached!

> Thorsten Claus

Re:Function that returns a TStringList


On Thu, 9 Aug 2001 10:03:13 +1200, "Bob M Brown"

Quote
<bob.br...@opus.co.nz> wrote:
>Hi,

>How do you go about writing a function that returns a TStringList?  I'm
>using the code below but I'm sure it's leaking memory.  I don't want a
>procedure that populates a passed TStringList, I want a function.  This is
>so I can use it in this context:

>ShowMessage(Explode('|','Bob|Brown|New Zealand')[0]);

Well in many people's opinion this is the wrong thing to want.
I don't see _any_ way to correctly make a function Explode
that creates and returns a stringlist, allows you to call
the function _exactly_ as above, and still doesn't leak
memory. Because when you say Showmessage(Explode(etc)[0])
you have not assigned the result of the call to Explode
to a variable, so there's simply no way to access the
list that you need to free.

The thing to do is to write it the way you say you
don't want to write it:

procedure Explode(data: string; Target: TStrings);
//parses data and adds strings to Target.

var s: TStringlist;
begin
  s:= TStringlist.Create;
  try
    Explode(whatever, s);
    if Length(s) > 0 then
       showmessage(s[0]);
  finally
    s.Free;
  end;
end;

If you have a good reason for not wanting to do it that
way you should probably explain what the reason is; you're
not going to have a lot of luck getting people to show
you how to do it "wrong" otherwise (especially since
in this case I actually don't see _how_ to do _exactly_
what you say you want. The closest I see to what you want
is this:

var s: TStriglist;
begin
  s:=Explode(whatever);
  try
    showmessage(s[0]);
  finally
    s.Free;
  end;
end;

As long as you need an actual variable to do something
remotely like what you want correctly I don't see why
you could object to the "right" way...)

Comments on Explode:

Quote
>function Explode(delimiter:string; source:string) : TStringList;
>(*
>// Name : Explode
>// Purpose : each of which is a substring of string formed by splitting it
>on
>// boundaries formed by the string delim. It does not support PHP's
>// 'limit' feature.
>// Date : 12 Feb 2001
>// Comments : Based on PHP's Explode function (http://www.php.net/explode).
>// Returns an array of strings, each of which is a substring of
>// string formed by splitting it on boundaries formed by the string //
>delimiter.
>*)
>var
>  c : word;

It's very hard for me to imagine a good reason c should be a word
instead of an integer.

Quote
>begin
>  Result:=TStringList.Create;
>  c:=0;
>  while source<>'' do
>  begin
>    if Pos(delimiter,source)>0 then
>    begin
>      Result.Add(Copy(Source,1,Pos(delimiter,source)-1));
>      Delete(Source,1,Length(Result[c])+Length(delimiter));

Using Delete here gives you something that is going to take
time n^2 (n squared) for source of length n. You can easily
rewrite the same algorithm to take time n. (may not make
any difference today - some day when you dig out your old
code and apply it to a long string you'll wonder why it
doesn't work.)

It's too bad that Pos does not include a StartAt parameter,
so you could use it to search for something and then search
for the next one without having to modify the string. You
can use PChar functions this way (StrPos is the one I
think I'm thinking of). Or you can simply do it yourself
without using Pos at all, simply looping throught the string
looking for delimiter. Something like

TokenStart:= 1;
for c:= 1 to length(source) do
  begin
    if source[c] = delimiter then
           //or "if source[c] in delimiters" for a more
           //general version you can't do with Pos
     begin
       target.add(Copy(source, TokenStart, c - TokenStart - 1);
         //off the top of my head, almost surely not quite right
       TokenStart:= c + 1;
     end;
  end;
Target.Add(anything left over);

That's probably not exactly right. But the point is that
writing a loop like that that simply scans through a string
once, keeps track of what it's seen and does the right thing
when it sees various things is really no harder than
writing a loop using things like Pos and Delete, and it
will often give _much_ better performance.

If, say, you're parsing filenames looking for '/' then
it doesn't matter. If you're parsing data files it can
make a _big_ difference. (Once years ago I actually
wondered why a certain routine would hang on a large
file. Relized it wasn't{*word*154}, it was just an n^2
routine and it was going to take a few weeks to terminate!)

Quote
>    end
>    else
>    begin
>      Result.Add(Source);
>      Source:='';
>    end;
>    inc(c);
>  end;
>end;

David C. Ullrich

Re:Function that returns a TStringList


Hence the name of the function, Explode(),  newsgroup, newsgroup authors,
and most likely the function itself.  :-)

Quote
Bj?rge S?ther <REMOVE_bsaether@THIS_online.no> wrote in message

news:9ssc7.5346$e%4.165425@news3.oke.nextra.no...
Quote
> "Bob M Brown" <bob.br...@opus.co.nz> skrev i melding
> news:9kscv5$la8$1@com.opus.co.nz...
> > Hi,

> > How do you go about writing a function that returns a TStringList?  I'm
> > using the code below but I'm sure it's leaking memory.  I don't want a
> > procedure that populates a passed TStringList, I want a function.  This
is
> > so I can use it in this context:

> Warning: Asking for a function that creates an object should rather be
posted
> here:

comp.lang.pascal.delphi.functioncreatingclassthatmustbedestroyedbycaller.mis
c
Quote

> ...here it's very likely to cause a loooong thread.

> --
> Bjoerge Saether
> Consultant / Developer
> http://www.itte.no
> Asker, Norway
> bjorge@takethisaway_itte.no (remve the obvious)

Other Threads