5

I have a singleton class with a private constructor. In the static factory method I do the following:

shared_ptr<MyClass> MyClass::GetInstance()
{
    static once_flag onceFlag;

    call_once(onceFlag, []() {
        if (_instance == nullptr)
            _instance.reset(new MyClass());
    });

    return _instance;
}

If I use

_instance = make_shared<MyClass>();

the code does not compile. My question is: why new can invoke a private constructor but make_shared not?

user2683038
  • 667
  • 6
  • 17
  • 2
    This code makes no sense because sharing ownership of singleton is pointless when you still keep an instance of `shared_ptr` for yourself (I assume that `_instance` is `shared_ptr< MyClass >`) and checking whether `_instance` is nullptr is pointless because this block is protected by once flag. And making `make_shared` work (by declaring it a friend for example) in this case makes no sense as well because then it would be possible to create instances of `MyClass` everywhere. – user7860670 Jul 16 '17 at 09:44
  • 1
    OK, I get your point, the condition ``if (_instance == nullptr)`` is not necessary. _instance is a static class variable. What i do not understand is why a ``shared_ptr`` does not make sense? – user2683038 Jul 16 '17 at 13:40
  • Possible duplicate of [How do I call ::std::make\_shared on a class with only protected or private constructors?](https://stackoverflow.com/questions/8147027/how-do-i-call-stdmake-shared-on-a-class-with-only-protected-or-private-const) – Toby Speight Oct 10 '19 at 11:20

3 Answers3

4
  1. As mentioned, std::make_shared or its component parts don't have access to private members.

  2. the call_once and once_flag are un-necessary. They are implicit in c++11 static initialisation,

  3. You normally would not want to expose the shared pointer.

 

class MyClass
{
    MyClass() {}

public:
    static MyClass& GetInstance()
    {
        static auto instance = MyClass();
        return instance;
    }
};

However, there is one case I can imagine where you would want to expose a shared pointer to the impl - this is in the case where the class can choose to 'break off' or 'reset' the impl to a new one. In this case I would consider code like this:

class MyClass2
{
    MyClass2() {};

    static auto& InternalGetInstance()
    {
        static std::shared_ptr<MyClass2> instance { new MyClass2 };
        return instance;
    }

public:

    static std::shared_ptr<MyClass2> GetInstance()
    {
        return std::atomic_load(std::addressof(InternalGetInstance()));
    }

    static void reset() {
        std::atomic_store(std::addressof(InternalGetInstance()),
                        std::shared_ptr<MyClass2>(new MyClass2));

    }  
};

However, in the end, it is my view that 'staticness' of a class should be an implementation detail, and unimportant to the user of the class:

#include <memory>
#include <utility>

class MyClass
{
    // internal mechanics

    struct Impl {

        auto doSomething() {
            // actual implementation here.
        }
    };

    // getImpl now becomes the customisation point if you wish to change the
    // bahviour of the class later
    static Impl& getImpl() {
        static auto impl = Impl();
        return impl;
    }


    // use value semantics - it makes for more readable and loosely-coupled code
public:
    MyClass() {}

    // public methods defer to internal implementation

    auto doSomething() {
        return getImpl().doSomething();
    }
};


int main() {

    // note: just create objects
    auto mc = MyClass();
    mc.doSomething();

    // now we can pass the singleton as an object. Other functions don't even
    // need to know it's a singlton:

    extern void somethingElse(MyClass mc);
    somethingElse(mc);
}

void somethingElse(MyClass mc)
{

}
Richard Hodges
  • 68,278
  • 7
  • 90
  • 142
  • If I abstract from things that do not address the actual question, what remains is that the mechanism invoked by `make_shared` does not retain the privilege of accessing private members (here the default constructor) that the caller of `make_shared` had; by contrast the constructor call in `new myClass()` is done in the context of the caller itself (here the lambda inside `myClass::getInstance`), not inside the implementation of `new`, so the privilege is retained in that case. In other words `make_shared` forwarding is not perfect: it retains rvalue/lvalue distinction, but not privilege. – Marc van Leeuwen Oct 24 '17 at 07:46
1

Please take VTT's comment seriously.


To your question:

My question is: why new can invoke a private constructor but make_shared not?

The new is actually used within a lambda in a member-function; Well, A lambda defines a local-class, and C++ standards permits a local-class within a member-function to access everything the member-function can access. And its trivial to remember that member functions can access privates..

std::make_shared has no access to the privates of MyClass. It's outside the scope of MyClass


Example:

class X{
    X(){};
public:
    static X* create(){ // This member-function can access private functions and ctors/dtor
        auto lm = [](){ // This lambda inherits the same access of enclosing member-function
               return new X();
        };
        return lm();
    }
};

int main(){
    auto x = X::create();    //valid;
    auto y = new X();        //invalid;
}
WhiZTiM
  • 21,207
  • 4
  • 43
  • 68
0

You should use Meyers singleton instead. You should ensure that your compiler supports C++11 magic static before.

Viktor
  • 1,004
  • 1
  • 10
  • 16