Tuesday, January 30, 2007

Double-checked locking doesn't work in Java

This doesn't work in Java:

class Foo {
private Helper helper = null;

public Helper getHelper() {
if (helper == null)
synchronized(this) {
if (helper == null)
helper = new Helper();
}
return helper;
}
}
It is possible that to due to compiler-based reorderings helper reference will be initialised while the fields in Helper object will be at their default values. There are other more subtle problems with the double-checked locking approach. You can read the complete paper here.

In most cases such an optimisation is not worth the trouble. If you want to lazy-initialise a singleton, define it as a static field in another class:

class Helper {
class HelperSingleton {
static Helper singleton = new Helper();
}

public Helper getHelper() {
return HelperSingleton.singleton;
}
}
The semantics of Java guarantees that singleton field is going to be initialised only when referenced, and that all threads accessing the field will see the writes performed during the initialisation.

Monday, January 29, 2007

Keeping tests useful

There is a subtle difference between writing production code and test code. Both have to be intention-revealing and maintainable, but good tests are also specifications. As such, tests have to be crystal clear about what's being tested and show the usage of api/code being tested.

Quite a few times, I've seen test-data, setup for mocks, and assertions being tucked (far) away from the actual test, all in the name of maintainability. Hiding these things away makes reading tests harder, and can hide problems with the production code. Imagine a situation when you have (oh, no!) a constructor with ten parameters. To make tests more maintainable, you create a 'helper' method that returns a new instance of the class, and requires only three parameters. You just saved yourself from tons of copy-paste code-reuse, right?

That's certainly one way to look at the problem. However, why does the constructor need ten parameters? Could it be the case that the class has too many responsibilities? Now it's hard to tell; the tests are hiding the problem. This makes them less *useful* than they could be.

Next time you're writing tests and find yourself creating shortcuts for repetitive bits of code or heavy setup, pause and say to yourself: "Hmmm, how interesting...". Perhaps a better way to solve the problem will present itself.

Thanks to Ryan Cooper "Agile Guru" for help with this post.
 
Hit Counter