Problem mit Constructor/Destructor

[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

[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.