Using Smart Pointers with Factory Pattern

Question

I am trying to use smart pointers in factory design pattern. I made a google search to get some ideas but could not find any kind of implementation, but lots of great ideas. So, I wrote my own code, but there are two points I am not sure.

First, I will add my code and then ask my questions.

// AgentGameStyleFactory.hpp

#ifndef AgentGameStyleFactory_hpp
#define AgentGameStyleFactory_hpp

#include "AgentGameStyle.hpp"

#include <memory>

class AgentGameStyleFactory
{
public:

    static std::unique_ptr<AgentGameStyle> instantiate(const AgentPlayingStyleData::ePLAYING_CHARACTER pStyle);

private:

    static AgentGameStyle* instantiateHelper(const AgentPlayingStyleData::ePLAYING_CHARACTER pStyle);
};

#endif /* AgentGameStyleFactory_hpp */

// AgentGameStyleFactory.cpp
#include "AgentGameStyleFactory.hpp"

using namespace AgentPlayingStyleData;

std::unique_ptr<AgentGameStyle> AgentGameStyleFactory::instantiate(const ePLAYING_CHARACTER pStyle)
{
    std::unique_ptr<AgentGameStyle> newStyle( instantiateHelper(pStyle) );
    return std::move(newStyle);
}

AgentGameStyle* AgentGameStyleFactory::instantiateHelper(const ePLAYING_CHARACTER pStyle)
{
    AgentGameStyle* newStyle = NULL;

    switch(pStyle)
    {
        case ePLAYING_CHARACTER::ePC_CONTAIN:
            newStyle = new ContainGameStyleAgent();
            break;
        case ePLAYING_CHARACTER::ePC_COUNTER:
            newStyle = new CounterGameStyleAgent();
            break;
        case ePLAYING_CHARACTER::ePC_STANDARD:
            newStyle = new StandardGameStyleAgent();
            break;
        case ePLAYING_CHARACTER::ePC_ATTACKING:
            newStyle = new AttackGameStyleAgent();
        case ePLAYING_CHARACTER::ePC_OVERLOAD:
            newStyle = new OverloadGameStyleAgent();
            break;
        default:
            newStyle = new StandardGameStyleAgent();
            break;
    }

        return newStyle;
}

As you can see, I am creating the related style in the factory and then returning and assigning it as

mPlayingCharacteristic = AgentGameStyleFactory::instantiate(pPlayingCharacteristic);

where mPlayingCharacteristic is std::unique_ptr<AgentGameStyle>

My first questions is about returning the unique_ptr. According to the posts I have read, it seems correct, but the compiler (Xcode) gives me "Moving a local object in a return statement prevents copy elision" warning. Is this something normal, something I should expect to have or is something wrong here?

My second question is whether I am initiating std::unique_ptr<AgentGameStyle> newStyle in AgentGameStyleFactory.cpp correctly, which means using a helper method as AgentGameStyleFactory::instantiateHelper is right thing to do?

Answer 1

Your compiler is right to warn you about return std::move(...) - you can (and should) simply write:

return newStyle;

I'd encourage you to ditch the separate helper with its bare new, and just create the smart pointer directly (assuming C++14 or later):

std::unique_ptr<AgentGameStyle> AgentGameStyleFactory::instantiate(const ePLAYING_CHARACTER pStyle)
{
   switch(pStyle) {
        case ePLAYING_CHARACTER::ePC_CONTAIN:
            return std::make_unique<ContainGameStyleAgent>();
        case ePLAYING_CHARACTER::ePC_COUNTER:
            return std::make_unique<CounterGameStyleAgent>();
        case ePLAYING_CHARACTER::ePC_STANDARD:
            return std::make_unique<StandardGameStyleAgent>();
        case ePLAYING_CHARACTER::ePC_ATTACKING:
            return std::make_unique<AttackGameStyleAgent>();
        case ePLAYING_CHARACTER::ePC_OVERLOAD:
            return std::make_unique<OverloadGameStyleAgent>();
        default:
            return std::make_unique<StandardGameStyleAgent>();
    }
}

Answer 2

There is no need for "helper method" returning raw pointers and there is no need to call new manually:

std::unique_ptr<AgentGameStyle>
AgentGameStyleFactory::instantiate(const ePLAYING_CHARACTER pStyle)
{
    switch(pStyle)
    {
        case ePLAYING_CHARACTER::ePC_CONTAIN:
        {
            return std::make_unique<ContainGameStyleAgent>();
        }
        //...
        default:
        {
            return std::make_unique<StandardGameStyleAgent>();
        }
    }
}