What speaks against static fields?

Question

I have two cases in our tool where I have some difficulties in my integration tests with static fields. In the application these fields are no problem. In my tests I create basically a new application with each test method. When I run these in one test run, you can imagine, there will be some problems. First let me show two such cases.

Case 1

class SomeClass
{
    private static IService service;

    public static void Initialize(IService service)
    {
        SomeClass.service = service;
    }

    public void DoSomthing()
    {
        service.Foo();
    }
}

Basically objects of this class will be created in huge numbers. To be able to use the IService object it is stored as an static field. In my tests this is a problem because the IService is actually an IServiceProvider and a service is retrieved only once in the first test. In the second test the service of the first test is used. In the real application there is only one IService or IServiceProvider. Therefor we do not have a problem here.

Case 2

abstract class BaseClass
{
    private static readonly Lazy<SpecificClass> specificClass = new Lazy<SpecificClass>(() => new SpecificClass());

    public static SpecificClass SpecificClass
    {
        get { return specificClass.Value; }
    }
}

class SpecificClass : BaseClass
{

}

This is even more troublesome for my tests, because even if I create a complete new application, when the SpecificClass was used in the first test it is the same object in the second test. In my tests I have a memory leak here, because the SpecificClass has a list where it remembers objects from the first test. With each tests more and more objects are added to the list. In the real application the list is filled only once on application startup. So no memory leak here.

I know, that tests typically show design flaws, but I cannot see one here. The only argument for removing these static fields I can think of is, that my tests do not work otherwise.

So my question now, why is it in these to cases considered bad code to use the static fields or is it not? I do not want to know solutions for these cases. I only need a justification why a change in the code is required other than "I cannot test it properly."

Answer 1

What you describe is a valid use case for all objects that are not suitable candidates for dependency injection (ORM entities are an example).

An alternative to static fields in those cases is providing the dependencies as method arguments:

class SomeClass
{
    public void DoSomething(IService service)
    {
        service.Foo();
    }
}

One of advantages of this approach, besides easier testing, is that you don't have to take care to properly initialize static state manually (which may sometimes be difficult to achieve and especially difficult to maintain afterwards when static dependencies start depending on each other, because of order in which static fields are initialized, etc).

Also, refactoring and changing code in the future is easier because you don't have to move/duplicate static field definitions and initializations when you decide to extract the logic that uses them into separate class, etc.

Answer 2

The design flaw here is a component decides the life-cycle of its dependencies, which is bad both in the actual application and it makes things harder when coding tests.

What you need is using an inversion of control container with dependency injection support, and let it decide by configuration injected dependencies' life cycle.

For example, in Castle Windsor it would be configured as follows:

IWindsorContainer container = new WindsorContainer();
container.Register
(
     Component.For<SomeClass>().LifeStyleTransient(),
     Component.For<IService>().ImplementedBy<SomeService>().LifeStyleSingleton()
);

So you're deciding that IService is a singleton by configuration but your code relies on that instances of SomeClass will get injected an instance of an implementation of IService.

Why is it bad for the actual application. I only need a justification why a change in the code is necessary.

• No EASY unit testing: you can't use automatic dependency injection if you don't use instance constructors and/or properties. This makes your code harder to test, because a simple configuration can't inject a fake instead of an actual implementation.

• No automatic dependency injection. Different applications and services can't be configured to use same or different dependency interface implementations and you lose a big feature: object life-cycle (transient, singleton, per-request, per-thread) could be defined by configuration and you code shouldn't be aware about this.

• No single responsibility principle. Is your class responsible of deciding the life-cycle of its dependencies, or isn't the framework which should define it?

You said to Jon Skeet in some comment in your question:

@JonSkeet I know why they are bad for my tests. But that is no accepted argument as long as it works for the real application. That is why I want to find a different argument. –

Software quality is as real as the actual application. If developing quality software isn't an accepted argument for you, maybe you need to get back to the roots and find why an application or service must be testable.