factory, unique_ptr and static_cast

Question

Consider polymorphic classes with a base object, a derived interface, and a final object:

// base object

struct object
{
    virtual ~object() = default;
};

// interfaces derived from base object

struct interface1 : object
{
    virtual void print_hello() const = 0;

    template<typename T>
    static void on_destruction(object* /*ptr*/)
    {
        std::cout << "interface1::on_destruction" << std::endl;
    }
};

// final object

struct derived1 : interface1
{
    virtual void print_hello() const override
    {
        std::cout << "hello" << std::endl;
    }

    static std::string get_type_name()
    {
        return "derived1";
    }
};

In the real use case, final objects are defined through a plugin system, but that is not the point. Note that I want to be able to call on_destruction when an object is destroyed (see register_object below). I want to use these classes as follows:

int main()
{
    // register derived1 as an instantiable object,
    // may be called in a plugin

    register_object<derived1>();

    // create an instance using the factory system

    auto instance = create_unique<interface1>("derived1");
    instance->print_hello();

    return 0;
}

Using std::unique_ptr to manage the objects, I ended up with the following code for register_object:

template<typename T>
using unique = std::unique_ptr<
    T,
    std::function<void(object*)> // object deleter
>;

namespace
{
    std::map< std::string, std::function<unique<object>(void)> > factory_map;
}

template<typename T>
void register_object()
{
    factory_map.emplace(
        T::get_type_name(),
        []()
        {
            unique<T> instance{
                new T,
                [](object* ptr)
                {
                    T::on_destruction<T>(ptr);

                    delete ptr;
                }
            };

            return static_move_cast<object>(
                std::move(instance)
            );
        }
    );
}

And the create* functions:

unique<object> create_unique_object(const std::string& type_name)
{
    auto f = factory_map.at(type_name);
    return f();
}

template<typename T>
unique<T> create_unique(const std::string& type_name)
{
    return static_move_cast<T>(
        create_unique_object(type_name)
    );
}

You noticed in register_object and create_unique the call to static_move_cast, which is declared as:

template<typename U, typename T, typename D>
std::unique_ptr<U, D>
static_move_cast
(
    std::unique_ptr<T, D>&& to_move_cast
)
{
    auto deleter = to_move_cast.get_deleter();

    return std::unique_ptr<U, D>{
        static_cast<U*>(
            to_move_cast.release()
        ),
        deleter
    };
}

The goal behind static_move_cast is to allow static_cast on std::unique_ptr while moving the deleter during the cast. The code is working, but I feel like hacking std::unique_ptr. Is there a way to refactor the code to avoid my static_move_cast?

Answer 1

static_move_cast is unnecessary within register_object, since you can just use the converting constructor of unique_ptr template< class U, class E > unique_ptr( unique_ptr<U, E>&& u ):

        unique<T> instance{
            new T,
            // ...
        };

        return instance;

Or, even simpler, construct and return a unique<object> directly, since T* is convertible to object*:

        return unique<object>{
            new T,
            // ...
        };

However for create_unique the use of static_move_cast is unavoidable, since the converting constructor of unique_ptr won't work for downcasts.

Note that shared_ptr has static_pointer_cast, which performs downcasts, but there is no corresponding facility for unique_ptr, presumably because it it is considered straightforward and correct to perform the cast yourself.

Answer 2

I would say it is good solution given the requirements. You transfer the responsibility to the caller of create_unique. He must give correct combination of type and string and string that is in the registry.

auto instance = create_unique<interface1>("derived1");
//                            ^^^^^^^^^^   ^^^^^^^^
//                         what if those two don't match?

You could improve it a bit by changing the static_cast to dynamic_cast. And the caller of create_unique should always check that he got non-null pointer before calling anything on it.

Or at least use dynamic_cast with assert in debug mode, so you catch mismatches while developing.

---

Alternative refactoring: Have separate factory for every existing interface.