2

Here are two singletons, what does make the first one preferable, as both will instantiate only one instance of the corresponding class:

First:

class SharedPointerSingleton  {
public:
      static std::shared_ptr< SharedPointerSingleton> getSingleton(
   {
      if( s_pSingleton== 0 ) s_pSingleton = std::shared_ptr< SharedPointerSingleton>(new SharedPointerSingleton());
      return s_pSingleton;
   }


private:
   SharedPointerSingleton(){};
   static std::shared_ptr< SharedPointerSingleton> s_pSingleton;
};

Second:

class PointerSingleton  {
public:
      static  PointerSingleton * getSingleton(
   {
      if( pSingleton== 0 )  pSingleton =  new PointerSingleton ());
      return  pSingleton;
   }


private:
   PointerSingleton (){};
   static  PointerSingleton *  pSingleton;
};
muaz
  • 550
  • 9
  • 15
  • 3
    You should definitely use Meyers' singleton instead of those 2 solutions. – Rakete1111 Jun 30 '17 at 16:52
  • 1
    If you really want to build a singleton (a lot of people say not to) then this is how you do it: https://stackoverflow.com/questions/17712001/how-is-meyers-implementation-of-a-singleton-actually-a-singleton – NathanOliver Jun 30 '17 at 16:53
  • actually I need to know the advantage of the first one rather than looking for a better solution – muaz Jun 30 '17 at 17:00
  • The destructor is never called in the second. – drescherjm Jun 30 '17 at 17:01
  • Both versions are bad for many reasons. For example, they are not thread-safe, they do not return a reference from `getSingleton` and they use `0` instead of `nullptr`. – Christian Hackl Jun 30 '17 at 17:18

2 Answers2

5

Both implementation have their pros and cons. First solution has overhead of using std::shared_ptr which could be noticeable in some situations. Second solution does not destroy singleton object at the end of program. Though memory would be released by OS at the end of program lifetime it is generally not a good practice not to properly destroy C++ objects. It may directly or indirectly release resources that are not cleaned by OS such as temporary files, shared memory etc and it is a common practice to deallocate resources in destructor.

Slava
  • 43,454
  • 1
  • 47
  • 90
  • The disadvantage of the second solution only applies if the singleton actually holds a resource. It can also be an advantage to avoid order-of-destruction issues with static objects (e.g. different singletons depending on each other). – Christian Hackl Jun 30 '17 at 17:20
  • @ChristianHackl first solution does not have order-of-destruction issue if other static object acquired `std::shared_ptr` of this singleton in advance. If singleton actually holds a resource - that is the probllem. It may not hold resource now, but later such resource can be added either directly or indirectly. And person, who adds that may be easily unaware that destructor never called. – Slava Jun 30 '17 at 17:24
1

The second leaks when the program ends (though the OS should clean up memory, your missing destructor calls might be a problem). That's probably why somebody added a smart pointer on top.

However, both versions are broken, being susceptible to initialization order problems.

Instead, the "proper" way to produce a singleton is like this:

class SharedPointerSingleton
{
   SharedPointerSingleton() = default;

public:
   static SharedPointerSingleton& getSingleton(
   {
      static SharedPointerSingleton instance;
      return instance;
   }
};

This is approach is safe and without the overhead of smart pointers or dynamic allocation. Now the object is constructed precisely when you first ask for it, which is far superior (unless you want it to happen before main … but then you can still instantiate it at namespace scope if you like by calling this function!).

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055