For Schleife zählt 0,2,3,4,5,...

OldMan00

Kennedy schrieb:

Das der Bug von mir verursacht wurde ist mir klar

ok, dachte nur Du meinst es wäre ein bug vom Programm und JA beschäftige Dich mal mit dem Debugger, ist sehr hilfreich und im Grunde genommen unerläßlich.

Was war denn nun der Fehler?

Geht aus dem Beitrag nicht hervor.

Potentiell ist so etwas aber gefährlich:

for (counter = 0; counter < 12; counter++) 
  Edit->Lines->Add (Script->Strings[counter]);

Hier wird nicht überprüft, ob in 'Script' überhaupt 12 Zeilen enthalten sind. Wenn nicht -> Index out of bounds... Man sollte immer überprüfen, ob eine Zeile tatsächlich vorhanden ist, bevor man auf sie zugreift. Das gilt für alle Listen.

Also lieber in der Art:

for (counter = 0; counter < Script->Count; counter++) 
  Edit->Lines->Add (Script->Strings[counter]);

Oder aber wenn nur eine Teilmenge verwendet werden soll:

for (counter = 0; counter < 12; counter++) 
{
  if (counter >= Script->Count - 1)
    break; // Abbrechen, wenn Counter großer als die Anzahl der Einträge in der Liste
  Edit->Lines->Add (Script->Strings[counter]);
}

audacia

Joe_M. schrieb:

Also lieber in der Art:

for (counter = 0; counter < Script->Count; counter++) 
  Edit->Lines->Add (Script->Strings[counter]);

Noch lieber so:

for (int counter = 0, count = Script->Count; counter != count; ++counter)
  ...

Das spart u.U. (Script->Count - 1) Aufrufe des Property-Getters.

witte

audacia schrieb:

Das spart u.U. (Script->Count - 1) Aufrufe des Property-Getters.

Dann sollten wir vielleicht erst mal rauskriegen, ob dieser Getter als O(n) oder als O(1) realisert ist.

audacia

witte schrieb:

Dann sollten wir vielleicht erst mal rauskriegen, ob dieser Getter als O(n) oder als O(1) realisert ist.

Warum? Selbst im letzten Fall sparst du ggf. jedesmal einen Funktionsaufruf.

witte

Was nützt Dir das Einsparen eines Aufrufes, wenn Script->Count als O(n) implementiert ist. Dann ist

const unsigned int count = Script->Count;
for (counter = 0; counter < count ; counter++)
  Edit->Lines->Add (Script->Strings[counter]);

erste Sparmaßnahme.

Braunstein

und wo ist jetzt dein Unterschied zu audacias Code? Mal abgesehen davon, dass count einmal in und einmal außerhalb der Schleife definiert wird.

witte

Stimmt, ich habe sein Beispiel unter "Noch lieber so: " übersehen.

audacia schrieb:

Noch lieber so:
for (int counter = 0, count = Script->Count; counter != count; ++counter)
  ...
Das spart u.U. (Script->Count - 1) Aufrufe des Property-Getters.

Es ging aber doch um die 'index out of bounds' Problematik. Dies habe ich versucht am gegebenen Beispiel zu erklären.

Eine Optimierung wollte ich gar nicht einbringen und ist - in diesem Fall - wohl auch ziemlich sinnlos, da die Schleife nur 12 Durchgänge hat, wird wohl auch kaum viel mehr in der StringList sein. Außerdem ist der Getter bei TStringList::Count recht fix. Bei weniger als 1000 Zeilen in der StringList wird man keinen Unterschied bemerken. Sogar bei einer Million Zeilen liegt der Unterschied bei wenigen Millisekunden. IMHO vernachlässigbar...

audacia

Joe_M. schrieb:

Es ging aber doch um die 'index out of bounds' Problematik. Dies habe ich versucht am gegebenen Beispiel zu erklären.

Das mag ja sein, aber deine Anregung ist hinreichend allgemein (sie trifft auf fast alle VCL-Container zu), daher dachte ich, es wäre ganz sinnvoll, sie gleich möglichst optimal umzusetzen. Analog vielleicht dazu, daß man im Gebrauch von for-Schleifen auch Anfängern eher zum Vergleich auf Ungleichheit anstatt auf Relation sowie zum Präfix-Inkrementoperator rät, damit der Ansatz auch funktioniert, wenn sie eines Tages mal Iteratoren benutzen.

Auch wieder wahr...