Friday, November 13, 2009

Static interference - the java killer

I was once a young and foolish developer.

Now I am a slightly older and foolish developer. However, I have at least come to terms with the evils of static.

Now, the static keyword can mean two different things in Java, depending on context. I'll describe the rare case first, with an example (I do love examples).

public class SomeClass {
public static class StaticClass {
}
public class InnerClass {
}
}

The static keyword lets us know that StaticClass is just a regular class. It does not have any implicit reference to objects of SomeClass, and can not touch instance variables in SomeClass.

The case with InnerClass is quite different. InnerClass has an implicit reference to SomeClass. You rarely need to worry about this in practice, you can just assume that instance methods inside InnerClass can access instance methods and fields in SomeClass. Every InnerClass instance knows about exactly one SomeClass instance.

The other meaning of static is when it's applied to fields and methods. Static fields don't live inside the instance, they are a singleton that just lives in one place in the JVM. That's great you may say; you can access them without having to create objects, and since there's only one instance per JVM you save lots of memory. That's correct, and I'll get back to when doing this is a good idea.

Static methods are just like fields, bound to the class instead of the instance. Instance methods can call static methods but not vice versa. Static methods may not call instance methods or access instance fields. Static methods also can't be overridden in the class hierarchy, which also means that they are faster to invoke in some JVMs.

(On a side note, I think it's unfortunate that Sun decided to reuse the static keyword for two different concepts.)

So, what's the problem with statics?

It's simple: they kill testability! They kill modularity! They kill reusability!

Static is only ok for a handful of cases:

Case 1: Final and immutable static fields.

If a field is final and immutable, it can't be used to contain any form of state. Static state is what kills reusability. Two components must be able to coexist without knowing about each other and mutating the same state. This almost always boils down to constants. Things that don't change can be shared:

// ok, immutable and final
public static final String DEFAULT_HOST = "localhost";

// not ok, final but not immutable
public static final AtomicInteger counter = new AtomicInteger();


Case 2: Side effect free static methods

Static methods can be fine, if they are side effect free and deterministic. Howver, as soon as you discover that your static method should do something that relies on some external state or produces a side effect, you should consider extracting an interface for it.

Math.min() and Math.max() are fine to use, they don't produce any side effects and they are quite deterministic.

System.currentTimeMillis() is not fine to use, it's side effect free but it is not deterministic.
If your code relies on calling this method, you probably need to add lots of Thread.sleep() in your tests to make them work. This is not a good design.

Instead, create a TimeProvider interface like this: public interface TimeProvider { long now(); }
The default implementation can (and most likely should) just call System.currentTimeMillis().
The important thing is that you've injected a layer between the smelly static method and your own code. This makes your code testable and your design cleaner.

System.out.println() is also something you should avoid. It's deterministic, but it's not side effect free. Your tests will have a hard time capturing output to verify correctness. You also lose control over where different components report their progress. Instead, just inject a PrintWriter to your object if you need one. Now your code is testable!

Case 3: Loggers

Loggers are a bit of a special case. It's very common to see this at the top of classes:

private static final Logger logger = LogFactory.getLogger(ClassName.class);


I personally like to use:

private final Logger logger = LogFactory.getLogger(getClass());

since I can copy paste it everywhere and not worry about changing name manually, but there is a cost involved. If there are many small objects that need to log, the memory overhead and creation overhead may be too expensive.

In this special case, you may consider it to be fine to use static. The argument is that loggers should not modify the code behaviour at all, so testing it is irrelevant.

There are of course similar cases where people may argue that static is ok to use. That's fine, just keep in mind to make static the exception, and convince yourself that you really really really need to use static here.

Case 4: Public static void main

Well, I personally think it was a poor design of Sun to let the entry point of an application be a static method. I guess it's due to historical reasons (C has a main())
I would have preferred it if you would have to implement something like this:

public interface CommandLineApplication {
// returns exit code
int main(String[] arguments);
}


So when are statics explicitly bad? Almost always!

Here are some concrete examples:
Static factories is a common pattern. Every object can locate any other object of interest by just going through the static factory. "It's easy! I don't have to inject stuff everywhere, just go through the factory". Unfortunately this kills testability and reusability.

Singletons: Singletons as such aren't necessarily bad. The problem is when you acquire singletons through SingletonClass.getInstance() which I've seen far too often (and I have been responsible for a couple myself). This really kills testability, since you can't replace the instance. Instead, just inject the instance.

Sometimes it may be a good idea to have some default settings for ease of use. But make it an optional default. Provide at least one full constructor where you can inject everything and optionally provide simpler constructors with some fields defaulted to a static implementation.

This turned out much longer than I intended yet I don't think I have fully expressed all I know that is wrong, evil and just plain immoral concerning the overusage of static.