using static methods or instantiate the class?

Question

Hi, recently i've got a dispute about using static methods in a class that is used by hundreds threads. In my opinion the "first" solution has no special benefits except it is easier to use for the client of class but i have been told that it is extremely bad idea and I have to use the "second" solution. Can anyone give me clear explanation why i'm so wrong? Thanks a lot!

PS Let's assume that dictionary is thread safe.

class A
{
    static Dictionary<int, string> m_dic = new Dictionary<int, string>();
    public static string GetData1(int nKey)
    {
        return m_dic[nKey];
    }
    public string GetData2(int nKey)
    {
        return m_dic[nKey];
    }
}

//these func are called from threads...
void ThreadFunc1()
{
    print A.GetData1(1);
}

void ThreadFunc2()
{
    A a = new A();
    print a.GetData2(1);
}

Regards, Leonid

Answer 1

If the threads need to access a thread-safe shared resource, a static class is perfectly viable. So you aren't wrong.

Answer 2

Using an instance member implies (to me, at least) that there's something relevant about the instance itself - either its state, or possibly its behaviour in terms of overridden methods (i.e. the "state" is the execution-time type of the instance).

In this case neither of these is true - so I would view creating an instance as bad style, leading to misleading code.

Answer 3

Static data in C# can be compared with global variables in C++, Delphi and other languages. They tend to be bad because the memory for the variable is allocated when the application starts and only released when the application ends. They are considered bad because they can be modified from any location within the code.
Of course, you use static methods to access the static data, but I would advise you to make the static data private, so only the class itself can modify the data through static methods. Then the whole data becomes a singleton.
Btw, you assume it's thread-safe but you better make sure it's thread-safe.
About calling a static method through an instance. That makes sense if you need the instance for other tasks but otherwise it's a small waste of resources. It's not wrong, unless some developer changes the method from static to non-static or when they add a non-static method with the same name and similar parameters. Calling static methods from an instance also adds confusion because it makes it unclear that the method does nothing with the data within the class itself.

Answer 4

I would definitely go with the static method for all the reasons Jon stated plus I would like to add one more. Consider the following code which runs in a multithreaded environment.

void ThreadFunc2() 
{ 
  var a = new A(); 
  if (a.GetData2(1) == "foo")
  {
    DoSomething(a.GetData2(1));
  }   
} 

An unsuspecting programmer creates a new instance of A and naively makes two calls to GetData2 that are codependent thinking that nothing could go wrong because the A instance is used nowhere else. But the problem is that there is no guarentee that GetData2 will return the same thing the second time because another thread could have changed the static dictionary that the method uses. That is totally not obvious when you use a instance method to extract static state.

I am not saying that using an instance member to read static state is necessarily wrong. What I am saying is that it has the potential to cause problems especially when there is an implication that separate object instances can be used simultaneously from multiple threads safely.