Problem mit Constructor/Destructor

[DelphiNerd84]

hi leute,

bin neu hier im forum und hab gleich ein problem. nach stundenlangen herumprobieren weiss ich nicht mehr weiter :( habe erst seit kurzem angefangen mit klassen zu arbeiten.. und scheitere bereits beim constructor bzw. dem pointer darauf.

Hier erstmal die Unit mit der Class:

Delphi-Quellcode:

UNIT UContainer;
INTERFACE
USES SysUtils;
TYPE
  TContainer=Class
    Private
      FFileName:String;
      Procedure DoSomething;
    Public
      Constructor Create(const FileName:String;AsReadOnly:Boolean);
      Destructor Destroy;override;
  End;
IMPLEMENTATION
Constructor TContainer.Create(constFName:String;AsReadOnly:Boolean);
Begin
 inherited Create;
 If FileExists(FName)Then Begin
  //HandleFile;
  //..
 End Else Destroy
End;
Destructor TContainer.Destroy;
Begin
 inherited Destroy
End;
END.

und dann..

Delphi-Quellcode:

var
  P:Pointer;
Begin
  P:=TContainer.Create('C:\Non-Existing-File.txt',False);
  If P=NIL Then 
   ShowMessage('P is NIL')
  Else 
   ShowMessage('P is NOT NIL');
  TContainer(P).DoSomething; //..erzeugt Exception!
End;

ich möchte also eine klasse erzeugen, die falls ein problem auftaucht (z.b. nicht existierende datei) sich gleich mittels destructor selbst zerstört. Durch aufruf des destructors sollte doch eigentlich auch der pointer darauf auf nil gesetzt werden? das passiert aber nicht! wie kann man sonst herausfinden ob die klasse noch existiert?

vielen dank!
gruss markus

[Zacherl]

Die von dir designte Klasse ist höchst "interessant". Ich würde es so lösen:

Delphi-Quellcode:

type  
  TContainer = class(TObject)
  private
    FFileName: String; 
    FInvalidFile: Boolean;
  public
    procedure DoSomeThing;
    constructor Create(const FileName: String; AsReadOnly: Boolean);
    destructor Destroy;
  published
    property InvalidFile: Boolean read FInvalidFile;
  end;

Dann kannst du im Create prüfen ob die Datei geöffnet werden kann und ansonsten setzt du FInvalidFile auf true. In keinen Fall darfst du im Constructor Destroy aufrufen. Das ist tödlich :D

[Sir Rufo]

Destroy sollte generell nicht aufgerufen werden, sondern Free.
Im Constructor allerdings auch kein Free ;)

Das Zerstören einer Instanz setzt die Referenzen nicht auf nil.
Darum muss man sich selber kümmern (gibt da ein FreeAndNil aber das würde auch nur diese eine Referenz-Variable auf nil setzen)

[jfheins]

Auch keine gute Lösung. Denn man bekommt erstmal nichts von dem Fehler mit.

Der "best practise" Weg ist eine Exception im Konstruktor zu werfen. Wenn das passiert, wird das Objekt automatisch wieder aufgeräumt und der Ersteller bekommt eine deutliche Warnung ;)

[Zacherl]

Da kann man sicherlich geteilter Meinung sein. Ich persönlich mag Klassen, die in ihren Methoden Exceptions werfen absolut nicht. Ich meine normalerweise würde man folgendes machen:

Delphi-Quellcode:

var
  Container: TContainer;
begin
  Containter := TContainer.Create(FileName);
  try
    if not Container.InvalidFile then
    begin

    end;
  finally
    Container.Free;
  end;
end;

Einen spezifischen Fehlercode kann man zur not auch noch irgendwo hinterlegen.

[DelphiNerd84]

wow, vielen dank für die schnellen antworten! habe schon befürchtet, dass man im constructor nicht gleich wieder den destructor aufrufen darf.. wäre auch zu schön gewesen! werde wohl den code auf mehrere blöcke aufteilen müssen, ähnlich Zacherl es vorgeschlagen hat. ein problem bleibt dennoch: wie kann ich herausfinden ob eine klasse noch existiert? sir rufo sagte bereits, dass das Zerstören die Referenzen nicht auf nil setzt! somit dürfte ja folgendes beispiel NICHT funktionieren..

Delphi-Quellcode:

var
  pFolder:Array Of ^TFolder;

Procedure RemoveFolder(Index:Integer;Recurse:Boolean);
Begin
 //TFolder(pFolder[Index]).DeleteChilds...
 TFolder(pFolder[Index]).Destroy;
End;

und dann später..

Procedure UpdateListItems;
var
  i:Integer;
Begin
 For i:=0To Pred(Length(pFolder))Do
  If TFolder(pFolder[i])=NIL Then RemoveListItem
End;

..aber wie kann ich dann mittels eines Pointers der auf eine Klasse verweist herausfinden ob diese überhaupt noch existiert (ohne eine exception aufzurufen)? naja werde mich wohl oder übel eingehender mit pointer, klassen etc. beschäftigen müssen ;) danke für eure geduld

[Sir Rufo]

Du könntest aber auch eine

Delphi-Quellcode:

class function

bemühen, die dir ein entsprechendes Objekt zurückliefest oder eben nicht (nil)

[mkinzler]

Dann halt statt

Delphi-Quellcode:

  finally
    Container.Free;
  end;

Delphi-Quellcode:

  finally
    FreeAndNil( Container);
  end;

Btw nie Destroy direkt aufrufen
Zudem würde ich statt eines arrays eine Liste verwenden

[jfheins]

Zitat:

Zitat von Zacherl (Beitrag 1096901)

Da kann man sicherlich geteilter Meinung sein. Ich persönlich mag Klassen, die in ihren Methoden Exceptions werfen absolut nicht. Ich meine normalerweise würde man folgendes machen:

Delphi-Quellcode:

var
  Container: TContainer;
begin
  Containter := TContainer.Create(FileName);
  try
    if not Container.InvalidFile then
    begin

    end;
  finally
    Container.Free;
  end;
end;

Einen spezifischen Fehlercode kann man zur not auch noch irgendwo hinterlegen.

OMG :roll:

Zuerst zu dem Fehlercode: Das läuft auf das Gleiche hinaus wie die Property. Man kann sie ignorieren. Und das ist nicht gut. Unter anderem aus diesem Grund gibt es ja gerade die Exceptions, damit nicht jede Klasse ihren eigenen Fehlermechanismus implementieren muss.

Das zweite: Man lässt den Code einfach mal laufen und guckt dann nachher, ob es einen Fehler gegeben hat. Warum nicht vorher prüfen?

Delphi-Quellcode:

var
  Container: TContainer;
begin
   if Fileexists(FileName) then
   begin
      Containter := TContainer.Create(FileName);
      try
         //Was machen
      finally
         Container.Free;
      end;
   end;
end;

Falls die Prüfung vorher tatsächlich nicht gewünscht ist, geht immer noch das hier:

Delphi-Quellcode:

try
   Containter := TContainer.Create(FileName);
except end;

  If Containter=NIL Then 
    ShowMessage('Containter is NIL')
   Else 
    ShowMessage('Containter is NOT NIL');
   Containter.DoSomething; //..erzeugt Exception!

Wenn der Konstruktor eine Exception wirft, wird die Variable nicht verändert - bleibt also nil.
Der Zugriff am Ende erzeugt aber immer noch eine AV.

[DelphiNerd84]

es funzt! keine ahnung ob des professionell gelöst ist, aber es geht erstmal

Delphi-Quellcode:

UNIT UContainer;
INTERFACE
USES
  SysUtils;
VAR
  pSelf:Pointer;
TYPE
  TContainer=Class
    Private
      FFileName:String;
      //pLevel:Arary Of ^TLevel;
      //Procedure HandleFile;
      //..
    Public
      Class Function Alive:Boolean;
      Constructor Create;
      Destructor Destroy;override;
      Procedure Assign(const FName:String;AsReadOnly:Boolean);
      Property FileName:String Read FFileName;
      //..
  End;
IMPLEMENTATION
Class Function TContainer.Alive:Boolean;
Begin
 Result:=pSelf<>NIL
End;
Constructor TContainer.Create;
Begin
 inherited Create;
 pSelf:=Pointer(Self);
 //..
End;
Destructor TContainer.Destroy;
Begin
 //..
 pSelf:=NIL;
 inherited Destroy
End;
Procedure TContainer.Assign(const FName:String;AsReadOnly:Boolean);
Begin
 If Not FileExists(FName)Then Begin
  //Destroy
  Free
 End
End;
END.

Delphi-Quellcode:

var
  P:Pointer;
Begin
 P:=TContainer.Create;
 TContainer(P).Assign('C:\Non-Existing-File.txt',False);
 If NOT TContainer(P).Alive Then Begin
  ShowMessage('Klasse existiert nicht mehr!');
  P:=NIL
 End
End;

Danke an alle! ;) und gute n8

[Zacherl]

Wie gesagt Geschmackssache. Angenommen die Prüfung gestaltet sich nicht so einfach, wie das Prüfen auf Vorhandensein einer Datei. Eine Exception wird im Constructor geworfen. Dann sieht der Quelltext schon extrem unübersichtlich aus:

Delphi-Quellcode:

var
  Container: TContainer;
begin
  try
    Containter := TContainer.Create(FileName);
    try
      // Hier mache ich was  
    finally
      Container.Free;
    end;
  except
    // Hier ging was schief
  end;
end;

[jfheins]

Okay, es kommt darauf an, ob die Klasse ohne Datei noch etwas leistet. nehmen wir als beispiel TFileStream. Wenn man den initialisiert, und eine ungültige Datei angibt (und das Flag so gesetzt ist, dass er eine erwartet) dann wirft das Ding eine FileNotFound Exception. Denn ohne Datei macht ein Filestream keinen Sinn.

Es ist Aufgabe des aufrufenden Codes, sicherzustellen dass das nicht passiert. Wenn der Filestream zum speichern benutzt wird, ist vermutlich fmCreate gesetzt. Beim Laden ist der Opendialog vermutlich so eingestellt dass er nur vorhandene Dateien zurückliefert. Beide Fälle vermeiden also im Normalfall die Exception wie es sein sollte, nur im Ausnahmefall gibt es sie dann halt doch.

Wenn die TContainer-Klasse ohne Datei keinen Nutzen hat, ist es nur korrekt, dass dann eine Exception geworfen wird wenn es diese Datei nicht gibt. Denn wenn ein Fehler erkannt wurde sollte dieser so früh wie möglich "gemeldet" werden. Das erleichtert die Fehlersuche ungemein ;)

Wenn die TContainer-Klasse auch ohne gültige Datei einen Nutzen hat, sollte der Dateiname in eine Property ausgelagert werden. (Optional ein überladener Konstruktor der einen dateinamen entgegennimmt.)

Im Konstruktor eine Prüfung einzubauen, und das Objekt ggf. wieder zu zerstören empfinde ich als grottenschlechten Programmierstil. Schon bei dem Standardkonstrukt

Delphi-Quellcode:

var
  Container: TContainer;
begin
    Containter := TContainer.Create(FileName);
    try
      // Hier mache ich was 
    finally
      Container.Free;
    end;
end;

bekommt man eine AV und weiß nicht warum.

Die "Lösung" aus Post #10 könnte man direkt als Gegenbeispiel für guten Code verwenden. Im speziellen stört mich:
Eine globale Variable pSelf bestimmt das Verhalten des Objekts, man darf anscheinend keine 2 Objekte direkt hintereinander erstellen, sonst zerschießt man den Mechanismus komplett - OMG.
Es wird zwar der Name "Assign" benutzt, aber nicht für den Zweck den es sonst erfüllt.
Man muss nachher das Objekt fragen, ob es einen Fehler gefunden hat und muss dann ggf. seine Variable selber wieder nil setzen.

Ganz ehrlich - die einzige Möglichkeit, diesen Code noch schlimmer zu machen ist ein goto.
@Zacherl: das ist vielleicht ein bisschen unübersichtlich, weil da 2 try-Blöcke vorkommen. Aber wenn man sich das genauer anguckt merkt man schnell, was passiert. Und welcher Code wann ausgeführt wird.
Da ist der Code

Delphi-Quellcode:

var
  P:Pointer;
Begin
 P:=TContainer.Create;
 TContainer(P).Assign('C:\Non-Existing-File.txt',False);
 If NOT TContainer(P).Alive Then Begin
  ShowMessage('Klasse existiert nicht mehr!');
  P:=NIL
 End
End;

zwar "flacher" von der Einrückung her, aber warum man jetzt das eine oder andere macht wird nicht deutlich. Der Code hat weniger Struktur.

Ich sehe hier ein Problem das durch Exceptions hervorragend gelöst werden kann und wundere mich etwas, dass so eine Krücke geschrieben wird, um Exceptionhandling zu vermeiden. Hängt möglicherweise auch mit Unwissenheit (seitens des TE) zusammen, deshalb bin ich gerne Bereit das ganze ein bisschen mehr zu erklären, wenn das gewünscht wird ;)

Btw.: Willkommen in der DelphiPRAXiS :hi:

[Sir Rufo]

ich hatte das eigentlich so gedacht:

Delphi-Quellcode:

UNIT UContainer;

INTERFACE

USES
  SysUtils;

TYPE
  TContainer=Class
    Private
      FFileName:String;
    Public
      class function Build(const FName:String;AsReadOnly:Boolean) : TContainer;
      Constructor Create(const FName:String;AsReadOnly:Boolean);
      Destructor Destroy;override;
      Property FileName:String Read FFileName;
  End;

IMPLEMENTATION

class function TContainer.Build(const FName:String;AsReadOnly:Boolean) : TContainer;
begin
  if FileExists( FName ) then
    Result := TContainer.Create( FName, AsReadOnly )
  else
    Result := nil;
end;

Constructor TContainer.Create(const FName:String;AsReadOnly:Boolean);
Begin
 inherited Create;
  FFileName := FName;
  // ...
End;

Destructor TContainer.Destroy;
Begin

 inherited Destroy
End;

END.

Aufruf:

Delphi-Quellcode:

var
  Container : TContainer;
begin
  Container := TContainer.Build( 'foo.txt', False );
  if Assigned( Container ) then
  begin
    
  end;
end;

[himitsu]

Wozu eigentlich das Rumgecaste mit dem Pointer?

Vorallem für Anfänger wäre das nicht zu empfehlen, da man sich dadurch schnell mal Fehler und unnötige Probleme einhandeln kann.