Was haltet ihr von diesem Code?

[freimatz]

Informatiker-Antwort: man schon, ein guter Entwickler nicht.

Und wenn wir schon dabei sind:
1. TDictionary<Integer, AnsiString>
a) 3x, dann besser Typ definieren. Am Besten mit einem Namen so dass man versteht was das soll.
b) AnsiString: macht man das heuzutage noch?
2. Methode zu lang
3. "Was hatet ihr von diesem Code?" Also haten muss man den Code nicht, es gibt wesentlich schlimmeres

[Olli73]

Zitat von freimatz:

2. Methode zu lang

Also bei Methoden, die weniger als ein paar Tausend Zeilen haben, rege ich mich erst gar nicht mehr auf ...

Ist halt teilweise uralter Code von Anderen, ich muss halt damit zurechtkommen. Das Problem hier war, dass der RangeCheck (im Release war der bei dem Projekt deaktiviert) zugeschlagen hat und ich der Meinung bin, Pointer und GetMem sollte man in solchen Fällen vermeiden, sonst kann man gleich C++ machen.

[himitsu]

Seit ein paar Delphi-Versionen ist die Bereichsprüfung standardmäßig aktiv.
[0..0] oder [1..1] gibt es öfters, aber bei größerem Index knallen diese Prüfungen.

Tipp: [1 .. MaxInt div SizeOf(MIB_IPADDRROW)] of MIB_IPADDRROW;
Alternativ die Pointer-Arithmetic aktivieren
und anstatt PMIBIpAddrTable = ^TMIBIpAddrTable;
ein PMIBIpAddr = ^MIB_IPADDRROW; , welches sich auch via .table[k] zugreifen lässt.

Achtung, da beginnt es mit Index 0, also so als wäre es [0..x]

Und wenn 0, dann beim for k := 0 to pIpTable^.dwNumEntries - 1 do aufpassen, da dwNumEntries ein DWORD ist und es somit Probleme gibt, wenn dwNumEntries 0 wäre, da bei -1 dann 4 Millarden, anstatt -1 rauskommt.
OK, zum Glück ist heutzutage standfardmäßig die Überlaufprüfung aktiv und würde bei 0 - 1 schön knallen.

Also besser to Integer(pIpTable^.dwNumEntries) - 1 do (größer als 2 Milliarden wird dwNumEntries hoffentlich nie sein)
oder vorher ein if pIpTable^.dwNumEntries > 0 then for ... .

[Blup]

Speicher muss man erst für den zweiten Aufruf der Funktion reservieren:

Delphi-Quellcode:

...
      ipTableSize := 0;
      pIpTable := nil;
      if GetIpAddrTable(pIpTable, ipTableSize, False) = ERROR_INSUFFICIENT_BUFFER then
      begin
        GetMem(pIpTable, ipTableSize);
        try
          if GetIpAddrTable(pIpTable, ipTableSize, False) = NO_ERROR then
          begin
...
          end;
        finally
          FreeMem(pIpTable);
        end;
     end;
...

[Olli73]

Da das nur ein Pointer ist, der noch nicht auf einen Record zeigt, muss man doch Speicher reservieren!?

[himitsu]

Es kommt drauf an.

Schau in die Hilfe.
GetIpAddrTable

Es gibt API,
* die wollen vom Nutzer Speicher haben
* Vielen wollen Speicher bekommen, aber ihnen reicht z.B. eine Record-Variable
* * hier würde sie auch reichen, wenn es nur eine IP gibt (oder wenn man sich selbst den Typen anpasst und z.B. [0..9] für maximal 10 IPs)
* Andere geben einen Zeiger auf internen Speicher raus
* oder sie erzeugen selbst den Speicher, welchen man dann ebenfalls freigeben muß

Zitat:

Delphi-Quellcode:

type
  MIB_IPADDRROW = ...

Warum?

Du verwendest eine API, welche nicht nur einen billigen Pointer als Typ hat, und da ist es natürlich einfacher/besser, auch diesen Typen dann zu verwenden.
Sonst kann es auch schnell passieren, dass der Compiler abraucht, wenn es ihm nicht gefällt, dass es "unterschiedliche" Typen sind.

Winapi.IpHlpApi
Winapi.IpRtrMib

PS: Windows.NetworkManagement.IpHelper aus'm GetIt WinMD
https://www.delphipraxis.net/214473-...vor-winmd.html

[Olli73]

Ja, aber in dem Fall will die API doch reservierten Speicher, und ein Pointer irgendwohin, schreibt halt auch "irgendwohin" oder crasht.

Außerdem ist der Code ja nicht von mir. Mir gefällt ja auch nicht, dass ich die Bereichsüberprüfung lokal abschalten musste.