Search code examples
c++classc++11copy-constructorros

Define copy constructor when keyword "this" is used inside constructor


I am having difficulties while defining the copy constructor of my class TextListener. The class TextListener bind a method callback using this keyword. Please see the complete code below:

#include <iostream>
#include <ros/ros.h>
#include <std_msgs/String.h>

class TextListener {
 private:
  std::string _text;
  ros::Subscriber _subscriber;

 public:
  TextListener() {
    std::cout << "[" << this << "] deafult constructor called" << std::endl;
  }

  TextListener(const TextListener &other)
      : _subscriber(other._subscriber), _text(other._text) {
    std::cout << "[" << this << "] copy constructor called" << std::endl;
  }

  TextListener &operator=(const TextListener &other) {
    std::cout << "[" << this << "] copy assignment called" << std::endl;
    _subscriber = other._subscriber;
    _text = other._text;
    return *this;
  }

  TextListener(ros::NodeHandle &nh, const std::string &topicName) {
    std::cout << "[" << this << "] constructor called" << std::endl;
    _subscriber = nh.subscribe(topicName, 1, &TextListener::callback, this);
  }

  void callback(const std_msgs::String::ConstPtr &msg) { _text = msg->data; }

  std::string &getText() { return _text; }

  ~TextListener() {
    std::cout << "[" << this << "] destructor called" << std::endl;
  }
};

To test the above class, I created an instance of it, which works without any problem. However, when I create a new instance and assign this instance to the new instance, the new instance doesn't work. Below is the code snippet:

int main(int argc, char **argv) {
  ros::init(argc, argv, "tet_listener");

  ros::NodeHandle nh;
  std::string topicName = "chatter";
  TextListener listener(nh, topicName);
  TextListener copyListener = listener;

  ros::Rate loop_rate(1);
  while (ros::ok()) {
    ROS_INFO("I heard: [%s]", copyListener.getText().c_str());
    ros::spinOnce();
    loop_rate.sleep();
  }

  return 0;
}

The method getText() doesn't have any value. See below the output:

[0x7ffc5698a2b0] constructor called
[0x7ffc5698a2d0] copy constructor called
[ INFO] [1549031938.250136695]: I heard: []
[ INFO] [1549031939.250183378]: I heard: []
[ INFO] [1549031940.250170333]: I heard: []
[ INFO] [1549031941.250176834]: I heard: []
^C[0x7ffc5698a2d0] destructor called
[0x7ffc5698a2b0] destructor called

I guess that the copy constructor is missing something. How to define copy constructor when keyword "this" is used inside constructor?


Solution

  • You need two Subscribers, one that notifies the original TextListener object, and one the that notifies the copy TextListener

    You do actually have two Subscribers, but as the one in the copy TextListener is a copy of the one in the original, they both update the original TextListener's _text member.

    I can't see a better way than keeping a reference to the NodeHandle in the TextListener, and having the copy constructor re-subscribe to the NodeHandle instead of copying the original object's subscription (which is calling the original object's callback).