Рекурсивного Destructor

Question

У меня есть следующие два класс:

type 
    TItemProp = class 
    private 
    FItemPropName: string; 
    FItemPropValue: string; 
    public 
    constructor Create(AItemPropName, AItemPropValue: string); 
    class function GetItemProp(AHTMLElement: OleVariant; AProp: string): TItemProp; 
    property ItemPropName: string read FItemPropName; 
    property ItemPropValue: string read FItemPropValue; 
end; 

TTest = class 
private 
    FName: string; 
    FProps: TList<TItemProp>; 
    FTest: TList<TTest>; 
public 
    constructor Create(AName: string); 
    destructor Destoroy(); 
    property Name: string read FName; 
    property ItemProps: TList<TItemProp> read FProps write FProps; 
    property Test: TList<TTest> read FTest write FTest; 
end; 

А вот код конструктора и деструктор класса Ttest:

constructor TTest.Create(AName: string); 
begin 
    Self.FName := AName; 
    Self.FProps := TList<TItemProp>.Create(); 
    Self.FTest := TList<TTest>.Create(); 
end; 

destructor TTest.Destoroy(); 
var 
    I: Integer; 
begin 
    for I := 0 to Self.FTest.Count - 1 do 
    begin 
    Self.FTest[I].Free; 
    Self.FTest[I] := nil; 
    end; 

    Self.FProps.Free; 
    Self.FTest.TrimExcess; 
    Self.FTest.Free; 
    inherited; 
end; 

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

Answer 1

Первая проблема здесь:

destructor Destroy(); 

Вы должны переопределить виртуальный деструктор, объявленный в TObject. Как это:

destructor Destroy; override; 

Ваша реализация деструктора неоправданно сложна, а также не уничтожать объекты, принадлежащие FProps. Этот деструктор должен быть написано так:

destructor TTest.Destroy; 
var 
    I: Integer; 
begin 
    for I := 0 to FTest.Count - 1 do 
    FTest[I].Free; 
    FTest.Free; 

    for I := 0 to FProps.Count - 1 do 
    FProps[I].Free; 
    FProps.Free; 

    inherited; 
end; 

Ваших свойства, которые предоставляют эти объекты списка не должен иметь методы сеттера. Таким образом, они должны быть только для чтения свойства, как это:

property ItemProps: TList<TItemProp> read FProps; 
property Test: TList<TTest> read FTest; 

Ваш код может привести к утечкам должна любая попытка тела, чтобы написать эти свойства.

Вы должны были использовать TObjectList, а не TList, вы могли бы делегировать управление жизненным циклом классу списка. Это приводит к значительному упрощению кода.

Публикация TList<T> экземпляров, поскольку публичные свойства скорее подвергли ваш класс злоупотреблению. Клиенты этого класса могут вызывать любые общедоступные методы этого класса и потенциально изменять список таким образом, которого вы не ожидаете, и не хотите обслуживать. Вам следует больше инкапсулировать эти объекты.

Как правило, вы всегда должны называть унаследованный конструктор класса. В вашем случае этот конструктор - это конструктор TObject, который ничего не делает. Но, это хорошая практика, чтобы назвать это в любом случае. Это означает, что если вы измените иерархию наследования позднее, вы не будете пойманы, не вызвав конструктор нового родительского класса.

constructor TTest.Create(AName: string); 
begin 
    inherited; 
    FName := AName; 
    FProps := TList<TItemProp>.Create(); 
    FTest := TList<TTest>.Create(); 
end; 

Вы используете Self всюду. Это прекрасно, но это вовсе не идиоматично. Я предлагаю вам отказаться от этой привычки, иначе ваш код будет очень многословным и менее легким для чтения.

В коде, который вы отправили, было очень много ошибок. В вашей программе явно больше кода, который вы не разместили. Я бы очень ожидал, что код тоже будет содержать ошибки, поэтому не удивляйтесь, если утечки остаются даже после применения всех изменений выше.