Tuesday, June 10, 2008

Immutability mutiny

As part of some legacy code we designed a Date wrapper for our persistent objects, that referenced dates, so that we could hide the fact of if we were using 'varchars' to store a date or using an actual 'timestamp' in our Oracle DB. All our code would use our wrapper class and be oblivious to the underlying implementation and also provide a more 'humane' interface.

As part of this initial design back in 2002 the developers decided to make the class a value object that was immutable, exhibited side-effect free functions, closure of operations and was a stand-alone class (see DDD by Eric Evans) which made developers life easier as they could safely perform operations on this Date object without needing to know the internals or combine these objects in a safe manner. Now I am not saying that the developers designed it knowing all of this but rather that it grew from a very simple interface into something that exhibited most of these behaviours.

Now to get to the point of this post: All of us thought that our Date wrapper was immutable and we relied on this fact heavily even caching all Date's ever created for garbage collection purposes, but it turns out after 6 years of use we just found out that it in fact is not.
The problem is that our ORM (Toplink) was mapped so that it could use protected method accessors to determine the value to write and read (which supplied our polymorphic behaviour to the clients). Now if we create an object and set an instance variable to a Date and then regsiter this with a unit of work, modify the object's date, and then try persist; Toplink will check the changeset and merge the result into the original object which bypasses our immutable side-affect free functions, and directly changes the object through the protected methods.

You could say where are all the unit tests; the unit tests should catch this; after 6 years you never came across this?

But we have nearly 2500 unit tests, run regression tests using our staging code vs our live code for about 400 000 (medical aid insurance) policies we have on our systems and yet we never came across this.

The problem boils down to: for some reason all the developers followed a way of coding that never included a default constructor that set a date on an object and then registered this new object with the unit of work. What we typically did was register the object at the end as we typically had control/awareness of what was being modified created. New developers that had joined the team had not been 'tainted' with the way that things had been done and bingo things began to break horribly.

I'm not sure how we can avoid immutability issues like this when using Toplink in this manner but anyway we have an idiom now on how to use dates and some code that checks if an existing date is being modified by Toplink or a client.

No comments: