Search code examples
c++containersabstract-classdangling-pointer

C++ Add derived object of abstract class into another class without dangling pointers?


I have a simple container class that points to an abstract class and I have functions to get/set the pointer in the container class. More concretely, the class looks like this:

class Container
{
    Abstract* thing;
public:
    void set(Abstract &obj)
    {
        thing = &obj; //danger of dangling pointer
    }

    Abstract* get()
    {
        return thing;
    }
};

Abstract is an abstract class. As can be seen already, there's a danger of a dangling pointer. I know that I could make a copy of the object (new) and then point to it. But I can't create an instance of an abstract class. What solutions to this are there?


The following are just more information:

Class definitions

class Abstract
{
public:
    virtual void something() = 0;
};

class Base : public Abstract
{
    int a;
public:
    Base() {}
    Base(int a) : a(a){}
    virtual void something()
    {
        cout << "Base" << endl;
    }
};

class Derived : public Base
{
    int b;
public:
    Derived() {}
    Derived(int a, int b) : Base(a), b(b){}
    virtual void something()
    {
        cout << "Derived" << endl;
    }
};

Simple tests

void setBase(Container &toSet)
{
    Base base(15);
    toSet.set(base);
}

void setDerived(Container &toSet)
{
    Derived derived(10, 30);
    toSet.set(derived);
}

int main()
{
    Container co;

    Base base(15);
    Derived derived(10, 30);

    Base *basePtr;
    Derived *derivedPtr;

    //This is fine
    co.set(base);
    basePtr = static_cast<Base *>(co.get());
    basePtr->something();

    //This is fine
    co.set(derived);
    derivedPtr = static_cast<Derived *>(co.get());
    derivedPtr->something();

    //Reset
    basePtr = nullptr;
    derivedPtr = nullptr;

    //Dangling pointer!
    setBase(co);
    basePtr = static_cast<Base *>(co.get());
    basePtr->something();

    //Dangling pointer!
    setDerived(co);
    derivedPtr = static_cast<Derived *>(co.get());
    derivedPtr->something();

    return 0;
}

Solution

  • What you need to do is to define your memory ownership concretely.

    Container::set accepts an instance of Abstract by reference, which usually does not imply an ownership transfer:

     void set(Abstract &obj){...} // Caller retains ownership of obj, but now we have a weak reference to it
    

    Then the onus of deletion is not on you.

    Container::get returns a pointer which implies ownership, indicating that someone who calls set should not invalidate the passed object.

    Abstract* get(){...}
    

    This could be problematic, as you've stated.

    You have a few options

    • Encode these memory ownership semantics within Container with proper documentation (Code by contract)
    • Use a smart pointer like std::shared_ptr

    In the former case, whether it works or not depends on the user reading and understanding your API, and then behaving nicely with it. In the latter case, the pointer object owns itself, and will delete the allocated memory when the last instance goes out of scope.

     void set(std::shared_ptr<Abstract> obj){...} 
    // now Container participates in the lifetime of obj, 
    // and it's harder to nullify the underlying object 
    // (you'd have to be intentionally misbehaving)