patterncsharpMinor
Instantiating if null
Viewed 0 times
nullinstantiatingstackoverflow
Problem
I often have static classes which uses the
But is there a cool trick to do this, or am I just coding it wrong? Or is it ok?
Update
The
But here is another example:
This is again nice because if I already have a context, I can provide it, and then I won't have to attach the entity (User) to another context if I want to modify it:
How can I improve it? Or is it okay?
Update 2
The
DataContext somehow, and they all make a new DataContext, but often I already have one instantiated elsewhere, so why not use that?public static bool SignIn(string email, string password, DataContext db = null)
{
bool disposeDb = false;
if (db == null)
{
disposeDb = true;
db = new DataContext();
}
//Sign In stuff, or any other stuff...
if (disposeDb) db.Dispose();
}But is there a cool trick to do this, or am I just coding it wrong? Or is it ok?
Update
The
DataContext is not expensive to instantiate. I guess I just leave it be.But here is another example:
public static User GetUser(DataContext db = null)
{
if (HttpContext.Current.Request.Cookies["Id"] == null)
{
return null;
}
else
{
bool disposeDb = false;
if (db == null)
{
disposeDb = true;
db = new DataContext();
}
int id = Convert.ToInt32(HttpContext.Current.Request.Cookies["Id"].Value);
string cookieKey = HttpContext.Current.Request.Cookies["CookieKey"].Value;
User user = db.Users.FirstOrDefault(x => x.Id == id && x.CookieKey == cookieKey);
if (disposeDb) db.Dispose();
return user;
}
}This is again nice because if I already have a context, I can provide it, and then I won't have to attach the entity (User) to another context if I want to modify it:
User user = MyClass.GetUser(db);
user.Email = "asdsasd";
db.SaveChanges();How can I improve it? Or is it okay?
Update 2
The
DataContext should only be instantiated once per page request. And here is a neat way to do it, that actually allows me to access it even from static methods.Solution
At the very least, you should place the second
From a design point of view: yes, I'd say this violates the single responsibility principle. You have a method that not only does certain calculations, but also creates a
It looks to me like
if statement in a finally block. As things stand, the code is not generally correct from a purely mechanical perspective. I would also phrase it as:bool dbDispose = db == null;
db = db ?? new DataContext();From a design point of view: yes, I'd say this violates the single responsibility principle. You have a method that not only does certain calculations, but also creates a
DataContext if necessary. If creation has to be modified at any point, then you'll have many places to change that. (Unlikely if it's default-constructed like this, but I doubt it's that simple in practice.)It looks to me like
SignIn should actually be a member function of DataContext.Code Snippets
bool dbDispose = db == null;
db = db ?? new DataContext();Context
StackExchange Code Review Q#19217, answer score: 5
Revisions (0)
No revisions yet.