1

I've got a static factory method in a base class. Because of some reasons I want to each derived class will be instantiated by this factory method hence all these classes have protected ctors.

In real situation the Create function does more additional logic along with error handling.

class Base
{
public:
    virtual ~Base() {}

    template <typename T>
    static void Create(std::unique_ptr<T>& pOut)
    {        
        pOut = std::unique_ptr<T>(new T);
        // ... 
    }
protected:
    Base() {}
};

class Derived : public Base
{
protected:
    Derived() {}
};

int main()
{
    std::unique_ptr<Derived> ptr;
    Derived::Create(ptr);
}

That code obviously doesn't compile since we don't have access to the protected ctor.

prog.cc: In instantiation of 'static void Base::Create(std::unique_ptr<_Tp>&) [with T = Derived]':
prog.cc:33:24:   required from here
prog.cc:17:35: error: 'Derived::Derived()' is protected within this context
   17 |         pOut = std::unique_ptr<T>(new T);
      |                                   ^~~~~
prog.cc:26:5: note: declared protected here
   26 |     Derived() {}
      |     ^~~~~~~

The first solution that seems to be the most common is friend declaration in derived class. However it works I don't like it because:

  1. I have to add such declaration in each derived class
  2. This is a friend
class Derived : public Base
{
protected:
    Derived() {}
    friend void Base::Create<Derived>(std::unique_ptr<Derived>&);
};

Thinking about more generic approach I was trying something like this:

 template <typename T>
    static void Create(std::unique_ptr<T>& pOut)
    {
        static_assert(std::is_base_of_v<Base, T>, "T should be Base-family class.");
        class CreateHelper : public T
        {
            public:
                static void InternalCreate(std::unique_ptr<T>& pOut)
                {
                    pOut = std::unique_ptr<CreateHelper>(new CreateHelper);
                    // ... 
                }
        };
        
        CreateHelper::InternalCreate(pOut);
    }

It works but looks weird to me and:

  1. The real pointer type is CreateHelper however outside this function we don't see that
  2. This approach needs that Base-familiy should be polimorphic since we use pointer to base class (it's seems this condition should be always met but still it's worth to mention about that)

My questions are

  1. What do you think about the last approach?
  2. Is it considered as a bad design?
  • Not related to your problem, but your `Create` is weird, why does it receive a `unique_ptr` when that's what it needs to create? It's like: I am the one to create Derived! Ok, give me an Derived! Sure, but first give me a Derived! ...?? – bolov Feb 13 '21 at 11:37
  • pOut is out parameter. This is how the Create function returns new instance. Why isn't it done by simple return (?) - because in real case this function returns various statuses since the logic of this function is more complicated (this is also reason why I don't want to put this logic to constructor). Second prop is fact that calling the Create I use template type deduction. – moses.nebogipfel Feb 13 '21 at 11:57
  • I know it's an out parameter. I am saying I don't see the point of it. Anyway you know better what you need. – bolov Feb 13 '21 at 14:13

1 Answers1

1

In general, a far better approach here is to simply refer to simple construction helper classes, then you can refer to make_unique() too:

class Base
{
protected:
    struct Accessor
    {
      explicit Accessor() = default;
    };
 
public:
    Base(Accessor) {}
    virtual ~Base() {}

    template <typename T>
    static void Create(std::unique_ptr<T>& pOut)
    {        
        pOut = std::make_unique<T>(Accessor());
        // ... 
    }
};

class Derived : public Base
{
public:
    Derived(Accessor) : Base(Accessor()) {}
};

Only drawback: Derived classes have to adapt their constructor(s) accordingly.

A general point: A factory should almost always be aware of its relevant types in general, at least of partial aspects of the relevant types, provided by polymorphism (interfaces) or/and via traits. So I think, this is a bit more convenient:

template <class T, typename std::enable_if<std::is_base_of<Base, T>::value>::type* = nullptr>
static void Create(std::unique_ptr<T>& pOut)
{        
    pOut = std::make_unique<T>(Accessor());
    // ... 
}

Further on: You might have to rethink about your general creation design here. The common approach here is to return the created object, not to fill a reference. With your current approach, you have to think about exception safety (contracts..) here at least twice for instance...

Possible approach:

template <class T, typename std::enable_if<std::is_base_of<Base, T>::value>::type* = nullptr>
static std::unique_ptr<T> Create()
{        
    auto pOut = std::make_unique<T>(Accessor());
    // ... 

    return pOut;
}
Secundi
  • 1,150
  • 1
  • 4
  • 12
  • Interesting approach. In this form of solution I can instantiate derived type without Create method: Derived d{{}}. Of course I could create special explicit ctor for Accessor but than compilation errors in case of attempt to instantiate Derived outside of Create function could not be helpful. – moses.nebogipfel Feb 13 '21 at 12:08
  • Yes, that's a possible issue independent of your particular aspects here: see [when is a private constructor not a private constructor](https://stackoverflow.com/questions/37618213/when-is-a-private-constructor-not-a-private-constructor) – Secundi Feb 13 '21 at 12:41
  • PS: Added the explicit default constructor for the accessor struct within my answer. What do you mean by not helpful? I thought you didn't want to allow the direct construction of these classes? – Secundi Feb 13 '21 at 13:17
  • Correct, it prevents but compiler will tell you that you can't instantiate because there's no valid conversion or proper ctor. It doesn't tell you that you can't instantiate this because this class expects it. I'm just wondering if exposing ctor in public interface isn't an explicit indication that there is nothing wrong that you instantiate this class wherever you want if only you have sufficient parameters. In other words the class interface doesn't indicate directly that you shouldn't use this ctor. I'm not sure that intentions are visible at the first glance. However it works :) – moses.nebogipfel Feb 13 '21 at 13:36
  • Yes, that's somehow a philosophical issue in doubt :) I made it public in order to allow the usage of make_unique(), that has to be the preferred scheme in contrast to the old construction scheme via new. I think, the parameters are self-explanatory for this case, and since they are very internal, there shouldn't be much confusion about the allowed construction pathways. – Secundi Feb 13 '21 at 13:41
  • 1
    I agree, but let me emphasize that making this ctor protected leads to situation that you're no able to call the "new operator" on this type in the Create method so even the old construction scheme won't work :) Still I'm happy about your post since I haven't considered such approach before. Thanks. – moses.nebogipfel Feb 13 '21 at 13:52