CODEWITHSUNDEEP
c++pointersreferencecompositionownership
Depenau
Depenau

Reputation: 133

c++ passing ownership to composite class

I have an object (let's call it A) that I want to store another object (let's say it has type B and is called m), which will be passed from the function that creates A. The creation of B is made by another function that I don't have control over.

class A
{
    public:
    A(B& m)
    {
        this->m = m
    }

    private:
    B& m;
};

//

void SomeLongLivedClass::someFunction()
{
    B m = createB();
    A a = new A{ m }

    // m goes out of scope
}

When someFunction()'s scope ends, m is destroyed and the reference in A becomes dangling. I can only obtain the right B through createB(), which is a part of a dll I don't have access to. How can I make m an lvalue and pass ownership to A?

I chose to create B outside of A to reduce clutter in A, but maybe that's a fundamentally wrong way to do it? I've read a little about composition vs. aggregation - is it wrong to do composition by shoving objects into a class like this?

I thought about passing m as an std::unique_ptr<B>, but I can't wrap my head around whether or not m will still go out of scope, and then leave the pointer dangling instead.

It goes without saying that this is just pseudocode and the real situation is more complicated. The creation of B is a little more involved than that, which is why I wanted to do it outside A.

Upvotes: 1

Views: 94

Answers (2)

Gajanana D S
Gajanana D S

Reputation: 59

This function does not appear to be clear and complete. Looks like this has other issues also.

void SomeLongLivedClass::someFunction()
{
    B m = createB();
    A a = new A{ m }

    // m goes out of scope
}

When you say m goes out of scope, did you call delete before (delete m)? Otherwise it is a memory leak. If you have already called it then anyway m is not accessible. Both m & a are local variables of function someFunction. so unless you have some code in someFunction which stores value of object a (may be in any member variable of SomeLongLivedClass) object a also goes out of scope. Hence no point in worrying about child object B of class A, since object of class A itself goes out of scope. That is the reason I believe code is incomplete. However when you decide to store object of A in someFunction in member variable of SomeLongLivedClass, like below:

void SomeLongLivedClass::someFunction()
{
    B m = createB();
    A a = new A{ m }
    m_A = a; // m_A is member variable of SomeLongLivedClass with type A*
    m_B = m; // m_B is member variable of SomeLongLivedClass with type B*
    // m goes out of scope
}

Then you can also store object of B in some other member variable of SomeLongLivedClass right? Then both object of A & object of B are long lived. In that scenario you do not have any issues either.

Upvotes: -1

Ted Lyngmo
Ted Lyngmo

Reputation: 117812

You should store a B in A, not a B&. If you store a reference, then accessing the referenced object after it's gone out of scope has undefined behavior.

Instead, copy or move m into A. A simple version is to take B by value in A's constructor:

class A {
public:
    A(B m) : m(std::move(m)) {}    // take by value, then move

private:
    B m;
};

Another alternative is to provide one or both of these constructors:

class A {
public:
    A(const B& m) : m(m) {}        // copy m
    A(B&& m) : m(std::move(m)) {}  // move m
    
private:
    B m;
};

Then in the calling code, if you have an lvalue, cast it to an rvalue reference by using std::move:

void SomeLongLivedClass::someFunction() {
    B m = createB();
    // 
    auto a = std::make_unique<A>(std::move(m));
    //
}

Note: This assumes that B is well-written with respect to ownership of resources, which all classes in the standard library are and presumably/hopefully the DLL writers did a good job too.

Upvotes: 1

Related Questions