Search code examples
c++shared-ptrcopy-constructor

Copy control in abstract classes


I have an abstract class and two concrete subclasses (Store), both with a pointer to another concrete subclass which is derived from an abstract class (Factory). Below is the code for the Store. I wanted to prevent memory leaks, so I started to write the copy control. I can't instantiate a new Factory however, because I don't know upfront what type it will be. What is a good practice to circumvent this? I could write the copy control in the concrete Stores, but then I have duplicate code. I also tried to work with smart pointers instead, but there I find another difficulty. The snippet myFactory = std::make_shared<AbstractFactory>(ConcreteFactoryA()); apparently creates an AbstractFactory first, and then fills it with a ConcreteFactoryA. However, as the name suggests, AbstractFactory cannot be instantiated, as the compiler is telling me. Can you use shared_ptrs with abstract classes?

Code with plain pointers:

#pragma once
#include "AbstractFactory.h"

class AbstractStore
{
public:
    // Copy control
    AbstractStore(const AbstractStore& orig) : myFactory(new AbstractFactory(orig.myFactory)) {}
    AbstractStore& operator=(const AbstractStore& orig) { return *this; } // TODO
    ~AbstractStore(void) {}
protected:
    // Constructor
    AbstractStore(void) {}
    // Data members
    AbstractFactory* myFactory;
};

class ConcreteStoreA : public AbstractStore
{
public:
    ConcreteStoreA(void) { myFactory = new ConcreteFactoryA; }
    ~ConcreteStoreA(void) {}
};

class ConcreteStoreB : public AbstractStore
{
public:
    ConcreteStoreB(void) { myFactory = new ConcreteFactoryB; }
    ~ConcreteStoreB(void) {}
};

Code with smart pointers:

#pragma once
#include "AbstractFactory.h"
#include <memory>

class AbstractStore
{
public:
    // Copy control
    AbstractStore(const AbstractStore& orig) : myFactory(orig.myFactory) {}
    AbstractStore& operator=(const AbstractStore& orig) { myFactory = orig.myFactory; return *this; }
    ~AbstractStore(void) {}
protected:
    // Constructor
    AbstractStore(void) {}
    // Data members
    std::shared_ptr<AbstractFactory> myFactory;
};

class ConcreteStoreA : public AbstractStore
{
public:
    ConcreteStoreA(void) { myFactory = std::make_shared<AbstractFactory>(ConcreteFactoryA()); }
    ~ConcreteStoreA(void) {}
};

class ConcreteStoreB : public AbstractStore
{
public:
    ConcreteStoreB(void) { myFactory = std::make_shared<AbstractFactory>(ConcreteFactoryB()); }
    ~ConcreteStoreB(void) {}
};

Solution

  • You are not using make_shared correctly. Use:

    std::make_shared<ConcreteFactory>();
    

    You call it without any arguments here. make_shared is not accepting a constructed object but the arguments that are forwarded to it's constructor. In your case you would be forwarding to the copy constructor, which works poorly with abstract hierarchies. If you want copyable objects in hierarchies, use clone member functions with covariant return types.

    This will return a shared_ptr<ConcreteFactory> which will be converted to shared_ptr<AbstractFactory> in the assignment (see (9) here. Also, use constructor initializer lists and virtual destructors.