Просьба проверить, если в коде утечка памяти

[Utkin]

Сообщение от Serge_Bliznykov

чрезвычайно чревато.

, возьму на вооружение .

впрочем, не удивлюсь, если сработает и такой механизм:

  var TS : TString;
...
   TS := функция();
пользуемся TS
после использования:
   TS.Free;

Да, сработает для простого примера, как раз чтобы проверить сработает или нет. Для серьезного проекта с интенсивным выделением и высвобождением памяти, образованием новых экземпляров и т.д. этот код иногда будет вываливаться в Access Violation.
Потому что, память занятая под объекты, после выхода из функции будет помечена как свободная область и есть вероятность, что до использования данного объекта она будет занята кем-то еще. Проблема трудноотлавливаемая плюс в данном случае была рекурсия. Более того, TS Вы разрушите, а кто разрушит Result?
Однозначно лучше процедура с изменяемым входящим параметром. Объект создан до процедуры и соответственно никаких заморочек с его разрушением.

[Anfall]

Всем спасибо за ответы.

DomiNick,
Не могли бы объяснить как пользоваться вашей процедурой?
Вот мне надо добавить список найденных файлов в листбокс. При использовании функции я делал так:
ListBox1.Items.AddStrings(GetFiles( 'C:\example'));

[GunSmoker]

Сообщение от Utkin

Да, сработает для простого примера, как раз чтобы проверить сработает или нет. Для серьезного проекта с интенсивным выделением и высвобождением памяти, образованием новых экземпляров и т.д. этот код иногда будет вываливаться в Access Violation.

Рекомендую перестать нести бред. Вы путаете стек и кучу.

Сообщение от Utkin

Да, только

Procedure GetFiles_New(Path: String; var Items: TStrings);

var не нужен.

Сообщение от Anfall

Кто-то сказал, есть утечка памяти в ней.

Да, есть. Но не там, куда вас тыкали две страницы.

Утечка не в этой функции, а в этом коде:

ListBox1.Items.AddStrings(GetFiles( 'C:\example'));

Надо:

SL := GetFiles( 'C:\example');
try
  ListBox1.Items.AddStrings(SL);
finally
  FreeAndNil(SL);
end;

А в функции у вас утечка в забытом FindClose.

И в догонку.

[mutabor]

Result.Free;

Конечно, так делать нельзя. Этим мы убиваем объект, когда функция еще не успела его даже вернуть.

TS Вы разрушите, а кто разрушит Result?

Это в данном случае одно и то же, тот же объект, точнее сказать это ссылки на один и тот же объект.

var TS : TString;
...
TS := функция();
пользуемся TS
после использования:
TS.Free;

Абсолютно нормальный и правильный код, никаких ошибок тут нет.

[GunSmoker]

Сообщение от GunSmoker

Надо:

Добавлю: по этой причине надо всё же сказать, что это не самый типичный подход. Да, он 100% корректен. Проблема в том, что вы разделяете ответственность за жизнь объекта между вызывающим и вызываемым. Это не самая типичная и прозрачная логика.

Обычно, есть хорошее правило (хорошего тона): любую вещь, возвращаемую функцией, можно использовать непосредственно. Иными словами, функция не должна возвращать нечто, требующее дополнительной обработки - в том числе освобождения.

Соблюдение этого простого правила позволяет вам писать цепочки вложенных вызовов вот так:

Func1(Func2(Func4), Func3);

Если же функции нужно передать подобную вещь - лучше сделать её процедурой с выходным параметром.

Но в большинстве случаем лучше сработает редизайн API, например:

procedure GetFiles(const AFiles: TStrings; Path: String; Full: Boolean = False);
var
  Rec:TSearchRec;
  TMP:TStrings;
  ls,ex: String;
  i: integer;
 ok: integer;
begin
  if Path[Length(Path)]<>'\' Then Path:=Path+'\';
  ok := FindFirst(Path+'\*.*',faAnyFile,Rec);
  ... // используем AFiles; обратите внимание, что TStringList.Create нет
end;

begin
  GetFiles(ListBox1.Items, 'C:\example');
end;

И ещё на почитать.

[mutabor]

Соблюдение этого простого правила позволяет вам писать цепочки вложенных вызовов вот так: Func1(Func2(Func4), Func3);

В Дельфи да, с его необходимостью явно освобождать память, чтобы такое замутить, придется объекты в параметрах передавать. Но если нет необходимости делать вложенные вызовы, то нет ничего плохого если функция будет возвращать объект, часто это удобно.

[DomiNick]

До меня кстати дошло... "Items: TStrings" - это ж по сути указатель будет...

Сообщение от Anfall

DomiNick,
Не могли бы объяснить как пользоваться вашей процедурой?
Вот мне надо добавить список найденных файлов в листбокс. При использовании функции я делал так:
ListBox1.Items.AddStrings(GetFiles( 'C:\example'));

Вот так:

procedure TForm1.Button1Click(Sender: TObject);
begin
// ListBox1.Visible:=False;
ListBox1.Items.Clear; // очистка (если нужна)
GetFiles_New('C:\example', ListBox1.Items);
// ListBox1.Visible:=True;
end;

Но в большинстве случаем лучше сработает редизайн API, например:
...
GetFiles(ListBox1.Items, 'C:\example');

Ну так у меня как раз примерно так и есть...))

Про FindClose я потом уже таки-вспомнил, вот только не могу найти на каждый ли вызов "FindFirst" нужно "FindClose"... Хотя всё равно придётся...

Anfall, перед последним "end;" в функции добавьте "FindClose(Rec);"...

[Пепел Феникса]

это по сути и есть указатель.
в Делфи объекты указательные всегда(всегда указатель на объект, а объект в куче)

[mutabor]

До меня кстати дошло... "Items: TStrings" - это ж по сути указатель будет...

Да что вы говорите

Иногда еще объясняют на таком примере: представьте что у вас на ниточках подвешена коробка, коробка - это объект, ниточки - указатели. Ниточек может быть много, а коробка одна, и вот по одной начинаете перерезать их. Объект будет висеть пока будет хоть одна ниточка, а вот когда обрежется последняя, то тут зависит от языка, в Java например сборщик мусора объект подберет, а в Дельфи перед обрезанием последней ниточки нужно сперва память освободить из под объекта.

[Anfall]

GunSmoker,
Спасибо. Попробую код:

procedure GetFiles(const AFiles: TStrings; Path: String; Full: Boolean = False);
var
  Rec:TSearchRec;
  TMP:TStrings;
  ls,ex: String;
  i: integer;
 ok: integer;
begin
  if Path[Length(Path)]<>'\' Then Path:=Path+'\';
  ok := FindFirst(Path+'\*.*',faAnyFile,Rec);
  ... // используем AFiles; обратите внимание, что TStringList.Create нет
end;

begin
  GetFiles(ListBox1.Items, 'C:\example');
end;

DomiNick, вот так верно?

Procedure GetFiles_New(Path: String; Items: TStrings);
Var Rec: TSearchRec; ex: String; ok: Integer;
Begin
If (Path[Length(Path)]<>'\') Then
      Path:=Path+'\';
ok:=FindFirst(Path+'\*.*', faAnyFile,Rec);
While (ok=0) Do
      Begin
      If (Rec.Name<>'.') And (Rec.Name<>'..') Then
            If ((Rec.Attr And faDirectory)<>0) Then
                  Begin
                  GetFiles_New(Path+Rec.Name, Items);
                  End
            Else
                  Begin
                  ex:=ExtractFileExt(Rec.Name);
                  If (ex='.txt') Then
                        Items.Add(Path+Rec.Name);
                  End;
      ok:=FindNext(Rec);
      FindClose(Rec);
      End;