Rule of thumb: If the “right” way to do something takes longer to write, is harder to read & maintain the then “wrong” way. Remember that the people that define the “right” way don’t really write software.
Wooo! Threadomancy!
I’ve been doing this with the global managers in my application:
public class GlobalManager
{
// Singleton Pattern ( Double Checked Lazy Loading )...
private static GlobalManager getInstance() { return instance; }
// Stuff...
public static void staticMethod()
{
GlobalManager.getInstance().internallMethod();
}
private void internalMethod()
{
// Do stuff...
}
}
Note that the getInstance method is private. The idea is for the access methods to be static, so externally there is no need to call the getInstance method, since the classes are meant to be unique and global all the time.
For example, for Logging, instead of this:
Logger.getInstance().Debug( "I'm logging!" );
Logger.getInstance().Error( "Like a BOSS!" );
I have this:
Logger.Debug( "I'm logging!" );
Logger.Error( "Like a BOSS!" );
Which is technically the same, but feels more concise, and cleaner.
My question… What horrible pitfalls await me?
Oh, and calling getInstance or the static methods from the internal methods I already figured out can lead to ugly times. Example:
//...
private static GlobalManager getInstance()
{
if(instance == null) instance = new GlobalManager();
return instance;
}
private GlobalManager()
{
init();
}
//....
public static void init() // static method
{
GlobalManager.getInstance().doInit(); //<- EVIL HEAP-KILLING LOOP OF EVIL!
}
private void doInit() // internal method
{
//...
}
Which is solved by making sure the static interface methods are never called from inside.
Sooooo, anything else I need to be wary of? Is this horribly inefficient? All I get from the net is comparisons of static vs singleton, but no comments on the mutant cross-breed I’m using. :clue:
aaaaaaaaaaaaaah
kill it, kill it with fire
so first you had a bunch of global functions,
as a next step you bundled them in a class as methods,
which you then exposed as static functions again, so back to square one.
so you made your stuff even more error prone and unreadable, congratulations^^
Ps:
instead of doing this, which isn’t even thread safe
private static GlobalManager getInstance()
{
if(instance == null) instance = new GlobalManager();
return instance;
}
you should do this for lazy initialisation of your Globaltons
public class GlobalManager
{
private static class Lazy
{
private static final GlobalManager instance = new GlobalManager();
}
private static GlobalManager getInstance()
{
return Lazy.instance;
}
}
and this should only be done if you have other stuff in this class which can be used without the need of the instance.
For your normal Globalton you should not need any lazy initialization stuff. So when you really need a Globalton, just creat a private constructor and one
public static final instance field.
Errrrr, sorry if the example wasn’t clear, but no, these classes are not just a conglomeration of static global functions, they actually need an instance.
For example, the Logger class keeps track of registered log appenders (console, files, window components, etc…) to log to, so it cannot be implemented as simple System.out.prinln calls.
As for the Singleton initialization, I actually use the more classic:
// Singleton Pattern ( Double Checked Lazy Loading )----------------------------
public class DCLSingleton
{
private static volatile DCLSingleton instance = null;
/** @return Returns the Singleton Logger instance. */
public static DCLSingleton getInstance()
{
if(instance == null)
{
synchronized(DCLSingleton.class)
{
if(instance == null)
instance = new DCLSingleton();
}
}
return instance;
}
private DCLSingleton()
{
if(instance != null)
throw new IllegalStateException( "Singleton Already Instanced!" );
}
}
I am aware of the “instance-holder class” pattern you have referenced, it’s just that I’m not entirely sure why it works (In the method I use, I understand why having the instance be volatile and synchronizing the instantiation help solve related concurrency issues)… so an explanation would be awesome, since it is a more elegant way to solve it.
I also considered using enums as my Singleton containers:
public enum EnumSingleton
{
INSTANCE;
private String my_hello = "Hey there World!";
public String sayHi() { return my_hello; }
}
// ....
EnumSingleton.INSTANCE.sayHi();
But I didn’t see clearly how to instantiate said Singleton with specific properties (say, system properties at runtime), and I really wanted to try my static access pattern, which isn’t possible since there is no way to hide the INSTANCE element from the enum, unless the whole enum is encapsulated within a container class… Which may be way too convoluted.
To summarize, all I’m doing is encapsulating the Singleton instance calls inside static interface methods. Internally it is a regular Singleton.
Oh, and in case someone wonders (And to prevent a “why do this?!” discussion) I’m using Singleton classes to wrap Java specific elements. The notion is to decouple the program’s inner logic from Java specifics as much as possible to facilitate porting to C++ later on.
What is the advantage of going this rather weird static singleton way instead of just separating implemention details by using interfaces and concrete implementation classes ?
Uh… No concrete advantage.
Apart form the goal of having all the specifics packed into a concrete namespace, I’m just having fun with the implementation, experimenting and learning. ;D
For me, it is more comfortable to treat System classes as static elements, and so far I haven’t found a solid reason not to (hence why I’m asking about pitfalls).
Oh, and the reason I’m posting here is because I’m coding my application’s screen manager currently and came here looking for ideas (maybe it’d be better to split this into a “Singleton Madness” thread?)
Class loading specification.
I know I know, I’m reading on it.
Edit: Ok, I think I get it. My problem was that I wasn’t seeing how the pattern was doing lazy initialization when the instance attribute instantiation was being hard-coded… Hadn’t realized that the loading of the internal static InstanceHolder class doesn’t happen until the wrapping class itself is used, resulting in a lazy initialization that the class loader itself makes sure will be thread safe.
Or that’s what I’m getting out of it, anyway. :clue:
While you’re reinventing a DI container with this GlobalManager nonsense, you may as well just use one. No one is a better coder for hand-writing half-assed solutions to long-solved problems. I’d go with Guice, since it’s very well-tested and closest to a standard (JSR330) but if you want the simplicity of just asking for an instance explicitly, then give PicoContainer a shot.
Thanks for the suggestion, but I purposefully want to implement everything by hand to get a better idea of the inner workings of the system. ;D