I am learning design patterns. I created a singleton Logger class that should return an new instance of logger only if it is null, and return the same instance every time otherwise. But implementing the class is resulting in creating a new instance everytime.
public class Logger
{
private Logger()
{
}
private static Logger instance;
public static Logger Instance
{
get
{
return instance == null ? new Logger() : instance;
}
}
}
static void Main ()
{
Logger log1 = Logger.Instance;
Logger log2 = Logger.Instance;
Console.WriteLine(log1.GetHashCode());
Console.WriteLine(log2.GetHashCode());
}
The resulting hashcode is supposed to be the same on both lines but its not. Why is that?
public static Logger Instance
{
get
{
return instance == null ? new Logger() : instance; // You do not _set_ 'instance' !
}
}
So, this should be more like
public static Logger Instance
{
get
{
if( instance is null ) instance = new Logger();
return instance;
}
}
Mind that this is not threadsafe.
About the correct and safe implementation of Singletons, there are many articles written by people that can do that way better than I ever could. I suggest you explore that a little.
you must initialize the "instance" field with the new logger instance before returning it or it will always be null.
if (instance == null)
{
instance = new Logger();
}
return instance;
you can also use a single expression
return instance == null ? (instance = new Logger()) : instance;
Note: in a multi-thread environment you need to use a lock or other synchronizations