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 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 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.