Wrong behavior from OnChanged event of dictionary

[Merijn Bosma]

Please consider the following code:

‌

procedure TMainForm.FormCreate(Sender: TObject);
begin
 fDict := TCollections.CreateDictionary<string, boolean>();
 fDict.OnChanged.Add(OnChanged)
end;

procedure TMainForm.OnChanged(Sender: TObject; const p: TPair<string, boolean>; Action: TCollectionChangedAction);

 function Contents: string;
 begin
  result := '';

  for var i in fDict do
   result := result + sLineBreak + i.Key + ' = ' + BoolToStr(i.Value, true);
 end;

begin
 var a := GetEnumName(TypeInfo(TCollectionChangedAction), integer(Action));

 Log(Format('action: %s %s = %s', [a, p.Key, BoolToStr(p.Value, true)]));
 Log('Contents: ' + Contents());
end;

procedure TMainForm.Button1Click(Sender: TObject);
begin
 fDict['1'] := false;
 fDict['2'] := false;
end;

procedure TMainForm.Button2Click(Sender: TObject);
begin
 fDict['1'] := false;
end;

When pressing button1, this log appears (as expected):

‌

action: caAdded 1 = False
Contents:
1 = False
action: caAdded 2 = False
Contents:
1 = False
2 = False

‌

However, when (consecutively!) pressing button2, we see this (unexpected) log:

action: caRemoved 1 = False
Contents:
2 = False
action: caAdded 1 = False
Contents:
2 = False

There are two issues here:

1. there is never a changed event for the entry with '1' as a key, we only see a remove and an add
2. a bigger issue is though that the change event where the item is added again, the list is not complete. It seems like the add event is fired before the item is actually added to the list.

‌

[Stefan Glienke]

Your assumption about getting a changed notification is wrong - there is no such thing. Changing the value for an existing key is always a removed and added event. It is similar to replacing an item at a given index in a list which also triggers removed and added.

caChanged is reserved for observable lists where a property of some object in them was changed if that object implements INotifyPropertyChanged.

I will look into the second issue - this is the result of iterating the dictionary from inside the event handler which will not yield the currently in-change entry.

Additional information: iterating or checking Count during these change events is undefined behavior and might yield unexpected results - for example when replacing an item even though you get a removed event the Count never changes. This is especially important because Clear also triggers removed events but the collection internally might be in a state where the items exist and thus possibly yields them as well as reporting an unexpected Count.

[Merijn Bosma]

Hi Stefan,

Thanks for the insights.

Are you saying that I should never trust the content during a changed event?

The reason I got here is because I need to keep track of a bunch of named booleans. I get changes to these booleans through several async events, in these events I change the state for that specific item in the dictionary. I use the change event to check of all booleans have the desired state. I’ve worked around now by sending myself a windows message in the change event so I can check the contents later. What would be the best approach in your opinion using spring collections in this case?

[Stefan Glienke]

Since I know most of the implementation details from memory I say it depends.

One example: caRemoved is not only triggered when removing a single element but also when a range of elements is removed - for a list all the caRemoved events are triggered after the entire range was being removed and thus the count and content of the list for each of those events will be the content after the last removal. Similar is the case for a Clear event - although while investigating this issue I found a few inconsistencies. I eventually want at least the behavior to be consistent and documented but right now I would not rely on such consistency. As you see with this example the amount would be at least consistent (the Count would match the elements you get from iterating) but with a dictionary currently, the situation is a bit more complex.

I cannot give you a quick solution with how the dictionary behaves right now and also no quick fix where I know that it won’t break anything (otherwise I would already have committed one).

But I will remember your use-case when working on this.

‌

[Stefan Glienke]

fixed #413

→ <<cset 4ee4360decd7>>

[Stefan Glienke]

The OnChange event now behaves similarly to how OnKeyNotify and OnValueNotify behave in the RTL with the small exception that when changing the value for an existing key the value is still the old one when caRemoved gets triggered - the RTL triggers both after the change.

Also when iterating the dictionary during events from Clear it will show as empty.