[gelöst] (ObjectList) Sort -> AccessViolation

[silver-moon-2000]

Hallo zusammen,

vielleicht sehe ich den Wald vor lauter Bäumen nicht (mehr), aber kann mir jemand von Euch erklären, warum das folgende "nicht funktioniert".
Dabei handelt es sich um die "Callback Funktion" der ObjectList.Sort Methode, wird also so aufgerufen OL.Sort(@CompareAudioAtEnd) Ziel ist es, diese MMItems so zu sortieren, dass diejenigen mit der Eigenschaft "isAudioOnly" ans Ende kommen, und beide Teile (isAudioOnly und not isAudioOnly) alphabetisch sortiert werden

Delphi-Quellcode:

  
function CompareAudioAtEnd(input1, input2: Pointer): Integer;
  var b1, b2 : Boolean;
  begin
    b1 := TMMItem(input1).isAudioOnly;
    b2 := TMMItem(input2).isAudioOnly;
    if (b1) and (b2) then
      Result := CompareText(TMMItem(input1).InputName, TMMItem(input2).InputName)
    else
      if b1 then
        Result := 1
      else
        Result := -1;
  end;

Funktioniert nicht bedeutet in diesem Fall: Der Debugger bleibt mit der Meldung "Stack Overflow" hängen und zeigt auf "begin" bzw. bricht das Programm einfach ohne Fehlermeldung ab ("Programm funktioniert nicht mehr") wenn ich es außerhalb der IDE starte.


Hingegen das Folgende funktioniert sehr wohl, ist meiner Meinung nach aber von Hinten durch die Brust ins Auge.

Delphi-Quellcode:

  function CompareAudioAtEnd(input1, input2: Pointer): Integer;
  var t1, t2 : string;
  begin
    if TMMItem(input1).isAudioOnly then
      t1 := 'audio'
    else
      t1 := 'video';
    if TMMItem(input2).isAudioOnly then
      t2 := 'audio'
    else
      t2 := 'video';
    Result := CompareText(t2, t1);
    if Result = 0 then
      Result := CompareText(TMMItem(input1).InputName, TMMItem(input2).InputName);
  end;

Falls es hilft (gekürzt):

Delphi-Quellcode:

TMMItem = class(TObject)
  private
    FIsAudioOnly : Boolean;
  public 
    property isAudioOnly : Boolean read FIsAudioOnly;
end;


TMMList = class(TObject)
  private
    FItems       : TObjectList;
  public 
    procedure Sort;
end;

procedure TMMList.Sort;
  function CompareAudioAtEnd(input1, input2: Pointer): Integer;
  begin
    [...]
  end;
begin
  FItems.Sort(@CompareAudioAtEnd);
end;

Wie gesagt, mich wundert, dass die eine Funktion (per Stringvergleich) funktioniert, die andere (per Boolean-Vergleich) jedoch nicht?
Oder übersehe ich (wieder mal) etwas Kritisches?

[edit]
Die "richtige" Lösung wurde von jaenicke zwei Posts weiter unten gegeben
if (b1) and (b2) then muss heißen if not (b1 xor b2) then

[MGC]

Hallo Tobias,

ist sichergestellt das input1 und input2 nicht auf Zeichenketten weisen die nil sind? Ansonsten sollte vorher durch überprüfung und Zuweisung von '' sichergestellt sein das, sich zumindest statt nil Nichts im der zugrundeliegenden Variable befindet.
Bei der Variante CompareText(t2,t1) war zumindest sichergestellt das der Vergleich nicht mit Nil-Zeichenketten durchgeführt wurde.
Im oberen Bereich könnte es sein, das es egal war ob der Pointer auf nil zeigt, da dann einfach davon auszugehen war, das sich kein Audio in der Zeichenkette verbirgt.

Wie sind die Variablen deklariert auf die der Pointer zeigt?

[jaenicke]

Die beiden Varianten machen schlicht nicht das gleiche (insbesondere im Falle von isAudioOnly = False bei beiden Objekten).

Dein funktionierender Code macht das:

Delphi-Quellcode:

function CompareAudioAtEnd(input1, input2: Pointer): Integer;
var
  b1, b2: Boolean;
begin
  b1 := TMMItem(input1).isAudioOnly;
  b2 := TMMItem(input2).isAudioOnly;
  if not (b1 xor b2) then
    Result := CompareText(TMMItem(input1).InputName, TMMItem(input2).InputName)
  else if b1 then
    Result := 1
  else
    Result := -1;
end;

Wobei im Falle von unterschiedlichen Werten für isAudioOnly das Verhalten dennoch anders ist. Aber ich schätze mal dieses ist gewollt.

[himitsu]

Ich würde auchg mal die generische TObjektList in den Raum werfen.

weniger Rumgecaste = weniger Fehler

[rollstuhlfahrer]

Ich würde noch (auch wenn es noch so abstrus erscheint) folgende Abfrage einbauen:

Delphi-Quellcode:

if ((not (Item1 is TMMItem)) OR (not (Item2 is TMMItem))) then
  raise EAbort.Create('Hier stimmt was nicht');

Grund: Bei nicht generischen Listen ist es ganz einfach, mal ein falsches Objekt in die Liste zu bekommen und das birgt dann unbekannte Risiken, insbesondere eine ganze Menge an AVs, wenn man die Objekte wieder frei geben will und sie vorher schon weg sind.

Bernhard

[silver-moon-2000]

Hallo und Danke an alle für die Antworten. Ich werde mal versuchen, zu antworten.

Zitat von MGC:

ist sichergestellt das input1 und input2 nicht auf Zeichenketten weisen die nil sind

Ich denke schon, da ich einfach das Codebeispiel zu TList.Sort aus der Onlinehilfe abgekupfert habe. Außerdem funktioniert ja eines meiner Beispiele und bei dem funktionierenden wird ja auch auf (TMMItem(input1).isAudioOnly gecastet/getestet. Dort würde es ja auch knallen, wenn input1 = nil wäre.

Zitat von jaenicke:

Die beiden Varianten machen schlicht nicht das gleiche (insbesondere im Falle von isAudioOnly = False bei beiden Objekten).
if not (b1 xor b2) then

Autsch. Das hatte ich doch tatsächlich nicht bemerkt. Peinlich, peinlich. In meiner Variante rennt er sich tatsächlich zu Tode. Jetzt, wo Du es schreibst...
Dass sie ein etwas anderes Verhalten aufweisen, ist tatsächlich so geplant gewesen, trotzdem danke, dass Du mich darauf hinweist.

Zitat von himitsu:

Ich würde auchg mal die generische TObjektList in den Raum werfen.

Normalerweise habe ich mir bisher immer meine ObjectLists passend abgeleitet type TMMItemObjectList = class(TObjectList) , sodass ich mir das Casten sparen konnte.
Das habe ich mir diesmal, warum auch immer, gespart. Das ist aber auch das erste Mal, dass ich wirklich mit etwas neuerem als TurboDelphi programmiere. Ich habe bisher überhaupt nicht an Generics gedacht, aber das ist mal eine gute Idee.

Zitat von rollstuhlfahrer:

Ich würde noch (auch wenn es noch so abstrus erscheint) folgende Abfrage einbauen:

Das habe ich an dieser Stelle nicht, weil ich es jeder Stelle, an der die OL befüllt wird, getestet habe. Es darf also kein anderer Typ in der Liste auftauchen. Generische Listen aber machen, wie erwähnt, das Ganze noch ein Stückweit sicherer. Ich denke, ich werde darauf umsteigen.

Trotzdem: Wenn ich mir die Hilfe zu TList.Sort ansehe, muss die Callback-Routine vom Type TListSortCompare sein, und der ist definiert als TListSortCompare = function (Item1, Item2: Pointer): Integer; . Somit würde mit eine typisierte OL auch nicht helfen, da die Callback Routine sich ja nicht ändert (solange ich Sort nicht überschreibe)

Zusammenfassung: Ich habe tatsächlich wieder etwas Wichtiges übersehen. jaenickes Code-Änderung behebt mein Problem, und auch generische Listen umzusteigen ist auch eine gute Idee.
Nochmals danke an Alle