Many unnecessary operations in ISet collection
Issue #343
resolved
The ISet collection does a lot of unnecessary operations, I discovered it because my code is based on the log of these changes and it stopped working properly. Here is an minimal example:
program Intersect;
{$APPTYPE CONSOLE}
{$R *.res}
uses
Spring,
Spring.Collections,
Spring.Collections.Sets,
System.SysUtils;
type
TTest = class
public
procedure OnChanged(sender: TObject; const item: Integer; action: TCollectionChangedAction);
procedure Test();
end;
procedure TTest.Test();
var
lFirst, lSecond : ISet<Integer>;
begin
lFirst := TCollections.CreateSet<Integer>([1, 2, 3]); //<-
lFirst.OnChanged.Add(OnChanged);
lSecond := TCollections.CreateSet<Integer>([1, 2, 3]); //<- the same like previous
lFirst.IntersectWith(lSecond);
end;
procedure TTest.OnChanged(sender: TObject; const item: Integer; action: TCollectionChangedAction);
begin
Writeln('action: ', TEnum.GetName<TCollectionChangedAction>(action), '; value=', item);
end;
var
Test : TTest;
begin
try
Test := TTest.Create;
Test.Test();
Test.Free;
Readln;
except
on E: Exception do
Writeln(E.ClassName, ': ', E.Message);
end;
end.
After execute this code I get in console:
action: caRemoved; value=1
action: caRemoved; value=2
action: caRemoved; value=3
action: caAdded; value=1
action: caAdded; value=2
action: caAdded; value=3
action: caRemoved; value=1
action: caRemoved; value=1
action: caRemoved; value=1
Moreover... Did you not mistake Self and Sender in this definition? ;-)
TCollectionBase<T> = class abstract(TEnumerableBase<T>)
private type
TNotify = procedure(Self: TObject; const item: T; action: TCollectionChangedAction);
private
Comments (2)
-
repo owner -
repo owner - changed status to resolved
fixed
#343→ <<cset b6a87250a8b2>>
- Log in to comment
This is indeed a regression and that change I did there is embarassingly ridiculous I admit.
My intention was to avoid the creation of a list but I also changed the algorithm in a bad way.
I will fix this and add a test.
As for the naming of the private TNotify - that is correct as this is just to eliminate the virtual call to Changed when it is not overridden and there is no event handler because then the fNotify is nil. Self becomes Sender when it is being passed to fOnChanged.Invoke. Declaring TNotify like that and not as
procedure(const item: T; action: TCollectionChangedAction) of object;
avoids having to store SizeOf(Pointer)*2 (which also affects the aligning of the class) but just SizeOf(Pointer) because Data is always Self anyway.