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.
5 thoughts on “Broken abstractions => broken code”
Good post Robert.
Now, if only we could have pretty arithmetic operators on BigDecimal and BigInteger, that would probably get rid of the last bit of resistance to using them for financial things. Although in your case, I am sure you would look *very* carefully at each and every calculation (he says as he is about to make a booking for the weekend).
And that is the most interesting captcha I have seen !
I’m glad you like it… I had to put it in about a month ago because I was getting hit by about 100 comment spams a day (stupid spammers).
I’m not sure why there is that much resistance to using BigDecimal as a basis for financial calculations. How many operations do you need? I can really only think of a few: addition, subtraction, percentage (most multiplication and division operations are actually percentage changes), and a few convenient power/exponential operations (e.g. for interest rate calculations).
I’ve been working either in the finanical domain or e-commerce applications for a total of about 8 years, and I’ve _never_ done anything more complex with money than that. In particular, avoiding multiplication and division helps a lot. 🙂
I agree, its certainly good enough for real world use.
I think people come up with cosmetic arguments as to why they don’t like something cause they have to do something.multiply(another).divide(foo) etc etc… rather then based on reality. This could be said probably about lots of aspects of programming languages (its fashion driven).
If you are doing lots of scientific thingamies like that, sure, it would annoy me (I did once, but only once, and it was very annoying). If it bothered me again really badly, I could justify using something like this: http://www.toad.net/~jkaplan2/infiqs/ – not pretty (pre processing) but could be effective in controlled situations.
I’m interested to know why you say “avoiding multiplication and division helps a lot”. Is it simply because then you have to worry about things like scale and rounding? As for not often needing multiplication, maybe I work in a different part of the finance industry to you, but total = price.multiply(quantity) is a pretty regular requirement in my neck of the woods 😉
Andrew, multiplying by integer amounts counts as addition. 🙂 When I say avoiding multiplication helps, I mean avoiding multiplying by weird numbers such as 3.14159265.
As you observe, it’s about scale and rounding errors. If you can limit yourself to integer-based operations (addition, subtraction, integer multiplication) and a few well-defined fractional operations (e.g. percentage and interest rates), you can save yourself a lot of grief.
Sometimes changing the rules helps. For example, mathematicians and engineers avoid nasty rounding errors when performing repeated trigonometric calculations by shifting to radians instead of degrees, thus avoiding that weird number I mentioned earlier.