Räumt diese Komponente hinter sich auf?

[hzzm]

Guten Morgen,

räumt diese Komponente hinter sich auf (sind die Destruktoren korrekt)?

Delphi-Quellcode:

unit ImageMultiStates;

    interface

    uses
      Vcl.Graphics, Vcl.StdCtrls, System.SysUtils, System.Classes, Vcl.Controls, Vcl.ExtCtrls, Forms, Generics.Collections;

    type

      TPic = class(TCollectionItem)
      private
        FPicture: TPicture;
        procedure SetPicture(const value: TPicture);
      protected
        function GetDisplayName: String; override;
      public
        procedure Assign(Source: TPersistent); override;
        constructor Create(Collection: TCollection); override;
      published
        property Picture: TPicture read FPicture write SetPicture;
      end;

      TPictures = class(TOwnedCollection)
      private
        function GetPic(Index: Integer): TPic;
        procedure SetPic(Index: Integer; Value: TPic);
      public
        constructor Create(AOwner: TComponent);
        destructor Destroy;
        property Picture[Index: Integer]: TPic read GetPic write SetPic;
      end;

      TImageMultiStates = class(TImage)
      private
        FPictures: TPictures;
        FOnMouseDownPic: Integer;
        FOnMouseUpPic: Integer;
        procedure SetPictures(const Value: TPictures);
      public
        ActivePic: Integer;
        constructor Create(AOwner: TComponent); override;
        destructor Destroy; override;
        procedure ActivatePic(Index: Integer);
        procedure MouseDown(Button: TMouseButton; Shift: TShiftState; X, Y: Integer); override;
        procedure MouseUp(Button: TMouseButton; Shift: TShiftState; X, Y: Integer); override;
      published
        property Pictures: TPictures read FPictures write SetPictures;
        property OnMouseDownPic: Integer read FOnMouseDownPic write FOnMouseDownPic;
        property OnMouseUpPic: Integer read FOnMouseUpPic write FOnMouseUpPic;
      end;

    procedure Register;

    implementation

    constructor TPic.Create(Collection: TCollection);
    begin
      inherited;
      FPicture := TPicture.Create;
    end;

    procedure TPic.Assign(Source: TPersistent);
    begin
      if Source is TPic then
        FPicture := TPic(Source).FPicture
      else
        inherited;
    end;

    procedure TPic.SetPicture(const Value: TPicture);
    begin
      FPicture.Assign(Value);
      if Index=0 then
        TImageMultiStates(TPictures(GetOwner).GetOwner).Picture.Assign(Value);
    end;

    function TPic.GetDisplayName;
    begin
      Result := Format('Picture %d',[Index]);
    end;


    constructor TPictures.Create(AOwner: TComponent);
    begin
      inherited Create(AOwner, TPic);
    end;

    destructor TPictures.Destroy;
    begin
      inherited Destroy;
    end;

    procedure TPictures.SetPic(Index: Integer; Value: TPic);
    begin
      inherited SetItem(Index, Value);
    end;

    function TPictures.GetPic(Index: Integer): TPic;
    begin
      Result := inherited GetItem(Index) as TPic;
    end;


    constructor TImageMultiStates.Create(AOwner: TComponent);
    begin
      inherited;
      FOnMouseDownPic := -1;
      FOnMouseUpPic := -1;
      FPictures := TPictures.Create(AOwner);
    end;

    destructor TImageMultiStates.Destroy;
    begin
      FPictures.Free;
      inherited;
    end;

    procedure TImageMultiStates.SetPictures(const Value: TPictures);
    begin
      FPictures.Assign(Value);
    end;

    procedure TImageMultiStates.ActivatePic(Index: Integer);
    begin
      if (Index < FPictures.Count) and (Index >= 0) then
      begin
        ActivePic := Index;
        Picture.Assign(TPic(FPictures.Items[Index]).FPicture);
      end
      else
      begin
        ActivePic := -1;
        Picture.Assign(nil);
      end;
    end;

    procedure TImageMultiStates.MouseDown(Button: TMouseButton; Shift: TShiftState; X, Y: Integer);
    begin
      if FOnMouseDownPic<>-1 then
        Picture.Assign(TPic(FPictures.Items[FOnMouseDownPic]).FPicture);
      inherited;
    end;

    procedure TImageMultiStates.MouseUp(Button: TMouseButton; Shift: TShiftState; X, Y: Integer);
    begin
      if FOnMouseUpPic<>-1 then
        Picture.Assign(TPic(FPictures.Items[FOnMouseUpPic]).FPicture);
      inherited;
    end;

    procedure Register;
    begin
      RegisterComponents('Standard', [TImageMultiStates]);
    end;

    end.

[Daniel]

Ein wichtiges Werkzeug in solchen Fragen: Delphi kann Dir das selbst sagen.
Wenn Du im Projektquellcode die Anweisung „ReportMemoryLeaksOnShutdown:=true;“ setzt, bekommst Du einen Hinweis, wenn am Ende etwas nicht artig aufgeräumt wird.
Die dahinterstehende Buchhaltung kostet Rechenzeit, o.g. Anweisung sollte also nur zum Testen in Deinem Projekt stehen.

[Der schöne Günther]

Und dann spiele doch mal etwas mit deiner eigenen Komponente wenn

Delphi-Quellcode:

ReportMemoryLeaksOnShutdown = true

ist. Oder noch besser: Schreibe vernünftige Unit-Tests. Zum Beispiel für

Delphi-Quellcode:

TPic.Assign(..)

. Da wirst du zum Beispiel feststellen dass das was du im Konstruktor in FPicture reingesteckt hast niemals freigegeben wird.


PS: "Richtig aufräumen" beinhaltet mehr als nur Speicher der Delphi-Objekte freigeben. Wenn deine Klasse beispielsweise mit Dateien hantiert und ein Handle nicht mehr freigibt, eine Netzwerkverbindung nicht schließt, … All sowas.

[jaenicke]

So wie du es aktuell hast, ist die korrekte Speicherverwaltung sehr schwer. Denn im Assign kopierst du z.B. nur den Pointer auf ein fremdes Objekt. Dadurch darf dieses Objekt außen gar nicht freigegeben werden, aber auch das Objekt, das den Pointer bekommen hat, kann es nicht tun, da es ja nicht weiß was außen mit dem Objekt passiert.

Delphi selbst macht das so (in TPicture z.B.), dass schlicht im Setter und in Assign der Inhalt kopiert wird. Dadurch sind die einzelnen Objekte voneinander unabhängig und können im Destruktor alles problemlos freigeben, das sie selbst erzeugt haben.

[Pfaffe]

In meinen Projektdateien steht immer:
{$IFDEF DEBUG}
ReportMemoryLeaksOnShutdown:= True;
{$ENDIF}

[MicMic]

Wie ist es denn eigentlich zu bewerten bzw. was passiert, wenn hier und da ein Speicher nicht freigegeben wird? Es wird dann ja wohl meistens um kleine Sachen gehen.
Ein Betriebssystem wird doch damit bestimmt auch umgehen können. Ich könnte mir vorstellen, dann ist halt irgendwo noch was im Speicher und Windows ist's schnuppe. Schaut natürlich bestimmt wieder anders aus, wenn der Rechner 24h an ist.

Michael

[haentschman]

Moin...:P
Wenn der Prozess (Anwendung) beendet wird, dann räumt Windows den gesamten Speicher des Prozesses auf. Aber es soll auch Anwendungen geben die 365/24 laufen...:zwinker: Für die wäre es fatal wenn irgend eine procedure nicht aufräumt sondern den Speicher zumüllt. 8-)

[KodeZwerg]

Windows stehen auch nur eine bestimmte Anzahl an Handles zur Verfügung, also kann es da irgendwann auch knapp werden.

[freimatz]

Zitat:

Zitat von haentschman (Beitrag 1409744)

Aber es soll auch Anwendungen geben die 365/24 laufen...:zwinker:

Kann gar nicht mehr sein. Irgendwann kommt Windows 10 und macht selbständig einen Neustart :twisted:

[Der schöne Günther]

Wenn du Windows für Heimversionen einsetzt, dann ja.
Es ist eher dass die Anlagen das können sollten, früher oder später fällt beim Kunden eh der Strom aus ;-)

[Blup]

Im TPic-construcor wird FPicture erzeugt. Deshalb sollte es auch einen TPic-destructor geben, der FPicture freigibt.

[jaenicke]

Zitat:

Zitat von Blup (Beitrag 1410107)

Im TPic-construcor wird FPicture erzeugt. Deshalb sollte es auch einen TPic-destructor geben, der FPicture freigibt.

Das geht nach der aktuellen Konstruktion aber nicht, weil der Pointer darauf einfach zwischen den Instanzen weitergereicht wird. Und dadurch gehen die Referenzen teilweise verloren und sich teilweise doppelt. Das ginge nur mit Interfaces so (oder mit ARC).

[hzzm]

Zitat:

Zitat von jaenicke (Beitrag 1409594)

So wie du es aktuell hast, ist die korrekte Speicherverwaltung sehr schwer. Denn im Assign kopierst du z.B. nur den Pointer auf ein fremdes Objekt. Dadurch darf dieses Objekt außen gar nicht freigegeben werden, aber auch das Objekt, das den Pointer bekommen hat, kann es nicht tun, da es ja nicht weiß was außen mit dem Objekt passiert.

Delphi selbst macht das so (in TPicture z.B.), dass schlicht im Setter und in Assign der Inhalt kopiert wird. Dadurch sind die einzelnen Objekte voneinander unabhängig und können im Destruktor alles problemlos freigeben, das sie selbst erzeugt haben.

Gold wert. Danke!