It's not really a good singleton implementation if the constructor is public....I had to make the constructor private and use friends to catch that mistake.
It's not really a good singleton implementation if the constructor is public....I had to make the constructor private and use friends to catch that mistake.
Of course not. But the singleton factory also had a public constructor. I didn't think anyone was stupid enough to actually create one o_O
It just shows - take nothing for granted.
Originally Posted by Adak
It was laserlight wasn't it?
Soma
The singleton pattern is a known pattern for this and as such I would certainly use it. Any other code to address this is probably fraught with issues. The only time I've had problems with singletons is with DLLs and in multi-threaded apps. Both issues can be overcome though quite easily.
Writing some off the wall code to accomplish what a singleton does, does not seem to me to be a good practice.
The idea behind Elysia's approach is that the class has a single responsibility (doing its thing) and the factory has another (ensuring how many are created). The main difference is that you get the uniqueness guarantee (what singleton is about but which is hardly ever essential) without making a global instance (why people, including OP, mostly want to use singletons).
So the singleton pattern ensures that you have only one instance. However, as with global, after calling GetInstance() you have really no idea whether it is OK to use it the way you do. May-be the object has been previously put into a specific state, so that only certain actions are valid now? Singleton may solve a single problem but may have the tendency to introduce innumerable other problems.
Last edited by anon; 04-13-2009 at 05:17 PM.
I might be wrong.
Quoted more than 1000 times (I hope).Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
Case in point. What happens in this section of code if two threads pass the if conditional?
Lets say that thread A gets past the if conditional but then gets task switched. Now thread B comes in and also gets passed the if and then gets task switched. Now thread A picks up at the new and returns and then gets task switched. Thread B will resume at the new past the if conditonal and as such will create another instance of the object. Thus we have just created two instances of an object in code that was supposed to guarantee we only create one instance. As such this code does not guarantee one instance of an object in a multi-threaded environment.Code:static T* Create() { assert(m_Instances == 0); // Allow only one instance of the object if (m_Instances >= 1) return NULL; T* p = new T; m_Instances++; return p; }
Though I agree with the general sentiment of "if it should be a singleton allowing more than one instance is crazy", criticizing the code because of a threading issue that can't be solved within standard C++ is also crazy.
Soma
But it can be solved. The only way to ensure 1 instance via C++ is to have a CreateInstance() and GetInstance(). The CreateInstance() is called during some application startup code so that the instantiation is not determined by who calls GetInstance() first and thus is not prone to out of order instantiation errors or early instantiation. Lazy instantiation is very prone to these types of errors. GetInstance() in this case would only return the instance that was created.Though I agree with the general sentiment of "if it should be a singleton allowing more than one instance is crazy", criticizing the code because of a threading issue that can't be solved within standard C++ is also crazy.
And I'm not criticizing the code. But if the comment in the code reads this will create only 1 instance when there are clear instances when it will not, to me that is a bug waiting to happen. This can be a serious problem in a multi-threaded environment and could mean the difference between a device listener having the right object to listen to. If you created two device objects by mistake then theoretically half of it's listeners could attach to the wrong object thus causing them to miss very important calls. Granted I work in an embedded environment where this can bring the system crashing down hard but it could be an issue in other types of code as well.
Last edited by VirtualAce; 04-13-2009 at 05:53 PM.
That works, the cleaner solution is to use pthread_once or some similar thing in your MT lib.
So the singleton-MT problems are solved. But the trouble goes on if you ever start trying to refactor your singleton class into an .dll/.so or access your singleton from a .dll/.so. What do the static vars do? Have they still a single instance or do they get doubled/tripled/n'led? Imo there is no portable solution, so get rid of singletons in C++, just create one object, pass it's reference through your program like anything else, add a line of documentation and save you a lot of headache.
Last edited by pheres; 04-14-2009 at 12:37 AM.
How is creating one object somewhere at the beginning and passing it around different than initializing a singleton somewhere at the beginning of the program? Other than that the singleton doesn't, well, have to be passed around everywhere.
If some leaf or near-leaf class uses the singleton, it's easy to decouple if you later decide it doesn't need the singleton anymore. If you're passing around a reference everywhere, that refactoring could have a ripple effect.
Like anything, Singletons aren't a perfect solution to every problem and can be abused, but I largely prefer them than passing some master object (that's only protected from multiple instances by documentation) through myriad classes that are merely acting as "middle men".
Hiding dependencies is a big difference and disadvantage (except perhaps in rare cases).How is creating one object somewhere at the beginning and passing it around different than initializing a singleton somewhere at the beginning of the program? Other than that the singleton doesn't, well, have to be passed around everywhere.
Perhaps you may decide that you don't want logging (but then there should be better ways of disabling it rather than rewriting code) but how would you, for example, imagine a CWorld instance becoming unnecessary?If some leaf or near-leaf class uses the singleton, it's easy to decouple if you later decide it doesn't need the singleton anymore. If you're passing around a reference everywhere, that refactoring could have a ripple effect.
A singleton / global is probably a quick way to hack on new features if the interfaces aren't flexible enough for the new requirements. Isn't it nice that you don't have to change a bunch of signatures and calls to make more information available to things? The price is degradation of code which probably makes each new change more difficult.
A master object is probably another code smell (God object) and there might still be too high coupling. I guess many of the other classes only need to know about some part of the master object which therefore should be broken up, so each part of the code can reference exactly what it needs and not more.Like anything, Singletons aren't a perfect solution to every problem and can be abused, but I largely prefer them than passing some master object (that's only protected from multiple instances by documentation) through myriad classes that are merely acting as "middle men".
I might be wrong.
Quoted more than 1000 times (I hope).Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
various problems (regarding multi threading/initialization or access over shared library boundaries) are described inside this thread. In a bigger project with lot of programmers and a bunch of interdependent modules these problems can strike you if you don't expect it. Then refactoring from the singleton pattern to "reference passing" will be your smallest problem.
A single instance of a non-singleton still has to have its creation managed somewhere in the program, and is subject to the same synchronization issues in a MT environment, no?
Cannot the same problem occur (more easily perhaps) with a single object passed by reference? What's to stop a programmer from constructing one where he thinks he needs it?In a bigger project with lot of programmers and a bunch of interdependent modules these problems can strike you if you don't expect it.
Last edited by medievalelks; 04-14-2009 at 09:24 AM.
Isn't a key to MT to minimize the amount of shared data between the threads? As such wouldn't anything of which only one instance can exist be rather unsuitable to be used across threads?
At least the dependencies are more visible. As to constructing more than one (CWorld), you'd probably notice and you can limit construction...Cannot the same problem occur (more easily perhaps) with a single object passed by reference? What's to stop a programmer from constructing one where he thinks he needs it?
I might be wrong.
Quoted more than 1000 times (I hope).Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
And of course, if there is and should only ever ONE, there are two obvious solutions to avoid it being created in multiple threads:
1. Create it before threads are started, in the main thread.
2. Create it on the stack in the main function.
Both or either can be used.
Assuming that the class object isn't large, this should work fine, and avoids all sorts of problems.
I agree that sharing should be avoided in a MT environment, as all shared data that is ever updated by any thread will be subject to locks to ensure that no other thread is getting inconsistent data.
--
Mats
Compilers can produce warnings - make the compiler programmers happy: Use them!
Please don't PM me for help - and no, I don't do help over instant messengers.
