How to implement factory+decorator pattern in c++11

Question

I decided to study/translate Head First Design Patterns' Java code to C++11, and I was able to implement most of the patterns using automatic memory management thanks to smart pointers. However, I have a problem with one of the examples. Here is my code:

#include <iostream>
#include <memory>

class AbstractBase {
public:
    virtual void foo() = 0;
    virtual ~AbstractBase() = default;
};

class A : public AbstractBase {
public:
    void foo() override { std::cout << "Class A: foo() called" << std::endl; }
};

class B : public AbstractBase {
public:
    void foo() override { std::cout << "Class B: foo() called" << std::endl; }
};

class FooDecorator : public AbstractBase {
public:
    FooDecorator(AbstractBase *pBase): mpBase(pBase) { }
    void foo() override
    {
        mpBase->foo();
        ++mNumberOfFooCalls;
    }
    static int getFooCalls() { return mNumberOfFooCalls; }

private:
    static int mNumberOfFooCalls;
    AbstractBase *mpBase;
};

class AbstractFactory {
public:
    virtual std::unique_ptr<AbstractBase> createA() = 0;
    virtual std::unique_ptr<AbstractBase> createB() = 0;
    virtual ~AbstractFactory() = default;
};

class CountingFactory : public AbstractFactory {
public:
    std::unique_ptr<AbstractBase> createA()
    {
        // auto pA = new A();
        // return std::unique_ptr<AbstractBase>(new FooDecorator(pA));
        std::unique_ptr<AbstractBase> pA(new A());
        return std::unique_ptr<AbstractBase>(new FooDecorator(pA.get()));
    }

    std::unique_ptr<AbstractBase> createB()
    {
        // auto pB = new B();
        // return std::unique_ptr<AbstractBase>(new FooDecorator(pB));
        std::unique_ptr<AbstractBase> pB(new B());
        return std::unique_ptr<AbstractBase>(new FooDecorator(pB.get()));
    }
};

int FooDecorator::mNumberOfFooCalls = 0;

int main()
{
    std::unique_ptr<AbstractFactory> pFactory(new CountingFactory());
    std::unique_ptr<AbstractBase> pObjA = pFactory->createA();
    std::unique_ptr<AbstractBase> pObjB = pFactory->createB();
    pObjA->foo();
    pObjB->foo();
    std::cout << "Foo called " << FooDecorator::getFooCalls()
              << " times." << std::endl;
}

What this code does essentially is; there are two derived classes A and B; and they each have a single member function that shows which one has been called. There is also a decorator called FooDecorator that adds capability to count calls made to foo().

In addition to these, there is also CountingFactory which is used to get decorated objects directly.

In the main part, using this factory, I create an instance of A and an instance of B. Then call foo() from each.

When I compile this code using clang 3.5 and run it, I do not get any errors, but the result is a bit different than expected as it calls B::foo() twice:

Class B: foo() called
Class B: foo() called
Foo called 2 times.

On the other hand, when I compile the code using gcc 4.9.2 and run it, I get the following error:

pure virtual method called
terminate called without an active exception

It looks like the problem is the unique_ptrs inside the CountingFactory. My understanding is the pointer that's used to initialize the decorated object gets freed and it leads to either undefined behavior (clang case) or termination (gcc case).

As a result, I decided to use raw pointers and added the (commented-out above) lines:

    auto pA = new A();
    return std::unique_ptr<AbstractBase>(new FooDecorator(pA));

    auto pB = new B();
    return std::unique_ptr<AbstractBase>(new FooDecorator(pB));

Doing this, things worked successfully, I got the expected output from both compilers. However, now there is a memory leak, and the allocations must be deleted.

Having almost always found a solution with smart pointers to problems like these, I am struggling to come up with the best approach to this issue. I also tried replacing unique_ptrs with shared_ptrs, but to no avail, it did not work.

Can I still get away with smart pointers following a different approach? Or do I have to manually manage the memory I allocated inside the factory (not preferred)?

Answer 1

The reason for the crash is that you just assign the inner pointer via get method.

std::unique_ptr<AbstractBase> pB(new B());
return std::unique_ptr<AbstractBase>(new FooDecorator(pB.get()));

Which means that when scope ends, the memory of your inner B (or A) is gets deleted. What you can do is call release instead.

But yeah to avoid the leak you should have a unique_ptr in your FooDecorator as well.

Live on IdeOne

Answer 2

As I understand, you need FooDecorator to take ownership of pBase. You can achieve this by changing

AbstractBase *mpBase;

to

std::unique_ptr<AbstractBase> mpBase;

So you create FooDecorator in CountingFactory like this:

return std::unique_ptr<AbstractBase>(new FooDecorator(new A()));

Or in C++14:

return std::make_unique<FooDecorator>(new A());