Singleton --- Just Look and give Suggestion's

[Pallav singh]

1. its not executing display() properly ....just look to it
2. Suggest effective way to write MUTEX class at User level


#include <iostream.h>
using namespace std;

template<typename TYPE>
class Singleton
{
private :
Singleton() { cout<<" constructor Invoked "<< endl; }
~Singleton() { cout<<" Destructor Invoked "<<endl; }
Singleton(const Singleton<TYPE> & a)
{ cout<<" Copy Constructor Invoked "<<endl; }

const Singleton<TYPE> & operator = (const Singleton<TYPE> &);

static TYPE * instance;
static volatile long Flag; // Flag is volatile.

public :
static TYPE * GetInstance( void )
{
if( Flag == 0 )
{
// TO BE DONE Guard<Mutex> acquire();
if( Flag == 0 )
{
if( instance != NULL)
{ try
{ instance = new TYPE();}
catch(...)
{ cout <<"Creation of Object failed
"<<endl; }
}
cout<<" Instance Created Successfully \n";

Flag = 1;
// Mutex.release();
}
return instance;
}
else
{
cout<<" Returning the Already Created Instance \n";
return instance;
}
}
};


template<typename TYPE>
TYPE * Singleton<TYPE>::instance = 0 ;

template<typename TYPE>
volatile long Singleton<TYPE>::Flag = 0;

class A
{
public :
int i,j;
A(int i = 1 , int j = 1):i(i),j(i){}

void display()
{ cout<<" value of i "<< i <<" value of j "<< j <<endl; }
};


int main()
{
A * obj1 = Singleton<A>::GetInstance();
A * obj2 = Singleton<A>::GetInstance();
A * obj3 = Singleton<A>::GetInstance();

obj1->display();
//obj2->display();
//obj3->display();

// To check if it call destructor of Object
delete obj1;
delete obj2;
delete obj3;
}

 

[red floyd]

1.   its not executing display() properly ....just look to it

FAQ 5.8. What is the expected behavior, and what behavior are you
seeing?

2.   Suggest effective way to write MUTEX class at User level

There is no Standard way to do that until C++0x. You will have to
look at your OS APIs and see what's available.

#include <iostream.h>

Non-standard header. Use #include <iostream> instead

[remainder redacted]

 

[srdgame]

于 Thu, 05 Mar 2009 08:16:45 -0800，Pallav singh写到：

1. its not executing display() properly ....just look to it 2.
Suggest effective way to write MUTEX class at User level


#include <iostream.h>
using namespace std;

template<typename TYPE>
class Singleton
{
private :
Singleton() { cout<<" constructor Invoked "<< endl; }
~Singleton() { cout<<" Destructor Invoked "<<endl; }
Singleton(const Singleton<TYPE> & a)
{ cout<<" Copy Constructor Invoked "<<endl; }

const Singleton<TYPE> & operator = (const Singleton<TYPE> &);

static TYPE * instance;
static volatile long Flag; // Flag is volatile.

public :
static TYPE * GetInstance( void )
{
if( Flag == 0 )
{
// TO BE DONE Guard<Mutex> acquire();
if( Flag == 0 )
{
if( instance != NULL)
{ try
{ instance = new TYPE();}
catch(...)
{ cout <<"Creation of Object failed
"<<endl; }
}
cout<<" Instance Created Successfully \n";

Flag = 1;
// Mutex.release();
}
return instance;
}
else
{
cout<<" Returning the Already Created Instance \n"; return
instance;
}
}
};


template<typename TYPE>
TYPE * Singleton<TYPE>::instance = 0 ;

template<typename TYPE>
volatile long Singleton<TYPE>::Flag = 0;

class A
{
public :
int i,j;
A(int i = 1 , int j = 1):i(i),j(i){}

void display()
{ cout<<" value of i "<< i <<" value of j "<< j <<endl; }
};


int main()
{
A * obj1 = Singleton<A>::GetInstance(); A * obj2 =
Singleton<A>::GetInstance(); A * obj3 = Singleton<A>::GetInstance();

obj1->display();
//obj2->display();
//obj3->display();

// To check if it call destructor of Object
delete obj1;
delete obj2;
delete obj3;
}

My perferred Singleton way, If I want to make class A to be singleton, I
will declear it as:
class A : public Singleton< A >
{
}

namespace srdgame
{

template <class T>
class Singleton
{
public:
static T* get_instance()
{
return &(get_singleton());
}
static T& get_singleton()
{
static T instance;
return instance;
}
~Singleton(){};
protected:
Singleton(const Singleton& sig);
Singleton& operator = (const Singleton& sig);
Singleton(){};
}
;

 

[Vaclav Haisman]

I suggest you do not use the singleton pattern at all.

Pallav singh wrote, On 5.3.2009 17:16:

1. its not executing display() properly ....just look to it
2. Suggest effective way to write MUTEX class at User level


#include <iostream.h>

using namespace std;

This code will be in header, do _not_ use "using namespace foo;" in headers.
You polute the namespace for all users of your header.

template<typename TYPE>
class Singleton
{
private :
Singleton() { cout<<" constructor Invoked "<< endl; }
~Singleton() { cout<<" Destructor Invoked "<<endl; }
Singleton(const Singleton<TYPE> & a)
{ cout<<" Copy Constructor Invoked "<<endl; }

const Singleton<TYPE> & operator = (const Singleton<TYPE> &);

static TYPE * instance;
static volatile long Flag; // Flag is volatile.

public :
static TYPE * GetInstance( void )
{
if( Flag == 0 )
{
// TO BE DONE Guard<Mutex> acquire();
if( Flag == 0 )
{
if( instance != NULL)
{ try
{ instance = new TYPE();}
catch(...)
{ cout <<"Creation of Object failed
"<<endl; }

Let the execption propagate. Returning NULL here will only misteriously break
things elsewhere.

}
cout<<" Instance Created Successfully \n";

Flag = 1;
// Mutex.release();
}
return instance;
}
else
{
cout<<" Returning the Already Created Instance \n";
return instance;
}
}
};


template<typename TYPE>
TYPE * Singleton<TYPE>::instance = 0 ;

template<typename TYPE>
volatile long Singleton<TYPE>::Flag = 0;

class A
{
public :
int i,j;
A(int i = 1 , int j = 1):i(i),j(i){}

void display()
{ cout<<" value of i "<< i <<" value of j "<< j <<endl; }
};


int main()
{
A * obj1 = Singleton<A>::GetInstance();
A * obj2 = Singleton<A>::GetInstance();
A * obj3 = Singleton<A>::GetInstance();

obj1->display();
//obj2->display();
//obj3->display();

// To check if it call destructor of Object
delete obj1;
delete obj2;
delete obj3;
}

Your class A is not singleton in itself; I can still write "new A" anywhere
else in the code. That you use Singleton<A> does not imply/force single
instance of A.

Singleton is glorified memory leak. Your A's dtor will never be called, bad.

You cannot use your singleton in other code that runs during global/static
variables initialization, if you intend to have global/static mutex to
synchronize the singleton instance creation. That limits its use.

Using template like that or anything similar also makes your singletons not
really having single instance when DLLs on Windows.

Bottom line, singleton is bad idea. Properly managed life time of globally
visible pointer to A is much much better.

 

[Alf P. Steinbach]

* Pallav singh:

1. its not executing display() properly ....just look to it

As others have remarked, this description is a little to vague to give any sound
advice.

2. Suggest effective way to write MUTEX class at User level

The best way is to find someone else's implementation. I'd start by checking
whether Boost has a Mutex class.

#include <iostream.h>

This is an old, pre-standard header, not supported by all compilers. In

using namespace std;

Don't put a blanket 'using namespace std;' in a header.

template<typename TYPE>

Preferentially use all uppercase names for macros, and for macros /only/.

There is an idiomatic naming convention for template type parameteres where
they're called 'T', 'U' and so on.

But that convention does (of course) not extend to multi-character names.

class Singleton
{
private :
Singleton() { cout<<" constructor Invoked "<< endl; }
~Singleton() { cout<<" Destructor Invoked "<<endl; }
Singleton(const Singleton<TYPE> & a)
{ cout<<" Copy Constructor Invoked "<<endl; }

const Singleton<TYPE> & operator = (const Singleton<TYPE> &);

static TYPE * instance;
static volatile long Flag; // Flag is volatile.

*DO NOT* rely on 'volatile' for thread safety.

It is *extremely* unsafe and misleading, it adds no thread safety whatsoever
unless your compiler specifically documents such a guarantee (whatever it is).

That said, it's a good idea to use self-describing names. 'Flag' is not
self-describing. It tells the reader nothing correct, and it implies that this
is a boolean, which is contradicted by the type 'long'. You could as well, and
better!, have used the name 'gnorble'. At least it communicates, to some degree,
that it doesn't say anything, as opposed to imparting an incorrect impression.

public :
static TYPE * GetInstance( void )

First, a singleton is supposed to exist, so return by reference, not a pointer.
The pointer implies that one may get out a nullpointer. That should never happen.

Second, 'void' as formal argument is a C'ism: it's distracting noise to most C++
programmers.

I know that at least one competent person frequenting this group prefers that
notation, but he also prefers some other notation that's generally frowned on by
most, so chances are that he reads code in a way that's slightly different from
the way most do. Source code is about communicating to other programmers. Of
course, if you work in an environment where this is very common, used by most,
then go with the flow, for again, source code is about communicating.

{
if( Flag == 0 )
{
// TO BE DONE Guard<Mutex> acquire();
if( Flag == 0 )
{

Read up on double-checked locking pattern. It is notoriously unreliable. It was
all the rage at one time, middle 1990's, but has since been shown to be
problematic -- for a novice it is a case of Evil Premature Optimization.

if( instance != NULL)
{ try
{ instance = new TYPE();}
catch(...)
{ cout <<"Creation of Object failed
"<<endl; }
}

Others have commented on this, but just for completeness: do not swallow that
exception.

cout<<" Instance Created Successfully \n";

Flag = 1;

Uhm, did you really mean for 'Flag' to be a 'bool'? Now I'm confused... Which is
not very surprising: use proper types and self-describing names.

// Mutex.release();
}
return instance;
}
else
{
cout<<" Returning the Already Created Instance \n";
return instance;
}
}
};


template<typename TYPE>
TYPE * Singleton<TYPE>::instance = 0 ;

template<typename TYPE>
volatile long Singleton<TYPE>::Flag = 0;

class A
{
public :
int i,j;
A(int i = 1 , int j = 1):i(i),j(i){}

void display()
{ cout<<" value of i "<< i <<" value of j "<< j <<endl; }
};


int main()
{
A * obj1 = Singleton<A>::GetInstance();
A * obj2 = Singleton<A>::GetInstance();
A * obj3 = Singleton<A>::GetInstance();

obj1->display();
//obj2->display();
//obj3->display();

// To check if it call destructor of Object
delete obj1;
delete obj2;
delete obj3;
}

Those 'delete' statements, with Undefined Behavior, constitute another example
why returning a pointer to the singleton is not a good idea. Return a reference.

Anyways, a reasonable approach for a thread-safe singleton can be to create it
before any threads (in addition to main thread) are created.

If it really needs to be created on demand, that is, creation is very costly and
it's not guaranteed that it will be created in any given run of the program and
there are a lot of these beasts (design smell), then proper synchronization is
needed. For that you need a general mutex and/or atomic operations. That's
currently not provided by standard C++, but is available via many third party
libraries (I think including Boost, not sure), which is what you should use
instead of rolling your own. By the way, "Modern C++ Design" by Andrei
Alexandrescu discusses singletons and provides generic implementations for a
large variety of requirements. And I suggest studying those (I think, but not
sure, that they're available via the Loki library).


Cheers & hth.,

- Alf

 

[anon]

I suggest you do not use the singleton pattern at all.

Pallav singh wrote, On 5.3.2009 17:16:

This code will be in header, do _not_ use "using namespace foo;" in headers.
You polute the namespace for all users of your header.

Let the execption propagate. Returning NULL here will only misteriously break
things elsewhere.


Your class A is not singleton in itself; I can still write "new A" anywhere
else in the code. That you use Singleton<A> does not imply/force single
instance of A.

True. There are better ways to implement singleton

Singleton is glorified memory leak. Your A's dtor will never be called, bad.

It will be called at the process end

You cannot use your singleton in other code that runs during global/static
variables initialization, if you intend to have global/static mutex to
synchronize the singleton instance creation. That limits its use.

Using template like that or anything similar also makes your singletons not
really having single instance when DLLs on Windows.

I agree - windows is shit.

Bottom line, singleton is bad idea. Properly managed life time of globally
visible pointer to A is much much better.

Then why are singleton questions appearing in this NG all the time?
It's uses are limited, but it is a very good idea

 

[Noah Roberts]

Singleton is glorified memory leak. Your A's dtor will never be called, bad.

No reason why it cannot be.

Furthermore, this is not a "memory leak". A memory leak is the
continued acquiring and non-release of memory by a continuously running
program.

You cannot use your singleton in other code that runs during global/static
variables initialization, if you intend to have global/static mutex to
synchronize the singleton instance creation. That limits its use.

Again, only applies to certain implementations.

Using template like that or anything similar also makes your singletons not
really having single instance when DLLs on Windows.

Bottom line, singleton is bad idea. Properly managed life time of globally
visible pointer to A is much much better.

And a singleton does exactly that.

 

[Matthias Buelow]

Then why are singleton questions appearing in this NG all the time?

Spill over from Java?

C++ has global variables; singletons are just a very bureaucratic way of
achieving the same thing.

 

[Vaclav Haisman]

anon wrote, On 6.3.2009 8:41:

Vaclav Haisman wrote: [...]

Singleton is glorified memory leak. Your A's dtor will never be
called, bad.

It will be called at the process end

Not in his implementation as he posted it.

I agree - windows is shit.

The same is AFAIK true on any *nix if you use ELF symbols visibility feature
hiding symbols not explicitelly exported, which seems to be growing trend
these days.

Then why are singleton questions appearing in this NG all the time?
It's uses are limited, but it is a very good idea

Because it has been popularized by the gang of four's Design Patterns book
and because those who use it do not know any better. Popular <=/=> good.

 

[James Kanze]

Spill over from Java?

C++ has global variables; singletons are just a very
bureaucratic way of achieving the same thing.

A singleton is a type, not a variable or an object. The concept
is perhaps even more relevant in C++ than in Java, because
managing order of initialization can be piggy-backed onto it.

 

[James Kanze]

1. its not executing display() properly ....just look to it
2. Suggest effective way to write MUTEX class at User level

#include <iostream.h>
using namespace std;

template<typename TYPE>
class Singleton
{
private :
Singleton() { cout<<" constructor Invoked "<< endl; }
~Singleton() { cout<<" Destructor Invoked "<<endl; }
Singleton(const Singleton<TYPE> & a)
{ cout<<" Copy Constructor Invoked "<<endl; }
const Singleton<TYPE> & operator = (const Singleton<TYPE> &);

A singleton should not support copy, and since there can never
be more than one of them, there's no point in supporting
assignment, either.

static TYPE * instance;
static volatile long Flag; // Flag is volatile.

What on earth for?

public :
static TYPE * GetInstance( void )
{
if( Flag == 0 )
{
// TO BE DONE Guard<Mutex> acquire();
if( Flag == 0 )
{
if( instance != NULL)
{ try
{ instance = new TYPE();}
catch(...)
{ cout <<"Creation of Object failed
"<<endl; }
}
cout<<" Instance Created Successfully \n";

Flag = 1;
// Mutex.release();
}
return instance;
}
else
{
cout<<" Returning the Already Created Instance \n";
return instance;
}
}
};

The comments about mutexes above are misleading; even with the
mutexes, the code isn't thread safe. Which generally isn't a
problem; just ensure that instance() is called at least once
before threading starts. Of course, the above can be written
much simpler:

static Type& instance()
{
if ( ourInstance == NULL ) {
ourInstance = new Type ;
}
return *ourInstance ;
}

This is far simpler, and works just as well as what you have
written.

template<typename TYPE>
TYPE * Singleton<TYPE>::instance = 0 ;

If I want to use the code in a multithreaded environment, I'd
write:
template< typename Type >
Type* Singleton< Type >:urInstance
= & Singleton< Type >::instance() ;
This will ensure that instance() is called at least once before
entering main (and thus, normally, before threads are started).

template<typename TYPE>
volatile long Singleton<TYPE>::Flag = 0;

As mentionned above, the variable isn't needed, and the volatile
doesn't affect anything. And using a long for something which
can only take two values, 0 and 1, is a bit strange as well.

class A
{
public :
int i,j;
A(int i = 1 , int j = 1):i(i),j(i){}

void display()
{ cout<<" value of i "<< i <<" value of j "<< j <<endl; }
};

int main()
{
A * obj1 = Singleton<A>::GetInstance();
A * obj2 = Singleton<A>::GetInstance();
A * obj3 = Singleton<A>::GetInstance();

obj1->display();
//obj2->display();
//obj3->display();

// To check if it call destructor of Object
delete obj1;
delete obj2;
delete obj3;
}

What's the relationship of A to Singleton? A isn't a singleton,
so delete of an A* is perfectly fine. A is NOT a singleton.
And since obj1, obj2 and obj3 all point to the same object, the
second and third deletes are undefined behavior.

 

[James Kanze]

于 Thu, 05 Mar 2009 08:16:45 -0800，Pallav singh写到：

[...]

My perferred Singleton way, If I want to make class A to be
singleton, I will declear it as:
class A : public Singleton< A >
{
}

With a semicolon after the }, of course.

This is more or less the standard idiom, of course.

namespace srdgame
{

template <class T>
class Singleton
{
public:
static T* get_instance()
{
return &(get_singleton());
}
static T& get_singleton()
{
static T instance;
return instance;
}
~Singleton(){};
protected:
Singleton(const Singleton& sig);
Singleton& operator = (const Singleton& sig);
Singleton(){};}
;

This has the disadvantage that the singleton will be destructed
some time when static objects are destructed. This usually
isn't what is wanted.

My own singleton uses a policy to control this:

<code>
// DestructionPolicy:
// ==================
//
//! Determines whether the destructor will be called on exit or
//! not. (Unless there is a very strong reason to call it, it is
//! recommended that it not be called, so as to avoid order of
//! destruction issues.)
//
---------------------------------------------------------------------------
enum DestructionPolicy
{
neverDestruct,
destructOnExit
} ;

// Singleton:
// ==========
//
//! Makes a singleton for UserClass. If UserClass should always
//! be a singleton, it can derived from this template class,
//! declaring this template class as friend and its constructor as
//! private.
//!
//! This implementation is thread safe if and only if the first
//! call to instance takes place before threading starts. This
//! template ensures that the constructor is called at least once
//! during static initialization; on normal implementations, this
//! means that the singleton will be thread safe if threading is
//! not started before main.
//
---------------------------------------------------------------------------
template< typename UserClass, DestructionPolicy dtorPolicy =
neverDestruct >
class Singleton
{
public:
static UserClass& instance() ;

private:
static UserClass* ourInstance ;

template< DestructionPolicy discrimPolicy >
class Discrim {} ;
static UserClass* createInstance( Discrim< neverDestruct > ) ;
static UserClass* createInstance( Discrim< destructOnExit > ) ;
} ;

template< typename UserClass, DestructionPolicy dtorPolicy >
UserClass* Singleton< UserClass, dtorPolicy >::
ourInstance
= &Singleton< UserClass, dtorPolicy >::instance() ;

template< typename UserClass, DestructionPolicy dtorPolicy >
UserClass&
Singleton< UserClass, dtorPolicy >::instance()
{
if ( ourInstance == NULL ) {
ourInstance = createInstance( Discrim< dtorPolicy >() ) ;
}
return *ourInstance ;
}

template< typename UserClass, DestructionPolicy dtorPolicy >
UserClass*
Singleton< UserClass, dtorPolicy >::createInstance(
Discrim< neverDestruct > )
{
return new UserClass ;
}

template< typename UserClass, DestructionPolicy dtorPolicy >
UserClass*
Singleton< UserClass, dtorPolicy >::createInstance(
Discrim< destructOnExit > )
{
static UserClass theOneAndOnly ;
return &theOneAndOnly ;
}
</code>

Like yours, it's meant to be derived from. This seemed so
obvious to me that I forgot to document it. (One should
probably also assign a UserClass* to a Singleton< UserClass >*
somewhere in the code, so that it won't compile *unless*
UserClass derives from it.)

 

[James Kanze]

Not knowing what problem he's trying to solve, I wouldn't go
that far. I use it from time to time.

[...]

It will be called at the process end

In his code, it will be called three times at the end of main.
On the same object. In a correctly implemented singleton, it
may or may not be called, depending on the implementation. Most
of the time, it is preferable not to call it.

You could use a statically initialized mutex, at least under
Unix. I', not sure how Windows programmers handle this problem,
but Boost threads has once, so there must be some solution.

Normally, the simplest solution is just to ensure that the
instance() function is called at least once before threading
start.

I agree - windows is shit.

It has nothing to do with Windows. You can ensure that there is
only a single instance in Windows, if that's what you need, or
you can end up with multiple instances in Unix, if that's what
you need (e.g. a plugin, where each plugin really operates in
its own environment).

Then why are singleton questions appearing in this NG all the
time? It's uses are limited, but it is a very good idea

It's a tool. Sometimes useful, sometimes abused. The reason
questions about it appear so often is probably because it is the
simplest pattern in the GoF book, so beginners start with it.

 

[James Kanze]

I couldn't put my finger on it before, but I think you've just
named the problem with Singleton. Well, not the problem,
exactly; the problem is that Singleton is over-used, and that
developers often call things Singletons that really aren't.

Singleton is a design pattern. Either a type or an object may
fit that pattern, or some other entity yet to be imagined.

It's obviously a question of definition, but according to the
GoF, the intent of a singleton is to "ensure a class only has
one instance, and provide a global point of access to it." What
makes a singleton a singleton is the fact that there can only be
one instance of the type.

Of course, it's a free country, and like Humpty-Dumpty, you can
use the word to mean exactly whatever you want it to mean. But
using a meaning other than the generally accepted one isn't
going to help communications.

As for being overused... I haven't found this to be the case in
production code. It tends to be over-talked about, because it
is in many cases the first example of a design pattern that
people abord. But I'm not sure it's overused.