Besprechnungsanfrage (iCalendar)

[himitsu]

Du solltest drinngend nochmal einen Kurs über OOP belegen.

Wozu gibt es diese globale Variable CD?
Wenn, dann würde diese maximal als Feld in die Form gehören.
Aber eigentlich wird diese Variable nur in Button1Click verwendet, also gehört sie dort auch rein.

Und schon würde dir auffallen, daß du hier total Mist gebaut hast.

Delphi-Quellcode:

TCalendarData.CreateInvitation(IdMessage: TIdMessage);
begin
  with cd.cdBody do

Genauso wie einem etwas komisch vorkommen sollte, wenn ma die Variable Form1, bei soeinen einfachen Code aus nur einer Unit/Form, niemals irgendwo verwendet werden sollte (außer es ist wirklich notwendig, was es aber fast niemals ist)

Also, wieso wird da innerhalb der Klasse TCalendarData auf eine externe Variable (CD) zugegriffen, anstatt auf seine eigenen Felder?
Die globale Variable CD entspricht hier zwar zufällig mal der eigenen Instanz, aber das muß nicht immer so sein.

Auch wird die Variable cdBody nur in CreateInvitation verwendet, womit Diese auch nur in dieser Methode deklariert sein sollte.
Obwohl man da eigentlich auch direkt IdMessage.Body verwenden könnte.

[tdroese]

Hallo Himitsu,

vielen Dank für den Hinweis und auch all die anderen Tipps vorab. Ich mag klare Worte .

Habe leider noch nie einen Kurs in Delphi belegt (sieht man offensichtlich ).

Aber man kann nur besser werden, also danke ich Dir für den Hinweis.

Einen schönen Tag wünscht,

Thomas

[himitsu]

Pssst, nicht erschrecken, aber sogesehn hatte ich auch nie einen Kurs gehabt und es stecken einfach nur viele Jahre an guten oder schlechten Erfahrungen dahinter.
Vieles selbst beigebracht (ausprobieren, bis es irgendwie klappt), in der DP auch noch viel gelernt und über die Zeit auch vieles "falsch", bzw. ungünstig gelöst.
Durch die DP, DT und Co. auch noch Dinge erfahren, wie man etwas besser lösen könnte/sollte.
Also keine Sorge, jeder fing mal klein an.

So hatte/hab ich auch gerne große funktionen gehabt, aber es macht sich einfach besser, wenn man sowas aufteilt.
z.B. wie Nachfolgend.

So hat man je eine Teilfunktionalität (Mail zusammenbauen, Verschlüsselung und Senden in Funktionen getrennt, die fast komplett auf Bildschirm passt).
Und jetzt hätte man noch genug Platz, um z.B. Funktionen für Übertragungskomprimierungen oder Proxies dazwischenzuschieben.

Das du dir extra Kommentare einbauen mußt, um viele Seiten später noch zu erkennen wozu ein End gehört, sollte dir da etwas zu denken geben.
PS: end;//BreakLines hast du dabei ganz übersehen ... vermutlich hieß die Funktion früher mal so und nun hast du auch noch einen "verwirrenden" Kommentar im Code.
Kommentare sind wichtig, aber zuviele, nutzlose oder redundante Informationen sind eher hinderlich.

Delphi-Quellcode:

procedure TCalendarData.SendCalendarRequest;
var
   SMTP: TIdSMTP;
   IdMessage: TIdMessage;
begin
   IdMessage := TIdMessage.Create(nil);
   try
     CreateMessage(IdMessage);
     SMTP := TIdSMTP.Create(nil);
     try
       InitAuthentification(SMTP);
       SendMail(SMTP);
     finally
       SMTP.Free;
     end;
   finally
     IdMessage.Free;
   end;
end;

Nochmal als Kommentare davor, was wo drinnen wäre:

Delphi-Quellcode:

procedure TCalendarData.SendCalendarRequest;
var
   SMTP: TIdSMTP;
   IdMessage: TIdMessage;
begin
   IdMessage := TIdMessage.Create(nil);
   try
     //with IdMessage do
     //begin
     //  //Header setzen
     //...  
     //CreateInvitation(IdMessage);
     CreateMessage(IdMessage);
     SMTP := TIdSMTP.Create(nil);
     try
       //if cdSSLConnection = true then
       //begin
       //  SSLHandler := TIdSSLIOHandlerSocketOpenSSL.Create(SMTP);
       //...
       //  SMTP.AuthType := satNone;
       //end;
       InitAuthentification(SMTP);
       //SMTP.Host := cd.cdHost;
       //...
       //  SMTP.Disconnect;
       //end;
       SendMail(SMTP);
     finally
       SMTP.Free;
     end;
   finally
     IdMessage.Free;
   end;
end;