orocos-toolchain / rtt

Orocos Real-Time Toolkit
http://www.orocos.org
Other
72 stars 79 forks source link

property: remove copy() method and allow to construct or create a property with a custom data source #159

Closed meyerj closed 7 years ago

meyerj commented 8 years ago

Following my suggestion in #158, this patch removes the RTT::base::PropertyBase::copy() method and introduces

PropertyBag assignment is supposed to have linking semantics (changing the value of a property in the copy also changes the value in the original bag), which is important for property composition and decomposition.

The PropertyBase::copy() method introduced in 44b9970e9685b9b812ddce1e8cc8c9b400553d54 had this linking semantics and was used for PropertyBag assignment, but the term copy was actually always associated with deep-copy semantics in RTT, like in PropertyBase::copy(const PropertyBase *) or in DataSourceBase::copy().

This patch introduces an alternative signature for the create() method, which accepts a custom data source instead of creating a default-initialized ValueDataSource. PropertyBag assignment now calls this PropertyBase::create(const DataSourceBase::shared_ptr&) method with the DataSource of the original property to be linked.

This reverts commit 44b9970e9685b9b812ddce1e8cc8c9b400553d54.

smits commented 8 years ago

We might get in trouble if someone started to use the copy method in the meantime. But since it was broken anyway merging this is ok, they'll get an "in the face" suggestion to replace/remove that usage. It might be wise to mention this removal in the release notes.

lgtm

meyerj commented 8 years ago

Done: https://github.com/orocos-toolchain/orocos_toolchain/commit/196677ef8183045dcb6296c6605e873b5d1252fe

meyerj commented 8 years ago

Good to merge?