Singleton Code review

[Moonraker101]

Is the following a good code implementation of a singleton

  
class Singleton
{
public:
	static Singleton  * getInstance();
	void showMessage();
	~Singleton();

private:
	static Singleton * Instance;
	Singleton();
	

};

Singleton * Singleton::Instance = nullptr;  

Singleton::Singleton()
{
	
}

Singleton *  Singleton::getInstance()
{
	Instance = new Singleton;
	return Instance;
}

void Singleton::showMessage()
{
   cout << "Hello World, this is a Singleton Pattern" << endl;
}

int main()
{

	Singleton * object = Singleton::getInstance();
	
	object->showMessage();
	

	system("Pause");
  return 0;
}

Whats really making me nervous is the new operator, and the possibility of a memory leak.

Could the following be a better C++ implementation:

class Singleton
{
public:
	static Singleton * getInstance();
	void showMessage();
	

private:
	Singleton();
	static Singleton * Instance;
};

Singleton * Singleton::Instance = nullptr;

Singleton * Singleton::getInstance()
{

	return Instance;
}

void Singleton::showMessage()
{
   cout << "Hello World, This is a singleton" << endl;
}


int main()
{

	Singleton * object = Singleton::getInstance();

	object->showMessage();

	system("Pause");
	return 0;
}

What are your thoughts?

[JLBorges]

Implementing a simple Meyer's singleton is quite straightforward:

struct a_singleton
{
    // favour reference semantics over pointer semantics
    public: static a_singleton& instance() ;

    // ...

    private: a_singleton( /* ... */ ) { /* ... */ }; // private construct

    a_singleton( const a_singleton& ) = delete ; // non-copyable
    a_singleton( a_singleton&& ) = delete ; // and non-movable

    // as there is only one object, assignment would always be assign to self
    a_singleton& operator= ( const a_singleton& ) = delete ;
    a_singleton& operator= ( a_singleton&& ) = delete ;

    // ...
};

a_singleton& a_singleton::instance()
{
    static a_singleton meyers_singleton { /* ... */ } ;
    return meyers_singleton ;
}

However, see: http://www.cplusplus.com/forum/general/145647/

[Moonraker101]

Thanks for the tip JLBorges, I appreciate it. I will use the Meyer Singleton going forward.


Is there anything specifically wrong with the second implementation that I posted?

[JLBorges]

> Is there anything specifically wrong with the second implementation that I posted?

This pointer Singleton * Singleton::Instance = nullptr; is never set to point to an actual object,

Asssuming that that is fixed (at the point of definition: avoiding the order of initialisation fiasco. In the function getInstance() when it is called for the first time: avoiding race conditions)

The class Singleton is copyable; so its singleton-ness is not enforced.

The singleton object needs to be destroyed when the program ends; if it is an object with a dynamic storage duration, it needs to be wrapped in a unique pointer.

std::unique_ptr<Singleton> Singleton::Instance = std::make_unique<Singleton>( /* ... */ ) ;

Singleton * Singleton::getInstance() { assert(Instance) ; return Instance.get() ; }

Ideally, Singleton::getInstance() should return a reference to the object (ownership is not transferred to the caller and an object is always available).

[Moonraker101]

Just realized your implementation is very C++11 heavy. Tried running the program and I keep getting these errors.

Error	1	error C2059: syntax error : ';'	
Error	3	error C2059: syntax error : ';'	
Error	5	error C2059: syntax error : ';'	
Error	7	error C2059: syntax error : ';'	
Error	2	error C2238: unexpected token(s) preceding ';'	

Error	4	error C2238: unexpected token(s) preceding ';'
Error	6	error C2238: unexpected token(s) preceding ';'
Error	8	error C2238: unexpected token(s) preceding ';'

I mostly use Visual Studio 2012.


or could it simply be I messed up somewhere

#include <iostream>

struct Singleton
{
public:   
	static Singleton& getInstance();
	void showMessage();
	int i;

private: 
	Singleton(){i = 7; };
	
	Singleton(const Singleton &) = delete;  
	Singleton(Singleton&&) = delete;        
	
	
	Singleton& operator=(const Singleton&) = delete;
	Singleton& operator=(Singleton&&) = delete;
	
};

Singleton & Singleton::getInstance()
{
  static Singleton  Instance;
  return Instance;
}

void Singleton::showMessage()
{
   std::cout << "Hello World, this is a Meyer Singleton Design Pattern" << std::endl;
}

int g = Singleton::getInstance().i;

int main()
{

	std::cout << g << std::endl;

	system("Pause");
  return 0;
}

[JLBorges]

Legacy C++:

#include <iostream>
// #include <boost/noncopyable.hpp>

struct Singleton // : private boost::noncopyable
{
public:   
    static Singleton& getInstance();
	void showMessage() const ; // ****
	int i;

private: 
	Singleton(){i = 7; };

 	// Singleton(const Singleton &) = delete;  
	// Singleton(Singleton&&) = delete; 
        Singleton(const Singleton &) ; // declared private, not defined
	
	// Singleton& operator=(const Singleton&) = delete;
	// Singleton& operator=(Singleton&&) = delete;
	Singleton& operator=(const Singleton&) ; // declared private, not defined

       // inheriting (privately) from boost::noncopyable is a convenient alternative
};

const int bg = Singleton::getInstance().i ;

Singleton & Singleton::getInstance()
{
  // caveat: legacy c++ is not concurrency aware
  // there could be a race-condition if multiple threads are involved
  static Singleton  Instance; 
  return Instance;
}

void Singleton::showMessage() const // ****
{
   std::cout << "Hello World, this is a Meyer Singleton Design Pattern\n" ; // << std::endl;
}

int g = Singleton::getInstance().i;

int main()
{

	std::cout << bg << ' ' << g << '\n' ; // std::endl;
        Singleton::getInstance().showMessage() ;

	// system("Pause");
    // return 0;
}

http://coliru.stacked-crooked.com/a/c96d1ec9b1d200f0

[Moonraker101]

Thanks again for the help JLBorges. Works perfectly.