Feedback on a06d9eaf on dfki-rtt

This patch:


commit a06d9eaf33545c5888f9a8848bc88baf01b88f9e
Author: Sylvain Joyeux <sylvain [dot] joyeux [..] ...>
Date:   Fri Nov 12 23:25:29 2010 +0100

    core: define TaskContext::getProperty<T>

diff --git a/rtt/TaskContext.hpp b/rtt/TaskContext.hpp
index b1f53b2..871ee94 100644
--- a/rtt/TaskContext.hpp
+++ b/rtt/TaskContext.hpp
@@ -479,6 +479,19 @@ namespace RTT
         }
 
         /**
+         * Get a Property with name \a name.
+         *
+         * @param  name The name of the property to search for.
+         * @return The Property with this name, zero
+         *         if it does not exist
+         */
+        template<typename T>
+        Property<T>* getProperty(const std::string& name) const
+        {
+            return dynamic_cast<Property<T>*>(tcservice->getProperty(name));
+        }
+
+        /**
          * Returns the properties of this TaskContext as a PropertyBag.
          */
         PropertyBag* properties() { return this->provides()->properties(); }


Can't be accepted because getProperty() may not take a template parameter by
2.x naming conventions (see also: getPort() and getOperation() ).
The correct use of getProperty is:


Property<Foo> f = task->getProperty("foo");
if (f.ready() ) {...}


There is a function 'getPropertyType<T>' in the ConfigurationInterface which
does take a template parameter. Using that function should however not be
needed given the above usage pattern.

Peter

Ruben Smits's picture

Feedback on a06d9eaf on dfki-rtt

On Monday 15 November 2010 16:07:08 Peter Soetens wrote:
> This patch:
>
> &#10;&gt; commit a06d9eaf33545c5888f9a8848bc88baf01b88f9e&#10;&gt; Author: Sylvain Joyeux &lt;sylvain [dot] joyeux [..] ...&gt;&#10;&gt; Date:   Fri Nov 12 23:25:29 2010 +0100&#10;&gt; &#10;&gt;     core: define TaskContext::getProperty&lt;T&gt;&#10;&gt; &#10;&gt; diff --git a/rtt/TaskContext.hpp b/rtt/TaskContext.hpp&#10;&gt; index b1f53b2..871ee94 100644&#10;&gt; --- a/rtt/TaskContext.hpp&#10;&gt; +++ b/rtt/TaskContext.hpp&#10;&gt; @@ -479,6 +479,19 @@ namespace RTT&#10;&gt;          }&#10;&gt; &#10;&gt;          /**&#10;&gt; +         * Get a Property with name \a name.&#10;&gt; +         *&#10;&gt; +         * @param  name The name of the property to search for.&#10;&gt; +         * @return The Property with this name, zero&#10;&gt; +         *         if it does not exist&#10;&gt; +         */&#10;&gt; +        template&lt;typename T&gt;&#10;&gt; +        Property&lt;T&gt;* getProperty(const std::string&amp; name) const&#10;&gt; +        {&#10;&gt; +            return&#10;&gt; dynamic_cast&lt;Property&lt;T&gt;*&gt;(tcservice-&gt;getProperty(name)); +        }&#10;&gt; +&#10;&gt; +        /**&#10;&gt;           * Returns the properties of this TaskContext as a PropertyBag.&#10;&gt;           */&#10;&gt;          PropertyBag* properties() { return this-&gt;provides()-&gt;properties();&#10;&gt; }
>
> Can't be accepted because getProperty() may not take a template parameter
> by 2.x naming conventions (see also: getPort() and getOperation() ).
> The correct use of getProperty is:
>
> &#10;&gt; Property&lt;Foo&gt; f = task-&gt;getProperty(&quot;foo&quot;);&#10;&gt; if (f.ready() ) {...}&#10;&gt;

Does this work for any Foo class type or only for the ones registered to the
typesystem, or only for the basic types?

Ruben

> There is a function 'getPropertyType<T>' in the ConfigurationInterface
> which does take a template parameter. Using that function should however
> not be needed given the above usage pattern.
>
> Peter

Feedback on a06d9eaf on dfki-rtt

On 11/15/2010 04:07 PM, Peter Soetens wrote:
> This patch:
>
> &#10;&gt; commit a06d9eaf33545c5888f9a8848bc88baf01b88f9e&#10;&gt; Author: Sylvain Joyeux&lt;sylvain [dot] joyeux [..] ...&gt;&#10;&gt; Date:   Fri Nov 12 23:25:29 2010 +0100&#10;&gt;&#10;&gt;      core: define TaskContext::getProperty&lt;T&gt;&#10;&gt;&#10;&gt; diff --git a/rtt/TaskContext.hpp b/rtt/TaskContext.hpp&#10;&gt; index b1f53b2..871ee94 100644&#10;&gt; --- a/rtt/TaskContext.hpp&#10;&gt; +++ b/rtt/TaskContext.hpp&#10;&gt; @@ -479,6 +479,19 @@ namespace RTT&#10;&gt;           }&#10;&gt;&#10;&gt;           /**&#10;&gt; +         * Get a Property with name \a name.&#10;&gt; +         *&#10;&gt; +         * @param  name The name of the property to search for.&#10;&gt; +         * @return The Property with this name, zero&#10;&gt; +         *         if it does not exist&#10;&gt; +         */&#10;&gt; +        template&lt;typename T&gt;&#10;&gt; +        Property&lt;T&gt;* getProperty(const std::string&amp;  name) const&#10;&gt; +        {&#10;&gt; +            return dynamic_cast&lt;Property&lt;T&gt;*&gt;(tcservice-&gt;getProperty(name));&#10;&gt; +        }&#10;&gt; +&#10;&gt; +        /**&#10;&gt;            * Returns the properties of this TaskContext as a PropertyBag.&#10;&gt;            */&#10;&gt;           PropertyBag* properties() { return this-&gt;provides()-&gt;properties(); }&#10;&gt;
>
> Can't be accepted because getProperty() may not take a template parameter by
> 2.x naming conventions (see also: getPort() and getOperation() ).
> The correct use of getProperty is:
>
> &#10;&gt; Property&lt;Foo&gt;  f = task-&gt;getProperty(&quot;foo&quot;);&#10;&gt; if (f.ready() ) {...}&#10;&gt;
>
> There is a function 'getPropertyType<T>' in the ConfigurationInterface which
> does take a template parameter. Using that function should however not be
> needed given the above usage pattern.
Ah. That's a quite uncommon usage pattern (doing automatic convertion at
assignation time), so I did not think about it.

If it is fine with you, I'll revert it with the following explanation
added in the commit message. I'd like to avoid rebasing.

Feedback on a06d9eaf on dfki-rtt

On Monday 15 November 2010 16:20:32 Sylvain Joyeux wrote:
> On 11/15/2010 04:07 PM, Peter Soetens wrote:
> > This patch:
> >
> > &#10;&gt; &gt; commit a06d9eaf33545c5888f9a8848bc88baf01b88f9e&#10;&gt; &gt; Author: Sylvain Joyeux&lt;sylvain [dot] joyeux [..] ...&gt;&#10;&gt; &gt; Date:   Fri Nov 12 23:25:29 2010 +0100&#10;&gt; &gt; &#10;&gt; &gt;      core: define TaskContext::getProperty&lt;T&gt;&#10;&gt; &gt; &#10;&gt; &gt; diff --git a/rtt/TaskContext.hpp b/rtt/TaskContext.hpp&#10;&gt; &gt; index b1f53b2..871ee94 100644&#10;&gt; &gt; --- a/rtt/TaskContext.hpp&#10;&gt; &gt; +++ b/rtt/TaskContext.hpp&#10;&gt; &gt; @@ -479,6 +479,19 @@ namespace RTT&#10;&gt; &gt; &#10;&gt; &gt;           }&#10;&gt; &gt;           &#10;&gt; &gt;           /**&#10;&gt; &gt; &#10;&gt; &gt; +         * Get a Property with name \a name.&#10;&gt; &gt; +         *&#10;&gt; &gt; +         * @param  name The name of the property to search for.&#10;&gt; &gt; +         * @return The Property with this name, zero&#10;&gt; &gt; +         *         if it does not exist&#10;&gt; &gt; +         */&#10;&gt; &gt; +        template&lt;typename T&gt;&#10;&gt; &gt; +        Property&lt;T&gt;* getProperty(const std::string&amp;  name) const&#10;&gt; &gt; +        {&#10;&gt; &gt; +            return&#10;&gt; &gt; dynamic_cast&lt;Property&lt;T&gt;*&gt;(tcservice-&gt;getProperty(name)); +        }&#10;&gt; &gt; +&#10;&gt; &gt; +        /**&#10;&gt; &gt; &#10;&gt; &gt;            * Returns the properties of this TaskContext as a PropertyBag.&#10;&gt; &gt;            */&#10;&gt; &gt;           &#10;&gt; &gt;           PropertyBag* properties() { return&#10;&gt; &gt;           this-&gt;provides()-&gt;properties(); }&#10;&gt; &gt; &#10;&gt; &gt;
> >
> > Can't be accepted because getProperty() may not take a template parameter
> > by 2.x naming conventions (see also: getPort() and getOperation() ). The
> > correct use of getProperty is:
> >
> > &#10;&gt; &gt; Property&lt;Foo&gt;  f = task-&gt;getProperty(&quot;foo&quot;);&#10;&gt; &gt; if (f.ready() ) {...}&#10;&gt; &gt;
> >
> > There is a function 'getPropertyType<T>' in the ConfigurationInterface
> > which does take a template parameter. Using that function should however
> > not be needed given the above usage pattern.
>
> Ah. That's a quite uncommon usage pattern (doing automatic convertion at
> assignation time), so I did not think about it.
>
> If it is fine with you, I'll revert it with the following explanation
> added in the commit message. I'd like to avoid rebasing.

No problem for me.

Peter