Container with unique pointer to an abstract class

Question

I've been trying to create a factory method which allocates subclasses entities from a given abstract class, which I'll name Shape for simplicity. It looks something like this:

Shape* makeChild(Type type) const
{
     switch(type)
    {
        case QUAD:
            return new Quad();
        case RECTANGLE:
            return new Rectangle();
        case TRIANGLE:
            return new Triangle();
        default:
            return new Rectangle();
    }
}

Now I'm trying to keep these entities inside a structure, and I understand I need to allocate Shape* entities, but I would like for the objects to be automatically managed by using std::unique_ptr<Shape> instead. My ShapeTree struct looks something like this:

struct ShapeTree {
    int index_;
    std::vector<std::unique_ptr<Shape>> shapes_;

    void add(Shape* shape) {shapes_.push_back(std::make_unique<Shape>(shape));}
    void removeLast(){shapes_.pop_back();}

    ShapeTree(const int index) : index_{index}{}
    ShapeTree(const int index, std::vector<std::unique_ptr<Shape>>& shapes) : index_{index}, shapes_{std::move(shapes)}{}};

Compiler complains about having Shape default copy constructor being marked as deleted, I understand this is the case since I have a unique pointer and I can't copy it, but deleting the copy constructor and assignment and using the default move constructor and assignment won't work either. It also complains that no instances of Shape can be created as it is abstract, even thought I am only returning subclasses.

A different version of the same structure works completely fine when using raw pointers. E.g.

struct ShapeTree {
    int index_;
    std::vector<Shape*> shapes_;

    void add(Shape* shape) {shapes_.push_back(shape);}
    void removeLast(){shapes_.pop_back();}

    ShapeTree(const int index) : index_{index}{}
    ShapeTree(const int index, std::vector<Shape*>& shapes) : index_{index}, shapes_{std::move(shapes)}{}};

What am I doing wrong? How can i achieve the same result using unique_ptr?

Answer 1

The immediate cause of your error is this line:

void add(Shape* shape) {shapes_.push_back(std::make_unique<Shape>(shape));}

std::make_unique is used to create a new object to be managed by the unique_ptr.
In your case you already allocated your object with new, and therefore you should simply use the std::unique_ptr constructor.

An immediate solution would be:

//----------------------------------------vvvvvvvvvvvvvvv-----------------
void add(Shape* shape) {shapes_.push_back(std::unique_ptr<Shape>(shape));}

---

However:
A better solution will avoid using new altogether.
makeChild can return a std::unique_ptr which can be moved into the vector:

#include <vector>
#include <memory>

struct Shape {};
struct Rectangle : public Shape {};
// ... all the derived Shapes

std::unique_ptr<Shape> makeChild()
{
    // Here you can create a std::unique_ptr to any derive from Shape based on a `Type` parameter you can add:
    return std::make_unique<Rectangle>();
}

struct ShapeTree {
    int index_;
    std::vector<std::unique_ptr<Shape>> shapes_;

    //---------------------------------------------------------vvvvvvvvv------
    void add(std::unique_ptr<Shape> shape) { shapes_.push_back(std::move(shape)); }
    void removeLast() { shapes_.pop_back(); }

    // ...
};

int main()
{
    ShapeTree  st{ 333 };
    st.add(makeChild());
}