Einzelnen Beitrag anzeigen

nahpets
(Gast)

n/a Beiträge
 
#5

AW: Code Analyse von (semi) Profis

  Alt 1. Jun 2013, 21:52
Hallo Benny,

der Quelltext könnte bei mir in etwa so aussehen:
(Allerdings wird hier von der Forumssoftware ein nicht unerheblicher Teil von Leerzeichen unterschlagen. So stehen bei mir bei den Variablendeklarationen die : vor dem Variablentyp untereinander. Ebenso stehen bei mehreren Zuweisung hintereinander die := untereinander.)
Delphi-Quellcode:
// Die Zeilenbreite geht bei mit bis ca. 80 Zeichen.
// Ich hasse es, wenn ich zum Lesen von Quelltext zwei Monitoren nebeneinander
// benötige bzw. ständig hin- und herscrollen muss.
// Damit ich Quelltexte anstrengungsfrei lesen kann, muss die Schriftgröße von Courier-New
// so sein, dass etwa 90 Zeichen in eine Zeile passen. Bei kleineren Schriften bekomme ich
// (Seh-)Probleme oder muss mit der Nase (fast) in den Bildschirm. (Bildschirmauflösung: 1680 * 1050)

// Abgesehen davon ist "überbreiter" Quelltext schlecht zu drucken und sieht
// auch in Dokumentationen nicht wirklich toll aus, da er dort dann irgendwie
// umgebrochen wird und seine logische Struktur verloren geht.

// Für die Namensvergabe nutze ich (weitgehend) die ungarische Notation.

// Entgegen "Vorgaben" von Borland... gehört das "begin" bei mir hinter then...
// In einer eigenen Zeile macht es für mich den Quelltext ausschließlich länger,
// aber nicht lesbarer. Und wenn "begin" eingerückt wird und anschließend in der
// nächsten Zeile der weitere Quelltext auch noch, so macht es den Quelltext nur "breiter".
// Und wenn dann durch diese Einrückungen der Quelltext erst in der rechten Bildschirmhälfte
// beginnt, geht die Lesbarkeit dann doch irgendwie flöten.

// Leerzeilen sind bei mir für die logische Trennung von Quelltextzeilen.
// Hinter function und procedure stören sie mich, weil ich hier immer davon
// ausgehe, dass nicht zusammengehörende Quelltexte folgen.

// Kommentare nur dann hinter eine Quelltextzeile, wenn sie vollständig im
// sichtbaren Bereich des Bildschirmes bleiben, bei einer "normalen" Auflösung,
// also auch für jemand, der keine Adleraugen hat, anstrengungsfrei lesbar bleiben.
// Ansonsten in die Zeile dadrüber unter Beachtung der Einrückung.

// Hinter kommentarbeginnende bzw. vor kommentarbeendende Zeichen gehört ein
// Leerzeichen.
// Kommentare, die aus vollständigen Sätze bestehen, sollte auch die entsprechenden
// Satzzeichen enthalten. Und wenn die Rechtschreibung auch noch (weitgehend)
// in Ordnung ist, bin ich zufrieden.
// Bei Meldungen, die an den Benutzer gehen, muss die Rechtschreibung stimmen.
// Aus der Qualität der Meldungen schließe ich immer auf die Qualität der Software.
// Fehlerhafte Meldung bedeuten bei mir, dass die Software vermutlich auch
// nicht besser ist.

// Bei Funktionsaufrufen... werden die Parameter untereinandergeschrieben,
// sofern sie hintereinandergeschrieben die normale Bildschirmbreite deutlich
// übersteigen.
// Hinter Satzzeichen (auch in Funktionsaufrufen...) gehört ein Leerzeichen.
// Dadurch sind die einzelnen Parameter besser unterscheidbar.

// Wenn hinter einem else mehr als eine Zeile folgt (incl. Kommentarzeilen)
// wird mit begin und end "geklammert". Dadurch ist (für mich) die Logik
// besser optisch darstellbar.

// Vor und hinter + - := ... gehört ein Leerzeichen, damit man die Trennung
// innerhalb der Quelltextzeilen besser erkennen kann. "eingebettete" + und - ...
// kann man schonmal schnell übersehen.

// Bitte auf Apostroph verzichten, wenn nicht erforderlich.
// 'Zielverzeichnis des GPS Ordner´s' <- seit wann mit? Und dann nichtmal '?
// 'Zielverzeichnis des GPS-Ordners' <- finde ich irgendwie "schöner" ;-)

var
  UserDirPageCAE : TInputDirWizardPage;
  DOSBoxConf : TStringList;
  GPSDirCheckBox : TRadioButton;
  SplashImage : TBitmapImage;
  SplashForm : TForm;
  SetupMajor : dword;
  SetupMinor : dword;
  SavedMajor : dword;
  SavedMinor : dword;

  CAE2000Path : string;
  DOSBoxCAE2000Path : string;
  DOSBoxCAE2000UninstallName : string;
  DOSBoxCAE2000UninstallNameRemovedQuotes : string;
  DOSBoxCAE2000UninstallPath : string;
  DOSBoxCAE2000UninstallPathRemovedQuotes : string;
  DOSBoxCAE2000UninstallString : string;
  DOSBoxCAE2000UninstallStringRemovedQuotes : string;
  GPSPath : string;
  GetCAE2000Path : string;
  SplashFileName : string;
  UserFolderCAE : string;
  tmp : string;

  GPSDirCheckBoxChecked : boolean;
  ResultDOSBoxCAE2000PathCheck : boolean;
  ResultAktuelleAppFound : boolean;
  ResultDOSBoxCAE2000DontDeinstall : boolean;

  // Das ist nicht zwingend aussagefähig!
  a, b, c, d : word;
  // abcd scheinen mir nichtmal erforderlich zu sein.

  // I kann in der Prozedur deklariert werden.
  I : Integer;
  // Variablen möglichst dort deklarieren, wo sie benötigt werdenund nicht pauschal
  // als globale Variabeln.
  // Auch dann nicht, wenn man zufällig in zwei Funktionen eine Stringvariabel tmp
  // benötigt. Dann bekommt jede Funktion ihre eigene Variabel.

// Registry-Schlüssel für die Versionsnummer
function CreateDWord(const Hi, Lo : word) : dword;
begin
  Result := (Hi shl 16) or Lo;
end;

// Entschlüsselung der Versionsnummer
function DecodeVersion(const dwMajor, dwMinor : dword) : string;
begin
  a := word(dwMajor shr 16);
  b := word(dwMajor and not $ffff0000);
  c := word(dwMinor shr 16);
  d := word(dwMinor and not $ffff0000);
  Result := Format('%d.%d.%d.%d',[a,b,c,d]);
   // Geht das nicht?
  Result := Format('%d.%d.%d.%d',[word(dwMajor shr 16),
                                  word(dwMajor and not $ffff0000),
                                  word(dwMinor shr 16),
                                  word(dwMinor and not $ffff0000)]);
end;

// Versionsabgleich der vorhandenen Installation zur neuen Installation
function IsSetupNewer : boolean;
var
  ResultCode : integer;
begin
  if (not RegQueryDWordValue(HKLM,'{#UNINSTKEY}', 'MajorVer', SavedMajor))
  or (not RegQueryDWordValue(HKLM,'{#UNINSTKEY}', 'MinorVer', SavedMinor)) then begin
    if (RegQueryStringValue(HKLM,'{#UNINSTKEY}', 'UninstallString', tmp))
    and (tmp <> '')
    and (fileexists(tmp)) then begin
      Result := (MsgBox(ExpandConstant('{cm:NotVerifiedVersionFound}'),
                        mbConfirmation,
                        MB_YESNO or MB_DEFBUTTON2) = IDYES);
      exit;
    end;
  end;

  SetupMajor := CreateDWord({#MAJOR},{#MINOR});
  SetupMinor := CreateDWord({#RELEASE},{#BUILD});

  // Neuere Version ist installiert
  Result := (SetupMajor > SavedMajor)
         or ((SetupMajor = SavedMajor) and (SetupMinor >= SavedMinor));
  // Gleiche Version ist bereits installiert
  ResultAktuelleAppFound := (SetupMajor = SavedMajor)
                         or ((SetupMajor = SavedMajor) and (SetupMinor = SavedMinor));

  // Ermitteln und Umformen des Deinstallationspfades
  RegQueryStringValue(HKLM,'
{#UNINSTKEY}
',
                      'UninstallString',
                      DOSBoxCAE2000UninstallString);
  DOSBoxCAE2000UninstallPath := (ExtractFileDir(DOSBoxCAE2000UninstallString) + '\' + '"');
  DOSBoxCAE2000UninstallName := ('"' + ExtractFileName(DOSBoxCAE2000UninstallString));

  DOSBoxCAE2000UninstallStringRemovedQuotes := RemoveQuotes(DOSBoxCAE2000UninstallString);
  DOSBoxCAE2000UninstallPathRemovedQuotes := RemoveQuotes(DOSBoxCAE2000UninstallPath);
  DOSBoxCAE2000UninstallNameRemovedQuotes := RemoveQuotes(DOSBoxCAE2000UninstallName);

  if not Result then begin
    if RegQueryStringValue(HKLM,
                           '{#UNINSTKEY}',
                           'InstallLocation',
                           DOSBoxCAE2000Path) then begin
      result := DirExists(DOSBoxCAE2000Path);
    end;
    if not Result then begin
      // Installationspfad der vorhandenen Installation kann nicht ermittelt werden.
      MsgBox(ExpandConstant('{cm:DOSBoxPathNotFound}'), mbError, MB_OK);
      // Abbruch der Installation, wenn der Pfad aktuelle Installationspfad nicht
      // ermittelt werden konnte
      ResultDOSBoxCAE2000PathCheck := false;
    end;
    // Meldung, dass eine aktuellere Version installiert ist.
    if MsgBox(ExpandConstant('{cm:NewerAppFound}'),
              mbError,
              MB_YESNO or MB_DEFBUTTON2) = IDYES then begin
      // Silent-Ausführung der Deinstallation, wenn aktuellere Version installiert
      // ist und der Benutzer diesem zustimmte.
      Exec(ExpandConstant(DOSBoxCAE2000UninstallStringRemovedQuotes),
           '/SILENT',
           '',
           SW_SHOW,
           ewWaitUntilTerminated,
           ResultCode);
    end else begin
      // Abbruch des Setups, sobald der Benutzer sich gegen eine Deinstallation
      // der aktuellen Version entscheidet.
      ResultDOSBoxCAE2000DontDeinstall := true;
    end;
  end;

  if ResultAktuelleAppFound = true then begin
    // Meldung, dass die aktuellere Version gleich der zu installierenden ist.
    MsgBox(Format(ExpandConstant('{cm:AktuelleAppFound}'),
                  [DecodeVersion(SavedMajor, SavedMinor)]),
           mbConfirmation,
           MB_OK);
  end;
end;

// Aktuelle Versionsnummer in die Registry schreiben.
function SaveVersionInfo: boolean;
begin
  Result := (RegWriteDWordValue(HKLM,'{#UNINSTKEY}','MajorVer',
                                CreateDWord({#MAJOR},{#MINOR})))
        and (RegWriteDWordValue(HKLM,'{#UNINSTKEY}','MinorVer',
                                CreateDWord({#RELEASE},{#BUILD})));
end;

function InitializeSetup: boolean;
begin
  UserFolderCAE := '';
  // Die folgende Zeile dürfte wohl überflüssig sein.
  Result := true;
  Result := IsSetupNewer;

  // Abbruchfunktion, wenn der aktuelle Installationspfad nicht ermittelt werden
  // konnte.
  // Ist hier gemeint: Abbruch der Funktion.
  // Für mich ist eine Abbruchfunktion eine Funktion die ein Programm abbricht.
  // (Ich weiß, Haarspalterei ;-))
  if ResultDOSBoxCAE2000PathCheck = true then begin
    result := false;
  end;

  // Abbruchfunktion, wenn der Benutzer sich gegen eine Deinstallation entschieden hat.
  if ResultDOSBoxCAE2000DontDeinstall = true then begin
    Result := false;
  end;

  // Abbruchfunktion, wenn bereits die aktuelle Version installiert ist.
  if ResultAktuelleAppFound = true then begin
    result := false;
  end;
end;

// Schreiben des, vom Benutzer ermittelten, CAE2000-Pfades.
function InstallCae(Param : String) : String;
begin
  Result := UserDirPageCAE.Values[0];
  CAE2000Path := UserDirPageCAE.Values[0];
  // Erstellen eines Registry-Eintrages für den CAE2000-Pfad
  RegWriteStringValue(HKLM, '{#UNINSTKEY}', 'CAE2000Location', CAE2000Path);
end;

function GPSDir(Param:String):String;
begin
  GPSPath := UserDirPageCAE.Values[1];
end;

// Einblenden und Einstellen eines separaten GPS-Pfades.
procedure GPSDirCheckBoxOnClick(Sender : TObject);
begin
  if GPSDirCheckBox.Checked then begin
    // GPS-Verzeichnisauswahlfenster
    UserDirPageCAE.Add('Zielverzeichnis des GPS Ordners');
    // GPS-Standardpfad in der Verzeichnisauswahl
    UserDirPageCAE.Values[1] := ExpandConstant('C:\CAE2000\');
  end
end;

procedure InitializeWizard;
begin
  // Einblenden vom WISAG-Logo für Zeit x.
  SplashFileName := ExpandConstant('{tmp}\WISAG Logo weiß.bmp');
  // WISAG-Logo aus dem tmp-Verzeichnis aufrufen.
  ExtractTemporaryFile(ExtractFileName(SplashFileName));

  SplashForm := TForm.create(nil);
  with SplashForm do begin
    BorderStyle := bsNone;
    Position := poScreenCenter; // Bildposition angeben
    ClientWidth := 980; // Bildgröße angeben (Breite in px)
    ClientHeight := 467; // Bildgröße angeben (Höhe in px)
  end;

  SplashImage := TBitmapImage.Create(SplashForm);
  with SplashImage do begin
    Bitmap.LoadFromFile(SplashFileName);
    Stretch := true;
    Align := alClient;
    Parent := SplashForm;
  end;

  with SplashForm do begin
    Show;
    // So schnell kann ich nicht gucken, da kann man auf den Splashscreen
    // auch verzichten.
    // Anzeigedauer in Sekunden, 1 to 2 = 2 Sekunden.
    for I := 1 to 2 do begin
      Repaint;
      Sleep(1000);
    end;
    Close;
    Free;
  end;

  // Zusätzliche Verzeichnisauswahlseite einbinden.
  UserDirPageCAE := CreateInputDirPage(wpSelectDir,
                    ExpandConstant('{cm:CAEVerzeichnis}'),
                    ExpandConstant('{cm:SpeicherortPRTDEF}'),
                    ExpandConstant('{cm:SpeicherzielPRTDEF}'),
                    false,
                    'Neuer Ordner');

  // CAE-Verzeichnisauswahlfenster
  UserDirPageCAE.Add('Zielverzeichnis der CAE2000.EXE');

  // CAE-Standardpfad in der Verzeichnisauswahl.
  UserDirPageCAE.Values[0] := ExpandConstant('C:\CAE2000\');

  // Radio-Button zum Einblenden bei separatem GPS-Pfad.
  GPSDirCheckBox := TRadioButton.Create(UserDirPageCAE);

  // Parameter für die GPS-CheckBox.
  with GPSDirCheckBox do begin
    GPSDirCheckBox.Parent := UserDirPageCAE.Surface;
    Caption := ExpandConstant('{cm:GPSVerzeichnis}');
    Left := ScaleX(0);
    Top := ScaleY(120);
    Width := ScaleX(400);
    Height := ScaleY(40);
    Checked := GPSDirCheckBoxChecked;
    OnClick := @GPSDirCheckBoxOnClick;
  end;
end;

// Eintragen der benutzerspezifischen Pfadangaben in die DOSBox-Konfigurationsdatei.
procedure CurStepChanged(CurStep: TSetupStep);
begin
  if CurStep = ssPostInstall then begin;
    DOSBoxConf := TStringList.Create;
    DOSBoxConf.LoadFromFile((ExpandConstant('{localappdata}\DOSBox\dosbox-SVN_MB6.conf')));
    DOSBoxConf[298] := copy(DOSBoxConf[298],1,22)
                     + (ExpandConstant('{app}\PrintOut\SELECT.PCL'))
                     + copy(DOSBoxConf[298],30,length(DOSBoxConf[298]));
    DOSBoxConf[355] := copy(DOSBoxConf[355],1,9)
                     + (ExpandConstant('"{code:InstallCae}\"'))
                     + copy(DOSBoxConf[355],24,length(DOSBoxConf[355]));
    DOSBoxConf[356] := copy(DOSBoxConf[356],1,9)
                     + (ExpandConstant('"{app}\"'))
                     + copy(DOSBoxConf[356],24,length(DOSBoxConf[356]));
    if GPSDirCheckBox.Checked then begin
      DOSBoxConf[357] := copy(DOSBoxConf[357],1,1)
                       + ' mount D '
                       + (ExpandConstant('"{code:GPSDir}\"'))
                       + copy(DOSBoxConf[357],24,length(DOSBoxConf[357]));
    end;
    DOSBoxConf.SaveToFile((ExpandConstant('{localappdata}\DOSBox\dosbox-SVN_MB6.conf')));
    DOSBoxConf.Free;
    SaveVersionInfo;
  end;
end;

// Code für die Deinstallation.
procedure CurUninstallStepChanged(CurUninstallStep: TUninstallStep);
begin
  RegQueryStringValue(HKLM,'{#UNINSTKEY}','CAE2000Location',GetCAE2000Path);
  if CurUninstallStep = usAppMutexCheck then begin
    // Kopieren der Backupdatei ins Originalverzeichnis.
    FileCopy(ExpandConstant(GetCAE2000Path) + '\Backup\PRTDEF.GER',
             ExpandConstant(GetCAE2000Path) + '\PRTDEF.GER',
             false);
  end;

  if CurUninstallStep = usDone then begin
    // Löschen des Rigistryeintrages nach der Deinstallation.
    RegDeleteKeyIncludingSubkeys(HKLM, '{#UNINSTKEY}');
  end;
end;
Allerdings gibt es keine Verpflichtung meinen Ansichten auch nur ansatzweise zuzustimmen. Es ist immer auch etwas Geschmackssache. Man sollte sich in Firmen und/oder Teams aber auf einheitliche optische Gestaltung von Quelltexten einigen. Ebenso sollte die Vergabe von Varibalnnamen, Funktion- und Prozedurenamen... einheitlich sein.

In anbetracht dessen, was ich im Laufe meines Berufslebens schon so an Quelltexten zu sehen und zu pflegen bekommen habe, ist Dein "Erstlingswerk" doch durchaus akzeptabel. Spätestens nach dem dritten Lesen sollte man damit (trotz vollkommen fremder Materie) klarkommen können.
Oder anders formuliert: Auch wer kein Pascalscript kann, sollte Deinen Quelltext lesen und (weitgehend) verstehen können.

PS.:
Zitat von Christian Seehase:
Wenn Du Variablen funktionsübergreifend benötigst, dann kannst Du sie, z.B., als Felder des Formulares deklarieren.
Das dürfte hier nicht gehen, da es sich nicht um Delphi sondern um Pascalscript innerhalb von INNO-Setup handelt.
  Mit Zitat antworten Zitat