isxaker
isxaker

Reputation: 9446

Initialize static field in constructor. It's normal?

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

Answers (5)

Krishna Nekkala
Krishna Nekkala

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

ukhardy
ukhardy

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

BrokenGlass
BrokenGlass

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

Mark Robinson
Mark Robinson

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

Jon Skeet
Jon Skeet

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

Related Questions