Null Pointer Exception in XmlSnapshotManager, problem with SnapshotManager

When the Skaringa XML demo is run the second time (after a snapshot has been created in the first run), it fails with a Null Pointer Exception. Here is the stacktrace...

java.lang.NullPointerException
at org.prevayler.implementation.XmlSnapshotManager.readSnapshot(XmlSnapshotManager.java:41)
at org.prevayler.implementation.SnapshotManager.readSnapshot(SnapshotManager.java:92)
at org.prevayler.implementation.SnapshotManager.readSnapshot(SnapshotManager.java:85)
at org.prevayler.implementation.SnapshotManager.<init>(SnapshotManager.java:25)
at org.prevayler.implementation.XmlSnapshotManager.<init>(XmlSnapshotManager.java:21)
at org.prevayler.demos.demo2.MainXml.main(MainXml.java:19)

What happens is that the XmlSnapshotManager calls super() in its constructor letting SnapshotManager do any work it needs to do. However, it assumes that control will come back to the constructor so it can set up an ObjectTransformer with which to read and write XML snapshot files. The problem is that upon the second run, SnapshotManager calls readSnapshot() from the constructor, so the ObjectTransformer will not have yet been created in the overloaded readSnapshot() method resulting in a null pointer when it is attempted to be used. Here is the problematic code in the SnapshotManager constructor...

_recoveredPrevalentSystem = _recoveredVersion == 0
? originalPrevalentSystem
: readSnapshot(_recoveredVersion);

This is not only problematic for XmlSnapshotManager, but any future extension of SnapshotManager which tries to set up things in the constructor and gets short-circuted. There is at least one way around this but even having to perform a workaround implies that you need to have more knowledge about the SnapshotManager than should be necessary in good object design. One should be able to extend SnapshotManager, call super() and override readSnapshot(), suffix(), and writeSnapshot() and be able to assume that everything will work without knowing about SnapshotManager's internal implementation details.

The only real fix I can think of in XmlSnapshotManager is to make the "trans" class variable non-final, not bother setting it up in the constructor, move the ObjectTransformer code from the constructor to a "setupTransformer()" method, and do the following in readSnapshot() before using the transformer...

if (this.trans == null) setupTransformer();

That seems a bit kludgy, if you ask me. We shouldn't be calling readObject() from the constructor in SnapshotManager. I'm not sure exactly how that solution would look. I'll leave that up to whoever takes this bug.

Jake