Was ist an diesen try und except block falsch?

[Deltachaos]

Ich weis nicht das ich hier falsch gemacht habe.
Es werden nur die versionen die mit dem erste und dem letzte try und except block ausgelesen werden angezeigt.

währe net wen da wer rüber gugen würde.

Delphi-Quellcode:

uses Registry;

procedure delphi_versions;
var regist: TRegistry;
    pub: TStringList;
    pubdir, pubbrc32, pubbrcc32: string;
    pubi: integer;
begin
  regist := TRegistry.Create;
  try
    regist.RootKey := HKEY_LOCAL_MACHINE;
    try
      pub := TStringList.Create;
      pubdir := '';
      pubbrc32 := '';
      pubbrcc32 := '';
      pubi := 0;
      regist.OpenKey('SOFTWARE\Borland\Delphi', false);
      regist.GetKeyNames(pub);
      regist.CloseKey;
      for pubi := 0 to pub.count - 1 do
      begin
        regist.OpenKeyReadOnly('SOFTWARE\Borland\Delphi\' + pub[pubi]);
        if regist.ValueExists('RootDir') then
        begin
          pubdir := regist.ReadString('RootDir');
          if pubdir[length(pubdir)] = '\' then
            pubdir := pubdir + 'Bin\'
          else
            if pubdir[length(pubdir)] = '/' then
              pubdir := pubdir + 'Bin/'
            else
              pubdir := pubdir + '\Bin\';
          if FileExists(pubdir + 'brc32.exe') then
            pubbrc32 := pubdir + 'brc32.exe';
          if FileExists(pubdir + 'brcc32.exe') then
            pubbrcc32 := pubdir + 'brcc32.exe';
          additem(pub[pubi], pubbrc32, pubbrcc32);
        end;
      end;
    finally
    end;


    try
      pub := TStringList.Create;
      pubdir := '';
      pubbrc32 := '';
      pubbrcc32 := '';
      pubi := 0;
      regist.OpenKey('SOFTWARE\CodeGear\BDS', false);
      regist.GetKeyNames(pub);
      regist.CloseKey;
      for pubi := 0 to pub.count - 1 do
      begin
        regist.OpenKeyReadOnly('SOFTWARE\CodeGear\BDS\' + pub[pubi]);
        if regist.ValueExists('RootDir') then
        begin
          pubdir := regist.ReadString('RootDir');
          if pubdir[length(pubdir)] = '\' then
            pubdir := pubdir + 'Bin\'
          else
            if pubdir[length(pubdir)] = '/' then
              pubdir := pubdir + 'Bin/'
            else
              pubdir := pubdir + '\Bin\';
          if FileExists(pubdir + 'brc32.exe') then
            pubbrc32 := pubdir + 'brc32.exe';
          if FileExists(pubdir + 'brcc32.exe') then
            pubbrcc32 := pubdir + 'brcc32.exe';
          additem(pub[pubi], pubbrc32, pubbrcc32);
        end;
      end;
    finally
    end;


    try
      pub := TStringList.Create;
      pubdir := '';
      pubbrc32 := '';
      pubbrcc32 := '';
      pubi := 0;
      regist.OpenKey('SOFTWARE\Borland\BDS', false);
      regist.GetKeyNames(pub);
      regist.CloseKey;
      for pubi := 0 to pub.count - 1 do
      begin
        regist.OpenKeyReadOnly('SOFTWARE\Borland\BDS\' + pub[pubi]);
        if regist.ValueExists('RootDir') then
        begin
          pubdir := regist.ReadString('RootDir');
          if pubdir[length(pubdir)] = '\' then
            pubdir := pubdir + 'Bin\'
          else
            if pubdir[length(pubdir)] = '/' then
              pubdir := pubdir + 'Bin/'
            else
              pubdir := pubdir + '\Bin\';
          if FileExists(pubdir + 'brc32.exe') then
            pubbrc32 := pubdir + 'brc32.exe';
          if FileExists(pubdir + 'brcc32.exe') then
            pubbrcc32 := pubdir + 'brcc32.exe';
          additem(pub[pubi], pubbrc32, pubbrcc32);
        end;
      end;
    finally
    end;

  finally
    regist.free;
  end;
end;

[jfheins]

Du weist schon, dass ein

Delphi-Quellcode:

try
// ...
// ...
finally
end;

Ungefähr ... gar keinen Effekt hat?

[Deltachaos]

nein weis ich nicht.
also muss ich etwas nach dem finally angeben oder wie schat das aus?

[taaktaak]

In den finally-Block gehört Code, der unter allen Umständen, also auch bei einem Fehler, ausgeführt werden muss. Also z.B. das Freigeben von Ressourcen.

[Deltachaos]

nun gut.

nur jetzt habe ich mal alles aus den blocks herausgenommen also so:

Delphi-Quellcode:

procedure delphi_versions;
var regist: TRegistry;
    pub: TStringList;
    pubdir, pubbrc32, pubbrcc32: string;
    pubi: integer;
begin
  regist := TRegistry.Create;
  try
    regist.RootKey := HKEY_LOCAL_MACHINE;
//    try
      pub := TStringList.Create;
      pubdir := '';
      pubbrc32 := '';
      pubbrcc32 := '';
      pubi := 0;
      regist.OpenKey('SOFTWARE\Borland\Delphi', false);
      regist.GetKeyNames(pub);
      regist.CloseKey;
      for pubi := 0 to pub.count - 1 do
      begin
        regist.OpenKeyReadOnly('SOFTWARE\Borland\Delphi\' + pub[pubi]);
        if regist.ValueExists('RootDir') then
        begin
          pubdir := regist.ReadString('RootDir');
          if pubdir[length(pubdir)] = '\' then
            pubdir := pubdir + 'Bin\'
          else
            if pubdir[length(pubdir)] = '/' then
              pubdir := pubdir + 'Bin/'
            else
              pubdir := pubdir + '\Bin\';
          if FileExists(pubdir + 'brc32.exe') then
            pubbrc32 := pubdir + 'brc32.exe';
          if FileExists(pubdir + 'brcc32.exe') then
            pubbrcc32 := pubdir + 'brcc32.exe';
          additem(pub[pubi], pubbrc32, pubbrcc32);
        end;
      end;
//    finally
//      showmessage('Borland\Delphi');
//    end;


//    try
      pub := TStringList.Create;
      pubdir := '';
      pubbrc32 := '';
      pubbrcc32 := '';
      pubi := 0;
      regist.OpenKey('SOFTWARE\CodeGear\BDS', false);
      regist.GetKeyNames(pub);
      regist.CloseKey;
      for pubi := 0 to pub.count - 1 do
      begin
        regist.OpenKeyReadOnly('SOFTWARE\CodeGear\BDS\' + pub[pubi]);
        if regist.ValueExists('RootDir') then
        begin
          pubdir := regist.ReadString('RootDir');
          if pubdir[length(pubdir)] = '\' then
            pubdir := pubdir + 'Bin\'
          else
            if pubdir[length(pubdir)] = '/' then
              pubdir := pubdir + 'Bin/'
            else
              pubdir := pubdir + '\Bin\';
          if FileExists(pubdir + 'brc32.exe') then
            pubbrc32 := pubdir + 'brc32.exe';
          if FileExists(pubdir + 'brcc32.exe') then
            pubbrcc32 := pubdir + 'brcc32.exe';
          additem(pub[pubi], pubbrc32, pubbrcc32);
        end;
      end;
//    finally
//      showmessage('CodeGear\BDS');
//    end;


//    try
      pub := TStringList.Create;
      pubdir := '';
      pubbrc32 := '';
      pubbrcc32 := '';
      pubi := 0;
      regist.OpenKey('SOFTWARE\Borland\BDS', false);
      regist.GetKeyNames(pub);
      regist.CloseKey;
      for pubi := 0 to pub.count - 1 do
      begin
        regist.OpenKeyReadOnly('SOFTWARE\Borland\BDS\' + pub[pubi]);
        if regist.ValueExists('RootDir') then
        begin
          pubdir := regist.ReadString('RootDir');
          if pubdir[length(pubdir)] = '\' then
            pubdir := pubdir + 'Bin\'
          else
            if pubdir[length(pubdir)] = '/' then
              pubdir := pubdir + 'Bin/'
            else
              pubdir := pubdir + '\Bin\';
          if FileExists(pubdir + 'brc32.exe') then
            pubbrc32 := pubdir + 'brc32.exe';
          if FileExists(pubdir + 'brcc32.exe') then
            pubbrcc32 := pubdir + 'brcc32.exe';
          additem(pub[pubi], pubbrc32, pubbrcc32);
        end;
      end;
//    finally
//      showmessage('Borland\BDS');
//    end;

  finally
    regist.free;
  end;
end;

nur der mitlere teil wird immernoch nicht ausgeführt. bzw. bringt kein ergebniss.

[Deltachaos]

hatt sich erledigt. nach

regist.OpenKeyReadOnly('SOFTWARE\Borland\BDS\' + pub[pubi]); wurde ein

regist.CloseKey; vergessen

[taaktaak]

Auch wenn es sich erledigt hat:

Es ist mühsam, den Code durchzuschauen. Im Buch "Clean code" wird dringend empfohlen, dass Funktionen nur 1-4 Zeilen enthalten sollen. Auch wenn ich das für übertrieben halte: Du solltest deinen Code möglichst in kleinere "Häppchen" unterteilen, dann wird sich ein Fehler viel schneller finden lassen!

PS: Wo wird/werden deine Stringlisten "pub" wieder freigegeben?

[Deltachaos]

nirgends
muss ich noch machen.

so ist er doch kürtzer:

Delphi-Quellcode:

procedure delphi_versionadd(ort: string);
var regist: TRegistry;
    pub: TStringList;
    pubdir, pubbrc32, pubbrcc32: string;
    pubi: integer;
begin
  regist := TRegistry.Create;
  try
    regist.RootKey := HKEY_LOCAL_MACHINE;
    pub := TStringList.Create;
    regist.OpenKey(ort, false);
    regist.GetKeyNames(pub);
    regist.CloseKey;
    if pub.count > 0 then
    begin
      for pubi := 0 to pub.count - 1 do
      begin
        regist.OpenKeyReadOnly(ort + '\' + pub[pubi]);
        if regist.ValueExists('RootDir') then
        begin
          pubdir := regist.ReadString('RootDir');
          regist.CloseKey;
          if pubdir[length(pubdir)] = '\' then
            pubdir := pubdir + 'Bin\'
          else
            if pubdir[length(pubdir)] = '/' then
              pubdir := pubdir + 'Bin/'
            else
              pubdir := pubdir + '\Bin\';
          if FileExists(pubdir + 'brc32.exe') then
            pubbrc32 := pubdir + 'brc32.exe';
          if FileExists(pubdir + 'brcc32.exe') then
            pubbrcc32 := pubdir + 'brcc32.exe';
          additem(pub[pubi], pubbrc32, pubbrcc32);
        end;
      end;
    end;
  finally
    regist.free;
    pub.Free;
  end;
end;

procedure delphi_versions;
begin
  delphi_versionadd('SOFTWARE\Borland\Delphi');
  delphi_versionadd('SOFTWARE\Borland\BDS');
  delphi_versionadd('SOFTWARE\CodeGear\BDS');
end;

[taaktaak]

Ja, schaut schon viel besser aus!
Wenn das Ganze jetzt z.B. noch zwei lokale Prozeduren bekommt, ist der Code schon "cleaner".
Mir ist noch aufgefallen: Was passiert, wenn weder 'brc32.exe' noch 'brcc32.exe' gefunden werden? Immerhin verwendest du in beiden Fällen ein if... - aber auch wenn in beiden Fällen nichts gefunden würde, fügst du in die Liste (dann) leere Inhalte ein?! Was/wo wird mit "additem" eigentlich aufgerufen?

[Deltachaos]

ja dann wir ein leere inhalt hinzugefügt

ich bin gerade dabei das GUI für mein "RES Builder" zu machen.
ich hab den nochmal komplett neu angefangen.

Ich leg das mal im anhang an.
hat aber noch keine richtige Funktion. Es wird nur das Forumular aufgebaut und in einem Feld benutzerringaben geprüft.

PS: Ich habs noch kürzer gemacht. Mehr oder weniger^^

Delphi-Quellcode:

procedure delphi_versionadd(ort, key: string);
var regist: TRegistry;
    pub: TStringList;
    pubdir, pubbrc32, pubbrcc32: string;
    pubi: integer;
begin
  regist := TRegistry.Create;
  try
    regist.RootKey := HKEY_LOCAL_MACHINE;
    pub := TStringList.Create;
    regist.OpenKey(ort, false);
    regist.GetKeyNames(pub);
    regist.CloseKey;
    if pub.count > 0 then
    begin
      for pubi := 0 to pub.count - 1 do
      begin
        regist.OpenKeyReadOnly(ort + '\' + pub[pubi]);
        if regist.ValueExists(key) then
        begin
          pubdir := regist.ReadString(key);
          regist.CloseKey;
          pubdir := IncludeTrailingPathDelimiter(pubdir) + 'Bin\';
          if FileExists(pubdir + 'brc32.exe') then
            pubbrc32 := pubdir + 'brc32.exe';
          if FileExists(pubdir + 'brcc32.exe') then
            pubbrcc32 := pubdir + 'brcc32.exe';
          additem(pub[pubi], pubbrc32, pubbrcc32);
        end;
      end;
    end;
  finally
    regist.free;
    pub.Free;
  end;
end;

procedure delphi_versions;
var i: integer;
begin
//L_NAME := lang.ReadString('lang', 'L_NAME', 'Deutsch');
  if settings.SectionExists('DelphiVersions') then
  begin
    i := 1;
    while settings.ValueExists('DelphiVersions', inttostr(i) + 'KEY') do
    begin
      delphi_versionadd(settings.ReadString('DelphiVersions', inttostr(i) + 'KEY', ''), settings.ReadString('DelphiVersions', inttostr(i) + 'DIR', 'RootDir'));
      i := i +1;
    end;
  end
  else
  begin
    delphi_versionadd('SOFTWARE\Borland\Delphi', 'RootDir');
    delphi_versionadd('SOFTWARE\Borland\BDS', 'RootDir');
    delphi_versionadd('SOFTWARE\CodeGear\BDS', 'RootDir');
  end;
end;