mana
mana

Reputation: 595

Using smart pointers to keep track of data members that may be deleted

I have two classes A and B. I compute a B deterministically from an A. For each A, I want to keep track of the B with my_B for as long as it exists. Once the B is destructed, I want the my_B to be changed to something like nullptr.

class A{
// stuff
public:
    B ComputeB(){
        if (my_B is null){
            B result = B(A);
            my_B = B; // some kind of reference
            return B(A);
        } 
        else {
            return my_B;
        }
    }
    
    ~A(){  /* Do I need a destructor? */ }
private:
    WhatTypeHere my_B;
}

When B is destructed, what will cause my_B to refer to nullptr (or the equivalent for WhatTypeHere)?

Upvotes: 2

Views: 168

Answers (2)

Amir Kirsh
Amir Kirsh

Reputation: 13810

Using shared_ptr and weak_ptr

In order to keep your B object alive in A as long as it is still in use, you should have a data member in A of type std::weak_ptr<B> this will allow to access the created B object, as long as it is alive.

The return value from computeB would be std::shared_ptr<B> that would be either acquired from the std::weak_ptr<B> member or created, if the latter is holding nullptr.


Thread safety

The decision whether to create or acquire the existing B shall be thread-safe. For this you shall try to fetch the actual B held by the weak_ptr by using the lock() method, then only if the return value is nullptr create a new one.


The code would look like this:

class A {
    // stuff
public:
    std::shared_ptr<B> ComputeB() {
        std::shared_ptr<B> shared_b = my_B.lock();
        if (!shared_b){
            shared_b = std::make_shared<B>(*this);
            my_B = shared_b;
        } 
        return shared_b;
    }
    // no need for a destructor, unless "stuff" needs one
    // ~A(){} 
private:
    std::weak_ptr<B> my_B;
};

Copy and assignment

The behavior of above class in copying and assignment is problematic, as the default copy constructor and default assignment operator would perform member-wise copy/assignment, which may result with two different A's holding a weak_ptr to the same B. Most probably this is not what you want, especially not if A is mutable (i.e. can change its inner values).

To present suggested code for copy and assignment, let's assume A holds an int member. The code would then look like this:

class A {
    int i;
public:
    A(int i1): i(i1) {}
    void set(int i1) { i = i1; }
    std::shared_ptr<B> ComputeB() {
        std::shared_ptr<B> shared_b = my_B.lock();
        if (!shared_b){
            shared_b = std::make_shared<B>(*this);
            my_B = shared_b;
        } 
        return shared_b;
    }
    A(const A& a): i(a.i) {}
    A& operator=(const A& a) { i = a.i; return *this; }
    ~A() {}
private:
    std::weak_ptr<B> my_B;
};

Preserving constness

In the code above, the call to ComputeB() cannot be done on a const A object. If we want to support that we need to have a const version of this function. For matter of semantics I prefer renaming this method (both the const and non-const versions) to getB.

To present suggested code that adds the option for calling getB on a const A object we need to present also an example of class B which is able to hold either a const or non-const reference to A. The code would then look like this:

class A {
    int i;
    // to prevent code duplication for the const and non-const versions
    template<typename AType>
    static auto getB(AType&& a) {
        std::shared_ptr<B> shared_b = a.my_B.lock();
        if (!shared_b){
            shared_b = std::make_shared<B>(std::forward<AType>(a));
            a.my_B = shared_b;
        } 
        return shared_b;
    }
public:
    A(int i1): i(i1) {}
    void set(int i1) { i = i1; }
    std::shared_ptr<B> getB() {
        return getB(*this);
    }
    std::shared_ptr<const B> getB() const {
        return getB(*this);
    }
    A(const A& a): i(a.i) {}
    A& operator=(const A& a) { i = a.i; return *this; }
    ~A() {}
private:
    mutable std::weak_ptr<B> my_B;
};

And for B:

class B {
    union Owner {
        A* const ptr;
        const A* const const_ptr;
        Owner(A& a): ptr(&a) {}
        Owner(const A& a): const_ptr(&a) {}
    } owner;
public:
    B(A& a): owner(a) {}
    B(const A& a): owner(a) {}
    const A& getOwner() const {
        return *owner.const_ptr;
    }
    A& getOwner() {
        return *owner.ptr;
    }
};

On the use of union to manage const and non-const versions of the same pointer, see:

Working example: http://coliru.stacked-crooked.com/a/f696dfcf85890977


Private creation token

The code above allows anybody to create objects of B which may lead to undesired possibilities, like creating a non-const B object via the constructor that gets const A& a, resulting with a potential casting from const to non-const when calling getOwner().

A good solution might be to block the creation of B and allow it only from class A. Since the creation is done via make_shared putting B's constructors in the private section of B with a friend declaration for A wouldn't help, it is not A that is calling new B it is make_shared. So we go for a private token approach as in the following code:

class A {    
    int i;
    // only authorized entities can create B
    class B_PrivateCreationToken {};    
    friend class B;

    template<typename AType>
    static auto getB(AType&& a) {
        std::shared_ptr<B> shared_b = a.my_B.lock();
        if (!shared_b){
            shared_b = std::make_shared<B> (
                  std::forward<AType>(a),
                  B_PrivateCreationToken{} );
            a.my_B = shared_b;
        } 
        return shared_b;
    }
public:

    // public part as in above version...

private:
    mutable std::weak_ptr<B> my_B;
};

And for B:

class B {
    union Owner {
        A* const ptr;
        const A* const const_ptr;
        Owner(A& a): ptr(&a) {}
        Owner(const A& a): const_ptr(&a) {}
    } owner;
public:
    B(A& a, A::B_PrivateCreationToken): owner(a) {}
    B(const A& a, A::B_PrivateCreationToken): owner(a) {}

    // getOwner methods as in above version...

};

Code: http://coliru.stacked-crooked.com/a/f656a3992d666e1e

Upvotes: 4

Tharwen
Tharwen

Reputation: 3107

You can return a std::shared_ptr from ComputeB(), and make my_B a std::weak_ptr. Something like this:

std::shared_ptr<B> ComputeB() {
    if (my_B.expired()) {
        auto result = std::make_shared<B>(*this);
        my_B = result;
        return result;
    } else {
        return std::shared_ptr<B>(my_B);
    }
}

private:
std::weak_ptr<B> my_B;

The idea is that any caller of ComputeB becomes the partial owner of the B instance, which means that it will only be destroyed when all shared_ptrs to it are destroyed. The purpose of the weak_ptr is to point to the B instance without owning it, so the lifetime isn't tied to the A instance at all

Upvotes: 2

Related Questions