Funktion kommt beim sortieren des Array durcheinander

[Popov]

Ich verwalte paar Infos über ein Record-Array. Das einzig besondere dabei ist die Bitmap. Da ich vorher nicht weiß wie viele Einträge ich benötige, wird die Array dynamisch verwaltet, d. h. kommt ein neuer Eintrag dazu, wird er hinzugefügt, wird ein Eintrag nicht mehr benötigt, bekommt er zuerst einen Delete Vermerkt. Erst auf Anweisung wird das Array dann bereinigt. Dazu habe ich die untere Funktion.

Unabhängig davon ob die Funktion effektiv ist, bringt sie gelegentlich, nicht immer, einiges durcheinander. Nur erkenne ich kein Muster. Irgendwann sind einige Infos durcheinander, aber nie alles. Wo liegt der Fehler?

Delphi-Quellcode:

type
  TInfo = record
    Textinfo: Integer;
    Bmp: TBitmap;
    Delete: Boolean;
  end;

var
  Info: array of TInfo;

procedure ClearArray;
var
  i, k: Integer;
begin
  for i := High(Info) downto 0 do
  begin
    if Info[i].Delete then
    begin
      if i = High(Info) then //Ausnahme für High(Info)
      begin
        Info[High(Info)].Bmp.Free;
        SetLength(Info, Length(Info) - 1);
        Continue;
      end;

      for k := i to High(Info) - 1 do //Rest
      begin
        Info[k].Textinfo := Info[k+1].Textinfo;
        Info[k].Bmp.Assign(Info[k+1].bmp);
        Info[k].Delete := Info[k+1].Delete;
      end;
      Info[High(Info)].Bmp.Free;
      SetLength(Info, Length(Info) - 1);
    end;
  end;
end;

[shmia]

Die Art und Weise, wie du den Record kopierst ist "ungünstig".
Besser man vermeidet permanente Zuweisungen mit Assign:

Delphi-Quellcode:

procedure MoveInfoRecord(var src, dst:TInfo);
begin
  dst.Textinfo := src.Textinfo;
  dst.Bmp      := src.bmp);  // nur den Objektzeiger kopieren
  dst.Delete   := src.Delete;
end;


procedure ClearArray;
var
  i, k: Integer;
begin
  for i := High(Info) downto 0 do
  begin
    if Info[i].Delete then
    begin
      // zuerst Bitmap freigeben
      Info[i].Bmp.Free;
      // der Platz i ist jetzt quasi ungültig

      // jetzt die gültigen Daten "runterschieben" 
      for k := i to High(Info) - 1 do //Rest
      begin
        MoveInfoRecord(Info[k+1], Info[k]);
      end;
      // Array um 1 verkürzen
      SetLength(Info, Length(Info) - 1);
    end;
  end;
end;

[Popov]

Zitat von shmia:

Die Art und Weise, wie du den Record kopierst ist "ungünstig".
Besser man vermeidet permanente Zuweisungen mit Assign:

Ja, das habe ich auch so gesehen, deshalb meinte ich auch, dass sie ineffektiv ist (irgendwie ist es das erste Mal, dass ich Objekte über Array sortiere). Mein erster Gedanke war auch die Zuweisung über Adressen, nur hatte ich einen Gedankenfehler, dass wenn ich alles runterkopiere und dann zuletzt die Adresse freigebe... Und das war der Dedankenfehler, die Reihenfolge. Also vielen Dank für den Tipp.

Ansonsten kommen die Daten immer noch durcheinander. Nur wenn das nicht hier ist, dann muß ich den Fehler wo anders suchen.

[himitsu]

[edit]
war falsch ... bis gleich

[shmia]

Zitat von Popov:

Nur wenn das nicht hier ist, dann muß ich den Fehler wo anders suchen.

Schreib Dir doch ein kleines Testprogramm, dass einige Einträge ins Array macht, dann ein paar Einträge zum Löschen markiert und zum Schluss wird ClearArray aufgerufen.

Du testet damit deinen eigenen Programmcode isoliert vom restlichen Programm.
Man nennt diese Vorgehensweise auch Unit-Testing.

[Bjoerk]

Das mit dem "Rest" hab' ich nicht verstanden, aber rauslöschen geht so (ungetestet)

Delphi-Quellcode:

procedure DelItem(const Index: integer);
var
  I: integer;
begin
  for I:= Index to Length(Info)-2 do
    Info[I]:= Info[I+1];
  Info[Length(Info)-1].Bmp.Free;
  SetLength(Info, Length(Info)-1);
end;

procedure ClearArray;
var
  I: Integer;
begin
  I:= 0;
  while I < Length(Info) do
  begin
    if Info[I].Delete then
    begin
      DelItem(I);
      Dec(I);
    end;
    Inc(I);
  end;
end;

end.

[himitsu]

Einfachen und sicheren Trick Tipp:

Leite dir eine neue Komponente von TBitmap ab und spendiere ihr ein neues Property "TextInfo",
oder erstell dir ein Container-Object für TextInfo und das Bitmap.

Nun alles in eine TObjektList mit OwnsObjects=True und fertig.

[Iwo Asnet]

Ich würde das eh anders machen (manchmal die einfachste Möglichkeit, ein Problem zu lösen)

Delphi-Quellcode:

i := Low(Info);
While i<=High(Info) do begin
  if Info[i].Deleted then begin
    MoveInfo(High(Info),i);
    setLength(Info, Length(Info) - 1);
  end; 
  inc(i);
end;

Also: Jedes 'Loch' wird einfach mit dem höchsten Element aufgefüllt und die Liste dann verkleinert.
Sofern Du keine sortierte Liste hast, geht das natürlich viel schneller.

Tipp: Verwende die 'Top-Down Programmierung mit Stepwise refinement' Das war vor 30 Jahren, als ich angefangen habe, state of the art. Und ist es immer noch, wenn ich an Refactoring denke und die Prämisse, Code lesbar und kompakt zu halten.

[Popov]

Zitat von shmia:

Schreib Dir doch ein kleines Testprogramm, ...

In der Regel mach ich das auch so, aber die eigentliche Array ist etwas komplexer, auch wenn der Kern des Problems sich auf das Beispiel reduzieren läßt. Wollte es mir sparen. Kommen wohl nicht drumherum.

@Bjoerk
Danke für das Beispiel. Auch eine Möglichkeit.

@himitsu
Eigene Klasse ist auch eine Möglichkeit, überlege ich auch schon die ganze Zeit. Aber das Programm ist einfach so mitgewachsen, bzw. ist erweitert worden. War am Anfang anders geplant. Aber wenn es nicht anders geht.

@Iwo Asnet
Danke für die Info, werde ich es mir überlegen.