Broken abstractions => broken code

I’ve just received another powerful lesson in this simple concept. When you break your abstractions, it invariably leads to broken code.

At Wotif, our domain model pretty obviously includes money – our deals have prices, customers pay for bookings, and so on and so forth. Money is important to us. 🙂

Wotif supports multiple currencies. To be more precise, we support multiple countries, each of which has a single currency. Some countries share currencies (e.g. most of Europe use the Euro, and several Asian countries use the US Dollar). Because of this, our concept of money (our Money class) is basically an amount, a currency, and some arithmetic methods.

We back our data with a database, using Hibernate. So we need a way to map our Money objects to a database structure. Some of our domain classes have several distinct Money objects (for example, we store the tax component of a price separately to the base price). A simple Hibernate user type or component wouldn’t work, as we would not want to have to have several columns for the Currency id on these tables. Because of this, until recently, we didn’t have a simple way of storing Moneys using Hibernate. Instead, we stored the currency and the amount separately, as different properties on the domain object. In other words, we hacked our domain model to make persistence a bit easier. At this point, I’m sure you see where I’m going…

Although I’ve been meaning to address this for a while, I only got around to it this week. I wrote a Hibernate user type to support Money directly, and I introduced an interface that the domain models supported to allow obtaining the Currency separately. This works because the nullSafeGet method of the Hibernate Us@erType interface provides the owning component[1] as a parameter. There were a few gotchas (see the footnote), but it worked reasonably well, and I had our Hibernate MoneyType developed and used in our core domain classes with about a day’s worth of effort. A good return, I think.

Now, had the earlier mentioned persistence hacks been kept private to the objects, this would have been the end of the story. But no… the persistence hack turned into a broken abstraction. You see, the fact that we stored the amount portion of the money as a BigDecimal[2] crept out in two ways:

• The getters were public (e.g. public BigDecimal getBookingFee();)
• The Money getters had the second-class names (e.g. public Money getBookingFeeMoney)

One of my motivations for introducing the Hibernate MoneyType was to fix these broken abstractions. I knew there was code out there doing the wrong thing, but boy was I surprised at the number of abuses I saw.

• JSPs that expected the BigDecimal so that they could format the value (instead of using the formatters we had for Money!)
• Tests that didn’t bother setting the currency, or that broke rules such as trying to use a different currency later.
• Validation code that didn’t treat null and zero the same (even though the Money class treated a null amount as 0)

However, by far the worst things weren’t that there were tests that didn’t conform to the constraints of our model (that all prices have an associated currency), but that there were hacks in the production code to allow the tests to pass while not conforming to the model.

None of the abuses of the code were done deliberately; it was almost certainly a case of following the path of least resistance. The simple fact that the decimal amounts were present in the domain objects and that the Money versions were “second class citizens” due to having the word “Money” appended to the end of the name, made abuse both possible and tempting. The broken abstraction directly lead to broken code.

I’ve just finished a four day exercise “fixing” the things I “broke” by enforcing the domain. And I haven’t even done it to the entire domain yet – just the core part of the domain. *sigh* Broken windows, indeed.

—-

fn1. There are two catches for this:

• The owning object isn’t initialised at the time of the nullSafeGet call – all of its properties are the defaults (usually null). So if you use this trick, you need a lazily-initialising object, which is what I provided.
• If the property is addressed directly via HQL, then the owner is null, at least in Hibernate 2 (which we are _still_ using, damn it…).

fn2. Purely for persistence… the Money class actually used a long which represented a shifted-decimal number. Repeat after me: floating point numbers are BAD for money. The database, BTW, used a fixed-precision decimal as well. We needed to store decimals to preserve compatibility with some older applications, and for those pesky reports that hit our reports database without going through our API.

Author: Robert Watkins

My name is Robert Watkins. I am a software developer and have been for over 20 years now. I currently work for people, but my opinions here are in no way endorsed by them (which is cool; their opinions aren’t endorsed by me either). My main professional interests are in Java development, using Agile methods, with a historical focus on building web based applications. I’m also a Mac-fan and love my iPhone, which I’m currently learning how to code for. I live and work in Brisbane, Australia, but I grew up in the Northern Territory, and still find Brisbane too cold (after 22 years here). I’m married, with two children and one cat. My politics are socialist in tendency, my religious affiliation is atheist (aka “none of the above”), my attitude is condescending and my moral standing is lying down. View all posts by Robert Watkins