1

If I return a reference to my instance of MySingleton from GetInstance() instead of the pointer in my code below, it will be valid, but is using the pointer in this way also valid?

I assume my pointer will only get initialized once because it is static, but not not sure.

I know the correct way by using raw pointers within a singleton is to check if the pointer is first null before assigning it, but wondering if the below code is also valid. Thanks

class MySingleton
{
    public:
    static MySingleton* GetInstance()
    {
        static MySingleton* inst = new MySingleton();
        return inst;
    }

    private:
    MySingleton(){};

};

EDIT: I haven't seen this exact implementation for a Singleton implemented in the reported duplicate question

Engineer999
  • 3,683
  • 6
  • 33
  • 71
  • What's the plan for freeing the memory allocated ? – P0W Apr 10 '18 at 10:35
  • 1
    Possible duplicate of [C++ Singleton design pattern](https://stackoverflow.com/questions/1008019/c-singleton-design-pattern) – JHBonarius Apr 10 '18 at 10:42
  • 1
    Also consider [this](https://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons) – JHBonarius Apr 10 '18 at 10:43
  • just google singleton pattern and try to understand why your code is not correct. – Arkady Godlin Apr 10 '18 at 10:44
  • @ArkadyGodlin . I've googled and don't see a singleton implemented in this way, which is why i'm asking, and trying to understand , if it is wrong, why. I currently don't see why this is incorrect. The singleton object is allocated on the heap. The address is returned from the function. As the pointer is a static variable, i think the pointer should be only initialized once, hence it is fine. Correct me if i'm wrong and explain why, which is why i posted this – Engineer999 Apr 10 '18 at 11:40
  • Also please consider, if I am actually implementing this now in a project for example, I will use a design pattern which works, such as just using a reference. I am in a process of deep learning C and C++ and trying to understand the reasons why code that appears to me to be fine, is not. It doesn't necessarily mean that I will go ahead and implement it in this way. Thanks. – Engineer999 Apr 10 '18 at 11:44
  • @P0W I should free the the memory in the destructor – Engineer999 Apr 10 '18 at 11:53

2 Answers2

0

See std::call_once and std::once_flag for implementing singleton pattern. Your implementation will lead to troubles in a multithreaded environment.

Most of is seem to have missed the simplest possible form of C++ singleton AKA Scott Meyer's Singleton. Ever since C++11 it has become as easy as a pie:

class singleton{
    singleton();
    singleton(singleton&&)=delete;
    singleton(singleton const&)=delete;
    void operator(singleton&&)=delete;
    void operator(singleton const&)=delete;
//...
public:
    static auto& singleton::instance(){
            static singleton val{};
            return val;
        }
};

No need for complex code.

Red.Wave
  • 2,790
  • 11
  • 17
  • Oh really? Static variables have been initialized in thread-safe mannet since as early as C++11. – bipll Apr 10 '18 at 13:12
0

Your implementation is acceptable (as is any other) as long as any one of the following holds:

  1. Your singleton does not acquire any external resources (anything: file handles, network connections, shared memory) for longer than a single exception-safe method invocation.

  2. The last user of your singleton deletes the instance manually.

Otherwise something serious can leak.

You can wrap the instance in something automatic, like std::unique_ptr, but this immediately casts usual singleton lifetime issues.

bipll
  • 11,747
  • 1
  • 18
  • 32