C++11 case study: How to implement factory design with smartpointers? [Example and test]

Question

This is a case study of working with a factory design pattern in C++11. I would be glad if anyone can explain what actually is going on in the fixing attemppts:

Basic design

Let's say we want to get elements from a factory that we don't want to explicitly save, but we want to make sure they are properly deleted. I googled for examples, but I didn't seem to have found any, so we need to do a bit of testing:

The way I think we can achieve our goal is to use std::unique_ptr objects, to handle all the deleting duties. I only prefer unique_ptr over shared_ptr here, because it calls the destructor of the held element whenever it leaves the scope, so using it makes any hidden copying visible. First let's make a simple factory that is capable of creating lambda functions:

class To_do_base
{
    public:
        std :: function<void()> what_to_do;
        To_do_base() {};
        virtual ~To_do_base() { std :: cout << "~To_do_base()..." << std :: endl; };
        std :: function<void()>& get_function() { return this -> what_to_do; };
};


class Special_to_do : public To_do_base
{
    public:
        Special_to_do()
        { 
            this -> what_to_do = []()
            {
                std :: cout << "I have a special thing for you to do!" << std :: endl;
            };
        };
        ~Special_to_do() { std :: cout << "~Special_to_do()..." << std :: endl; };
};

So now whenever we call the Special_to_do() constructor we generate a function to print out "I have a special thing for you to do!". Let's now make a factory that will instantiate these classes for us:

class To_do_factory
{
    public:
        static std :: unique_ptr<To_do_base> get_to_do( const std :: string& keyword_p )
        {
            if( keyword_p == "special" )
            {
                return std :: unique_ptr<To_do_base>( new Special_to_do() );
            }
            std :: cerr << "Error: Argument_factory::get_argument()..." << std :: endl;
            return nullptr;
        };
};

Let's now make a main() and cross our fingers!

int main()
{
    To_do_factory :: get_to_do( "special" ) -> get_function()();
    return 0;
}

The program outputs:

I have a special thing for you to do!
~Special_to_do()...
~To_do_base()...

The problem

So far so good. But let's double-check the results by saving the function first and invoking it later:

int main()
{
    std :: function<void()> to_do = To_do_factory :: get_to_do( "special" ) -> get_function();
    to_do();
    return 0;
}

The program outputs:

~Special_to_do()...
~To_do_base()...
I have a special thing for you to do!

So it seems that the return value now is actually a copy of the originally created function. Why this is awful? Usually when you are passing lambda functions, they capture some of the variables in their scope (for example this). However, when copying them, those variables can get out of scope, causing not so hefty segmentation faults.

---

I have an answer in which I wrote down everything I deduced so far, and a way to "hack" this design. But I don't know what I was doing there, it's just a case study. Please help me to understand what's going wrong there. Thank you!

Answer 1

Trying to solve the problem

Testing the get_function() return value

Maybe it is not about the smartpointer, but the return value of the get_function(). We should test it:

int main()
{
    To_do_base* a = new Special_to_do();
    std :: function<void()> to_do = a -> get_function();
    to_do();
    delete a;
    return 0;
}

This program outputs:

I have a special thing for you to do!
~Special_to_do()...
~To_do_base()...

Which is the correct behaviour.

Trying to force of passing the unique_ptr by moving it

Maybe we should somehow try and force the factory to return the original object. Maybe returning an rvalue by performing an std::move() on the return value can help:

class To_do_factory
{
    public:
        static std :: unique_ptr<To_do_base>&& get_to_do( const std :: string& keyword_p )
        {
            if( keyword_p == "special" )
            {
                return std :: move( std :: unique_ptr<To_do_base>( new Special_to_do() ) );
            }
            std :: cerr << "Error: Argument_factory::get_argument()..." << std :: endl;
            exit( -1 );
            return std :: move( std :: unique_ptr<To_do_base>( nullptr ) );
        };
};

The code compiles and the output is:

~Special_to_do()...
~To_do_base()...
[1]    329 segmentation fault  ./main

Well, this is a bit embarassing... What if we save the rvalue first and then invoke it? After checking: it just fine without passing rvalue references, but it still creates segmentation fault with rvalues.

Saving the unique_ptr in the main()

So let's revert to lvalues. This code with the first approach to the factory:

int main()
{
    std :: unique_ptr<To_do_base> saved_return_value( To_do_factory :: get_to_do( "special" ) );
    std :: cout << "Return value saved." << std :: endl;
    //std :: function<void()> to_do = To_do_factory :: get_to_do( "special" ) -> get_function(); // WRONG: assigns by copying OR frees dinamically allocated memory after assigning
    std :: function<void()> to_do = saved_return_value -> get_function();
    to_do();
    return 0;
}

generates a good output:

Return value saved.
I have a special thing for you to do!
~Special_to_do()...
~To_do_base()...

Well, it works just fine... But it kind of loses the point of having a function-factory. Here we are able to generate factory elements but only when we are saving them...

Saving the pointers in the factory

Maybe there is a way to save the pointers in the factory. We should try creating a static member variable in the factory to handle the saving. An std :: vector should be fine.

class To_do_factory
{
    public:
        static std :: vector<std :: unique_ptr<To_do_base>> to_do_list;
        static std :: unique_ptr<To_do_base>& get_to_do( const std :: string& keyword_p )
        {
            if( keyword_p == "special" )
            {
                to_do_list.push_back( std :: unique_ptr<To_do_base>( new Special_to_do ) );
                return to_do_list.back();
                //return std :: unique_ptr<To_do_base>( new Special_to_do() );
            }
            std :: cerr << "Error: Argument_factory::get_argument()..." << std :: endl;
            exit( -1 );
            return to_do_list.back();
        };
};

std :: vector<std :: unique_ptr<To_do_base>> To_do_factory :: to_do_list; 
// global scope I have no idea, where else could I define this...

And the input is:

I have a special thing for you to do!
~Special_to_do()...
~To_do_base()...

Finally, a good output! :)

Testing

Let's not get ahead of ourselves, we should test if the lambda returned really is what we want. We can try giving it a value to capture:

class Special_to_do : public To_do_base
{
    private:
        int x = 2;
    public:
        Special_to_do()
        { 
            this -> what_to_do = [&]()
            {
                std :: cout << "I have " << this -> x << " special thing for you to do!" << std :: endl;
            };
        };
        ~Special_to_do() { std :: cout << "~Special_to_do()..." << std :: endl; };
};

Outputs:

I have 2 special thing for you to do!
~Special_to_do()...
~To_do_base()...

Which is the expected output. Still I am really confused about this behaviour... Can anyone show me a "more correct" way to implement this? Thank you :)

Answer 2

If I understand, the question is how to keep variables captured by a lambda from being out of scope (already destructed) if the lambda is stored, copied, etc.

Here are two suggestions.

Lambdas are frequently callbacks. Before going out of scope, the object de-registers itself from the service that would call it back.

Alternatively, all pointers captured by the lambda (whose lifetime is not obviously safe) should be shared_ptrs. This is good if the object should indeed always be available when the lambda is called. You might find http://en.cppreference.com/w/cpp/memory/enable_shared_from_this a helpful discussion.