Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Adding the same Object twice to a TObjectDictionary frees the object

Look at this code:

dic:=TObjectDictionary<Integer, TObject>.Create([doOwnsValues]);
testObject:=TObject.Create;
dic.AddOrSetValue(1,testObject);
dic.AddOrSetValue(1,testObject);

The code

  1. Creates a Dictionary that owns the contained values
  2. Adds a value
  3. Adds the same value again, using the same key

The surprising thing is that the object is freed when you add it the second time.

Is this intended behaviour? Or a bug in the Delphi libraries?

The documentation simply says "If the object is owned, when the entry is removed from the dictionary, the key and/or value is freed". So it seems a little odd to Free an object that I have just asked it to Add!

Is there any way to tell the TObjectDictionary to not do this? Currently, each time I add a value I have to check first if that Key-Value combination is already in the Dictionary.

Delphi 2010

[EDIT: After reading all the comments:

My conclusions (for what they are worth)]

  • This seems to be the intended behaviour
  • There is no way of modifying this behaviour
  • Don't use TObjectDictionary (or any of the other similar classes) for anything other than the common "Add these objects to the container. Leave them there. Do some stuff. Free the container and all the objects you added" usage. If you are doing anything more complicated, it's better to manage the objects yourself.
  • The behaviour is poorly documented and you should read the source if you want to really know what's going on

[/EDIT]

like image 751
awmross Avatar asked Aug 02 '11 01:08

awmross


2 Answers

TObjectDictionary<TKey,TValue> is in fact just a TDictionary<TKey,TValue> which has some extra code in the KeyNotify and ValueNotify methods:

procedure TObjectDictionary<TKey,TValue>.ValueNotify(const Value: TValue; 
  Action: TCollectionNotification);
begin
  inherited;
  if (Action = cnRemoved) and (doOwnsValues in FOwnerships) then
    PObject(@Value)^.Free;
end;

This is, IMO, a rather simple minded approach, but in the ValueNotify method, it is impossible to tell for which key this is, so it simply frees the "old" value (there is no way to check if this value is set for the same key).

You can either write your own class (which is not trivial), deriving from TDictionary<K,V>, or simply not use doOwnsValues. You can also write a simple wrapper, e.g. TValueOwningDictionary<K,V> that uses TDictionary<K,V> to do the brunt of the work, but handles the ownership issues itself. I guess I would do the latter.

like image 145
Rudy Velthuis Avatar answered Nov 15 '22 10:11

Rudy Velthuis


Thats because with reusing the key youre replacing the object and since the dictionary owns the object it frees the old one. Dictionary doesn't compare the value, only key, so it doesn't detect that the value (object) is same. Not a bug, as designed (IOW user error).

On second thought - perhaps the designer of the dict should have taken more care to have both doOwnsValues and AddOrSetValue()... one can argue both ways... I suggest you file it in QC, but I wouldn't hold my breath - it has been so now in at least two releases so it's unlikely to change.

like image 7
ain Avatar answered Nov 15 '22 08:11

ain