Reputation: 9446
public class SharedCacheData : ISharedCacheData
{
private readonly ISharedPetsService mPetsService;
private static ISharedEmployeeService mSharedEmployeeService;
public SharedCacheData(ISharedPetsService petsService, ISharedEmployeeService employeeService)
{
mPetsService = petsService;
mSharedEmployeeService = employeeService;
}
public static string GetUserFullName(string key)
{
Hashtable value = HttpContext.Current.Application[USERFULLNAME_KEY] as Hashtable;
if (value == null)
{
value = new Hashtable();
HttpContext.Current.Application[USERFULLNAME_KEY] = value;
}
if (value.ContainsKey(key))
return value[key] as string;
string name = mSharedEmployeeService.FindOneUser(key);
value[key] = name;
return name;
}
Whether initialize static field mSharedEmployeeService (service) in constructors?
Upvotes: 8
Views: 14753
Reputation: 301
Good practice is "static" fields should be initialized inline Hope this makes sense. Below code snippets for the same
namespace staticFieldsInit
{
public class Foo
{
static int Id;
static string Name;
static Foo() // Noncompliant
{
Id = 10;
ResourceManager sm = new ResourceManager("strings", Assembly.GetExecutingAssembly());
s = sm.GetString("mystring");
}
}
}
Better way to write the above code
namespace staticFieldsInit
{
public class Foo
{
static int Id =10;
static string Name = InitString();
static string InitString()
{
ResourceManager sm = new ResourceManager("strings", Assembly.GetExecutingAssembly());
return sm.GetString("mystring");
}
}
}
Upvotes: 1
Reputation: 2104
static fields are Class members; constructor are for creating instance members. And initialising static fields in instance ctors is contrary to OOPS philosphy. Certainly you can do that; but this will change the value of static field for every instance created for that type.
To initialise static fields use static initialiser - which is called at class loading, i.e. before any instance members are created - before ant ctor calls.
A 'static constructor' doesn't actually construct an instance. I prefer to call it a static initialiser.
Also static initialisers don't allow an access modifier.
Upvotes: 2
Reputation: 160852
This is not recommended, and generally bad practice: Now a call to the static method GetUserFullName()
will cause an Exception if no instance of SharedCacheData
has been created beforehand.
Upvotes: 2
Reputation: 13278
Initializing it in a static constructor would make more sense:
http://msdn.microsoft.com/en-us/library/k9x6w0hc(v=vs.80).aspx
class SimpleClass
{
// Static constructor
static SimpleClass()
{
//...
}
}
Upvotes: 7
Reputation: 1500105
No, that's generally a bad idea. Every time you create a new instance, it will be replacing the static variable. That's almost certainly not what you want to do.
I wonder whether actually there should be two classes here - one for the pets service and one for the employee service. It's hard to tell from the code presented, but what we've got here really doesn't look like a good idea.
Upvotes: 13