CODEWITHSUNDEEP
c++reference-counting
ibrewster
ibrewster

Reputation: 3632

C++ class destructor delete member if "owner"?

I know in C++ that a pointer is just that: a pointer to a memory location, and there is no concept of "owners". But consider the following situation (not necessarily good code):

class A {
public:
    A(){}
    ~A()
    { if(myObject!=nullptr)
        delete myObject;
    }

    void createMember()
    {myObject=new CrazyCustomClass();}

    CrazyCustomClass *getMember()
    {return myObject;}
private:
    CrazyCustomClass *myObject=nullptr;
}

If it makes a difference, CrazyCustomClass does NOT have a copy constructor, as it makes no sense to copy it. So pretty straight forward - I have a class that, at some point after instantiation, may call new to instantiate a member of type CrazyCustomClass *

The problem is that if at some point I have a copy of class A created (which is fine - I want to be able to copy class A). When that copy is deleted, so is the object pointed to by the original class A instantiation. For example:

void StupidFunction(A *firstObject){
//This is NOT a real function, it simply illustrates the effect of a third-party library function
    //create a new object that is a copy of first object
    A secondObject(*firstObject);
    <do whatever with second object>
    //secondObject goes out of scope here and gets deleted.
}

A *firstObject=new A();
firstObject->createMember();
stupidFunction(firstObject);
CrazyCustomClass *customObject=firstObject.getMember(); //this is now an invalid pointer

In the above example, the StupidFunction is from a third-party library, the idea being that it gives a "temporary" copy of the object that you can work with without messing with the original object, which is good. Class A and CrazyCustomClass are both my code and can be changed at will. Unfortunately, when the "temporary" copy is deleted, the way I wrote my destructor causes problems.

My first thought was to use shared_ptr, something like so:

std::shared_ptr<CrazyCustomClass> sharedObject=std::make_shared<CrazyCustomClass>(new CrazyCustomClass);

...but that gave me an error when compiling:

candidate constructor (the implicit copy constructor) not viable: no known conversion from 'CrazyCustomClass *' to 'const CrazyCustomClass' for 1st argument; dereference the argument with *

and if I do dereference the argument with *, it gives me an error about the copy constructor of "CrazyCustomClass" being deleted, which is true - there is no sensible way to copy CrazyCustomClass.

So my question is: how can I refactor class A such that myObject gets properly deleted when firstObject goes out of scope, but not when any "temporary" copies of A get deleted?

Upvotes: 0

Views: 249

Answers (1)

ibrewster
ibrewster

Reputation: 3632

Using a shared_ptr is in fact a solution to this problem, however the code as attempted in the original question is incorrect. There are two (at least) different ways to initialize a shared_ptr (ref: https://msdn.microsoft.com/en-us/library/hh279669.aspx). First, you can do it by using new as a constructor argument:

shared_ptr<CrazyCustomClass> myObject(new CrazyCustomClass)

Secondly, and this is the generally preferred method, you can use the make_shared function (as attempted in the original post), which takes not the new object, but the arguments to be passed to the object constructor, in this case nothing:

shared_ptr<CrazyCustomClass> myObject=make_shared<CrazyCustomClass>()

The original code simply got these two methods mixed up, thus the errors about copy constructor: it was trying to instantiate a new CrazyCustomClass object with a pointer to a CrazyCustomClass object as the constructor argument.

Once using a shared_ptr, the delete in the destructor must be removed.

Tip of the hat to @tkausl and @alterigel for pointing out the error in the comments on the question!

Upvotes: 1

Related Questions