server_mainform_unit enthaelt unnoetige Namensbestandteile. Das es eine
Unit ist weiss man doch schon.
"geg1" unnoetig abgekuerzt und nicht mit Grossbuchstaben vorne.
"Profile1" vs "Profile2" da stimmt doch was nicht. Nie gleiche Menuepunkte in veschiedenen Untermenues.
Besser alles in englisch. Ich hasse gemischtsprachliche Programme.
"Prepare_classes" besser "PrepareClasses". Man sollte sich strikt an den CamelCase halten und auch die Gross/Kleinschreibung durchziehen als ob Delphi casesensitiv waere. Es liest sich einfach leichter.
Desgleichen bei den Typen, also "Boolean", "Integer", "True", "False" usw. so wie die Delphi-Tooltips es vorgeben. Nur "string" ist da eine Ausnahme. Schluesselworte sollten klein geschrieben werden.
"Free_Classes" "besser "FreeClasses".
"{ Private-Deklarationen }" schmeiss diesen Mist raus. Da hast du doch genau schon das gemacht zu was der Kommentar dich auffordert.
"filetocheck: TFilename" besser "FileToCheck: TFileName" oder "FileToCheck: string". TFileName gibt es nur damit man in der
IDE einen eigenen Property-Editor daran binden kann.
"TLog = class" in der JVCL bevorzugen wir immer eine explizite Ableitungsangabe. "TLog = class(TObject)".
"File_version", "server_mainform_form: Tserver_mainform_form", "Server_version" das sollte inzwischen klar sein.
"//********************** Ausführende Prozeduren ******************************//" nichtssagend und falsch und daher ueberfluessig.
{ TLog.AddMessageToLog(Sender: TMemo; TextMessage: string);
-----------------------------------------
Auszuführender Befehl:
- Fügt einem Memo eine Nachricht mit Uhrzeit hinzu}
Ebenso mit unnoetigen Bestandteilen versehen. Nur "Fügt einem Memo eine Nachricht mit Uhrzeit hinzu" enthaelt Information. Alles andere weglassen.
"timetostr(now)" besser "TimeToStr(Now)". Der CamelCase wurde zur besseren Lesbarkeit erfunden.
Sender.Lines.Add(timetostr(now) + ' : ' + TextMessage);
Solche Strings sollten immer mit Format zusammengebaut werden und der Formatstring sollte die Positionsangaben %1 ... enthalten.
Der Grund ist die Internationalisierung des Programms. Andere Sprachen koennen die Satzbestandteile unterschiedlich zusammenbauen.
"if server_mainform_form.IdTCPServer1.Active = True then" umstaendlich. Einfacher ist "if server_mainform_form.IdTCPServer1.Active then".
Prepare_classes ist gefaehrlich. Besser den Konstruktor der Form ueberschreiben und dort die Klassen anlegen. Entsprechend gehoert Free_Classes in den Destruktor.
"function Tserver_mainform_form.Import(AType: integer; AFilename: string): boolean;" ist falsch segmentiert. Das sind zwei unterschiedliche Aufgaben die in separate Methoden gehoeren.
"if FileExists(AFilename) = False then" besser "if not FileExists(AFileName) then".
"if RightStr(AFilename, 4) <> '.xml' then" besser "if ExtractFileExt(AFileName) <> '.xml' then", das zeigt besser an das es hier um Filenamen geht.
Ich hasse diese Vortests mit Exit.
"Free_Classes(1);" wenn man es nicht sowieso anders segmentieren muesste, dann muesste hier eine Enumeration fuer den Parameter verwendet werden. Der Name ist dann viel aussagekraeftiger und sicher gegen Fehleingaben. Eine 4 wuerde naemlich durch den Compiler gehen, aber nicht funktionieren.
"procedure Tserver_mainform_form.ExecuteDialog(AOnCanClose: TCloseQueryEvent; AType: TDialogType);" ueblicherweise will man doch Open- und Save-Dialoge als Komponenten auf der Form, damit sie sich merken koennen wo man schon mal war.
Wieder falsch segmentiert. Entweder zwei einzelne Methoden oder ausnutzen das TSaveDialog und TOpenDialog von TCommonDialog abstammen und alles nur einmal machen. Das hier ist klassisches Copy & Paste-Programmieren.
"FormCloseQuery" ist falsch. Das sollte alles nach "FormClose".
"'\FDS_Server_profiles.xml'" nie Stringliterale wiederholen sondern Konstanten benutzen. In diesem Fall "cServerProfile = 'FDS_Server_profiles.xml';". Entsprechend nie den Pfad explizit zusammenbauen, sondern in eine Methode konzentrieren. Das erlaubt es dann den Speicherort zentral zu aendern.
"FormShow" ist ein Versager. Die Methode wird oefters aufgerufen, aber "Prepare_Classes" ist nicht auf Mehrfachaufruf vorbereitet.
Besser keine unnoetigen begin end Bloecke benutzen. Zeilen die nicht da sind stoeren beim Lesen nicht.
"end; // case" Ich hasse diese Kommentare. Die Einrueckung sollte doch ausreichend sein.