yarp icon indicating copy to clipboard operation
yarp copied to clipboard

Should Subscriber::read() be const?

Open randaz81 opened this issue 4 years ago • 1 comments

In my application, I cannot call yarp::os::Subscriber::read() method inside a const method, e.g:

class test
{
    public:
    yarp::os::Subscriber<mytype> sub;   //I need to mark sub as mutable
    void getter(mytype& data) const
    {
       data = *sub.read(true);
    }
}

I am wondering if yarp::os::Subscriber::read() and similar yarp::os::Port methods should be const or not.

randaz81 avatar Aug 25 '20 16:08 randaz81

I agree, the UnbufferedContactable methods should not modify the internal state, but just the state of the Portable that is passed.

Therefore I would suggest to change these 3 methods

diff --git a/src/libYARP_os/src/yarp/os/UnbufferedContactable.h b/src/libYARP_os/src/yarp/os/UnbufferedContactable.h
index e28792c5f..3af6e2aec 100644
--- a/src/libYARP_os/src/yarp/os/UnbufferedContactable.h
+++ b/src/libYARP_os/src/yarp/os/UnbufferedContactable.h
@@ -56,7 +56,7 @@ public:
      * @param willReply you must set this to true if you intend to call reply()
      * @return true iff the object is successfully read
      */
-    virtual bool read(PortReader& reader, bool willReply = false) = 0;
+    virtual bool read(PortReader& reader, bool willReply = false) const = 0;
 
     /**
      * Send an object as a reply to an object read from the port.
@@ -68,7 +68,7 @@ public:
      *               network connection - see for example Bottle
      * @return true iff the object is successfully written
      */
-    virtual bool reply(PortWriter& writer) = 0;
+    virtual bool reply(PortWriter& writer) const = 0;
 
     /**
      * Same as reply(), but closes connection after reply.
@@ -80,7 +80,7 @@ public:
      *               network connection - see for example Bottle
      * @return true iff the object is successfully written
      */
-    virtual bool replyAndDrop(PortWriter& writer) = 0;
+    virtual bool replyAndDrop(PortWriter& writer) const = 0;

I made a preliminary test and it seems to require to change only the signature, but I need further testing. Also this is technically a breaking change, since it changes some interfaces, even though I don't think that, outside YARP, anyone is actually deriving from port and overriding these method

drdanz avatar Aug 27 '20 08:08 drdanz