There are several remarks I could tell on your singleton implementation but since there's a far more better expert than me on these things, I'll reference his work. His name is Jon Skeet and he has an article on the singleton pattern which can be found here. Here are several points from the article that can help:
Characteristics:
- A single constructor, which is private and parameterless. This prevents other classes from instantiating it (which would be a violation of the pattern). Note that it also prevents subclassing - if a singleton can be subclassed once, it can be subclassed twice, and if each of those subclasses can create an instance, the pattern is violated.
- The class is sealed. This is unnecessary, strictly speaking, due to the above point, but may help the JIT to optimise things more.
- A static variable which holds a reference to the single created instance, if any.
- A public static means of getting the reference to the single created instance, creating one if necessary.
Basically your implementation of the pattern boils down to his first example:
public sealed class Singleton
{
private static Singleton instance=null;
private Singleton()
{
}
public static Singleton Instance
{
get
{
if (instance==null)
{
instance = new Singleton();
}
return instance;
}
}
}
And this example is bad code. I stringly suggest you read the article to understand all this and implement the correct way of the singleton pattern.
Other points:
Take your following code:
public Cart UserCart
{
get
{
if (HttpContext.Current.Session["Cart"] != null)
{
return HttpContext.Current.Session["Cart"] as Cart;
}
return new Cart();
}
set
{
HttpContext.Current.Session["Cart"] = value;
}
}
This is wrong, the setter should be used to set the value of a Cart
instance, certainly not to save a value in the HttpSessionState
object. Also since this has all to do with Cart
and not with the SessionFactory
, maybe this belongs in the Cart
class.
Why mix fieldnaming with and without the underscore and capitalize some and do not so with other? Prefixing them with an underscore is personal choice but fieldnames have camelCase, according to the capitalization conventions of Microsoft.
- 5.6k
- 24
- 40