Re: Singleton --- Just Look and give Suggestion's
* 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
particular modern Visual C++ doesn't support it. Use standard <iostream>.
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
--
Due to hosting requirements I need visits to [http://alfps.izfree.com/].
No ads, and there is some C++ stuff! :-) Just going there is good. Linking
to it is even better! Thanks in advance!