Introduction

I have two classes I call Node and Point here. Points are objects which hold specific information (which information exactly does not matter I use a simple integer in the example). Nodes are objects which manage several points if they should share the same information. So Node has a list of pointers to all the points it manages (which is fine because Pointremoves its reference from the node if its lifetime ends) and each point has a back reference to the node itself. This creates a circular reference which makes things a bit tricky. I do not use multithreading, so races of any kind are not a concern. Also I am bound to use C++03 (so C++98, basically) and boost 1.53 so there is a limitaion in features I can use.

EDIT:

as it seems to be a bit unclear i wanted to add that this is really a very minimalistic extraction of the actual code. As i wanted to provide a minimal working example i tried my best to extract the functionality from the original program. However be sure that using shared pointers is necessary in the actual program since these resources are shared across several objects with different classes. Also i use shared pointers because they are always optional which means you can check them for NULL and act differently if they are not assigned as can also be seen in the code below.

The problem

I have not yet described the "back reference to the node" any further because this is where the problem begins. So at the moment I settled with this solution:

My Node contains a weak pointer to itself so the ownership can be shared to other points. On adding a point, the node creates a shared pointer from the weak pointer like this

p->node = this->weakThis.lock();

but I do not know if this is just bad practice or contains serious problems which I do not see since the reference to weak_ptr::lock() only states that it creates a new shared object despite its confusing name, or if this is acceptable code.

Other solutions I have thought of

• All classes get shared pointers

  • good: No need to create shared objects from weak ones
  • bad: The shared object inside the class needed to be explicitely deleted (like with a call of a method for example Node::destroySelfReference() which can easily be forgotten. Otherwise the object could live forever with a reference to itself.

• All classes get weak pointers

  • good: we just need to copy the weak pointer and not share ownership
  • bad: everytime we wanted to access the node from the point we needed to call lock(). And since the reference in Point is not really used as observer but is used like a shared reference this is obsolete.

Question

What would be the best solution here? And which are the real dangers of them?

Code

/* Using boost 1.53 */
#include <vector>
#include <boost/shared_ptr.hpp>
#include <boost/make_shared.hpp>
#include <boost/weak_ptr.hpp>

using boost::shared_ptr;
using boost::make_shared;
using boost::weak_ptr;
using std::cout;
using std::endl;

/*-------------Classes-----------*/

class Point;

class Node {
private:
    std::vector<Point *> points;
    weak_ptr<Node> weakThis;
public:
    int value;
    Node(Point * p, shared_ptr<Node> & sp);
    void add(Point * p);
    void remove(Point * p);
    int getValue() {
        return this->value;
    }
};

class Point {
private:
    int value;
public:
    shared_ptr<Node> node;
    Point() : value(2) {}
    ~Point() {
        if(this->node != NULL) {
            this->node->remove(this);
        }
    }
    int getValue() {
        if (this->node == NULL) return this->value;
        else {
            return this->node->getValue();
        }
    }
};

/*----------Node definition------------*/

Node::Node(Point * p, shared_ptr<Node> & sp) : value(1) {
    sp.reset(this);
    this->weakThis = sp;
    this->add(p);
}

void Node::add(Point * p) {
    this->points.push_back(p); /* simplified without checking if element exists since it is unnecessary for the problem */
    p->node = this->weakThis.lock(); /* <- critical part */
}

void Node::remove(Point * p) {
    std::vector<Point *>::iterator it = std::find(this->points.begin(), this->points.end(), p);
    if (it != this->points.end()) {
        Point * pr = *it;
        this->points.erase(it);
        pr->node.reset();
    }
}

/*------------main-----------*/

int main() {
    Point p;
    cout << "Value of unmanaged p is " << p.getValue() << endl;
    shared_ptr<Node> n;
    new Node(&p, n); /* This is a bit strange too but other creations will not work */
    cout << "Added p to n" << endl;
    cout << "Value of managed p is " << p.getValue() << endl;

    n->remove(&p);
    n.reset();
    return 0;
}