[Core] Adding merge to data value container #3134
Conversation
loumalouomega
changed the title
|
@RiccardoRossi The idea is that I want to have a general property lets say Property 0. Then, I have a particular Property 1 that only has some specific values. I want all the property values from Property 0 to be copied into Property 1 (except if this variable already exists, then it keeps its old values.) I tried to do this without doing any change in the core, but after discussing it with @pooyan-dadvand I did not find any other way to do it. |
|
I think that the decison of 'respecting the old value' or 'overwriting the old value' could be left to the end developer, in the form of a true/false flag or coding two different methods. |
|
my point however is: does this need to be a class method? |
|
@RiccardoRossi so.. what is your suggestion? mData is private so I do not know what else you could do... |
|
I would go for a class method as it only changes this class members with almost full cohesion. |
|
ok, about the method. I'll dismiss my review since it is no longer relevant |
|
@ddiezrod please add a test before merging this |
|
Ok, I will use a local flag as @RiccardoRossi and @maceligueta proposed. I will have to add then the corresponding .cpp file and the test for this method. |
|
test and flag added. Please review @RiccardoRossi @pooyan-dadvand @jrubiogonzalez |
|
@RiccardoRossi Can you approve now? |
|
Anyone? @RiccardoRossi @pooyan-dadvand ? |
| for (const_iterator i = rOther.mData.begin(); i != rOther.mData.end(); ++i) { | ||
| bool variable_already_exist = false; | ||
| for (iterator j = mData.begin(); j != mData.end(); ++j) { | ||
| if (i->first->Key() == j->first->Key()) { |
There was a problem hiding this comment.
why are u using the key here? i think you can use directly
i->first == j->first
|
|
||
| //Second case: do overwrite old values | ||
| options.Set(DataValueContainer::OVERWRITE_OLD_VALUES, true); | ||
| container_target.Merge(container_origin, options); |
There was a problem hiding this comment.
just as a comment, but i think you can write
container_target.Merge(container_origin, DataValueContainer::OVERWRITE_OLD_VALUES)
…container' into core/adding-merge-to-data-value-container
The idea is to use this method to "complete" a property from other property.