On Wednesday 24 September 2008 10:29:34 Vandenbroucke Sander wrote:
> Hello,
>
> I ran into a little problem concerning the create/get/set of
> DataSources.
>
> I have a class holding some data that is real-time once it is
> contructed, it needs to allocate some memory. After construction access
> to it (using a operation=) does not allocate anything.
>
> Now put this into a property things go wrong... Take a look at the
> following code:
> template
> class Property
> {
> ...
> bool refresh( const Property& orig)
> {
> if ( !ready() )
> return false;
> *this = orig.getAssignableDataSource()->value();
> return true;
> }
> ...
> };
>
> The line
> *this = orig.getAssignableDataSource()->value();
> Results in code using the copy constructor of my class (which allocates
> memory). Not good in real-time...
>
> Using
> *this = orig.getAssignableDataSource()->rvalue();
> Fixes this.

Or even shorter:

$ svn di src/Property.hpp
Index: src/Property.hpp
===================================================================
--- src/Property.hpp (revision 29606)
+++ src/Property.hpp (working copy)
@@ -324,7 +324,7 @@
return false;
_description = orig.getDescription();
_name = orig.getName();
- *this = orig.get();
+ *this = orig.rvalue();
return true;
}

@@ -338,7 +338,7 @@
return false;
if ( _description.empty() )
_description = orig.getDescription();
- *this = orig.get();
+ *this = orig.rvalue();
return true;
}

@@ -350,7 +350,7 @@
{
if ( !ready() )
return false;
- *this = orig.getAssignableDataSource()->value();
+ *this = orig.rvalue();
return true;
}

$ svn ci src/Property.hpp -m"Fix real-time breaking bug in Property writing
(Fix Proposed by S. Vandenbroucke)."
Sending src/Property.hpp
Transmitting file data .
Committed revision 29619.

Peter

On Wednesday 24 September 2008 10:29:34 Vandenbroucke Sander wrote:
> Hello,
>
> I ran into a little problem concerning the create/get/set of
> DataSources.
>
> I have a class holding some data that is real-time once it is
> contructed, it needs to allocate some memory. After construction access
> to it (using a operation=) does not allocate anything.
>
> Now put this into a property things go wrong... Take a look at the
> following code:
> template
> class Property
> {
> ...
> bool refresh( const Property& orig)
> {
> if ( !ready() )
> return false;
> *this = orig.getAssignableDataSource()->value();
> return true;
> }
> ...
> };
>
> The line
> *this = orig.getAssignableDataSource()->value();
> Results in code using the copy constructor of my class (which allocates
> memory). Not good in real-time...
>
> Using
> *this = orig.getAssignableDataSource()->rvalue();
> Fixes this.

So it should be fixed.

>
> There are also a lot of different get/set functions defined in the
> interface (from Property.hpp):
> /**
> * Get a copy of the value of the property.
> * @return A copy of the value of the property.
> */
> operator value_t() const
> {
> return _value->get();
> }
>
> /**
> * Get a copy of the value of the property.
> * @return The value of the property.
> */
> DataSourceType get() const
> {
> return _value->get();
> }
>
> /**
> * Access to the value of the Property. Identical to value().
> * @warning This function is not suitable
> * for remote (distributed) access of properties,
> * use operator=() or set( param_t v ) to assign a value.
> */
> reference_t set()
> {
> return _value->set();
> }
>
> /**
> * Set the value of the Property.
> */
> void set(param_t v)
> {
> _value->set(v);
> }
>
> /**
> * Access to the value of the Property. Identical to set().
> * @warning This function is not suitable
> * for remote (distributed) access of properties,
> * use operator=() or set( param_t v ) to assign a value.
> *
> */
> reference_t value()
> {
> return set();
> }
>
> /**
> * Read-only (const&) access to the value of the Property.
> */
> const_reference_t rvalue() const
> {
> return _value->rvalue();
> }
>
> Some of these can give similar problems and create overhead in the form
> of temporary objects.
> Why not define only:
>
> param_t get() const
> {
> return _value->get();
> }
>
> void set(param_t v)
> {
> _value->set(v);
> }
>
> This will use 'const T&' as type if appropriate.
>
> I have seen this on RTT::Property but is goes up to RTT::DataSource.

We can't return a const& in get() because it may be returning a temporary
(especially in DataSource). For example if _value->get() would be implemented
as: return a->get() * b->get().
For properties, this is not the case and a const& is practically always valid
(although not enforced by the RTT). For this reason, value() was added which
is better suited for properties, because it returns a & and rvalue a const&.

So in practice, they are used as in:
Property p("p","");
p.value() = 3.0; // OR: p = 3.0
cout << p.value() < // OR: cout << double(p) <

set() and get() seem superfluous here but are on the other hand return 'always
safe' copies (but may ironically use mallocs).

Although I understand your concern, the only interface duplicacy is between
reference_t set(void)/value(void) and the operator value_t()/get() and the
operator=(param_t)/set(param_t) pairs.

In this perspective I would only consider dropping the above three set and get
versions but I wonder if it's worth the trouble.

Another aspect I just recall. When copying is an issue (ie should not be
done), the current RTT types use the 'const&' type, such as for
vector. This causes the offending line:
*this = orig.getAssignableDataSource()->value()
To return a const&. Yes indeed. (The line needs fixing anyway of course!)

I'm not saying your proposals are wrong, It's just that for everything, there
is a reason, especially when considering the real-time constraints.

So thanks anyway for finding this one,

Peter