FX refactoring

| 0 Comments

Bleugh! You are lost in a maze of crapy code, all alike (and much of it copy and pasted!). The last few days have been deep in the heart of darkness. Gently teasing the business logic and the display logic of the FX code apart so that we might one day be able to write tests for the business logic.

We're nearly there but it's been a long job. Much longer than any of the other refactorings that we've done. We're refactoring to make it testable and because it's currently broken (we don't do things correctly and all the edge cases are wrong!). It's scary stuff. There are so many things that make this code hard to test and it's so tangled together that you have to break things to get it apart.

I've ditched two pieces of functionality on the basis that they don't work particularly well at present and they'll be easier to add back in once the rest of the code is working. Ideally I'd have preferred to keep them functional all the way through, but, well, needs must.

We have some code that talks to the Reuters library. It subscribes to live FX data. A nice simple job. It does it poorly. Multiple tickers make up a single forward point: Everything's relative to spot, so we always need the spot ticker; some points are dates that are directly contributed - like the 1M point; whilst others are interpolated - like 1month and 1 day needing the 1M point and the 2M point. So a ForwardPoint needs 1-3 tickers. The actual point displayed in the grid might need multiple forward points - it may be a cross currency thing and so need a point for each currency pair; it may be a forward forward and so need a near point and a far point, etc. This was modelled in an "interesting" and sadly familiar way; lots of fixed sized vectors of stuff with no typedefs.

All of this rubbish ends up with a dialog that does some of the display work - I know it should probably do all of the display work, but that's not how they did things around these parts. So we have a dialog that's really a rates engine. Originally the dialog could run 'invisibly' but we nailed that one fairly sharpish and moved the real rates engine functionality into something that didn't need a UI. Even so the dialog has multiple "live points" which contain 1-n forward points each of which is 1-3 tickers. You can subscribe to tickers, so guess who does that - of course it's Mr Dialog... When the ticker ticked the dialog got told and then did some horrible hoop jumping to get from the ticker to the live point and thence on down to grab the data.

Let me hear you say "Bleugh!".

It's cleaner now. Not ideal, but better. The FX point and calc code is now in a library and it doesn't need a GUI. The dialog can subscribe to the live points, these subscribe to the tickers they need and they only poke the dialog once even if they have several points dependant on spot and spot ticks...

Still awake at the back?

Testing up until now is a case of running old and new side by side and making sure the numbers tick the same way. Not nice, but we're almost at a point where we can test the FX calcs in isolation; driven from data that we supply (captured from the existing 'correct' version of the app). Once we get there we can do some heavy duty (and safe) refactoring of the remaining mess. I expect the refactoring will result in not a single line of the original code remaining. We'll take nice small steps though.

The code is already an order or magnitute simpler than it was. So much complexity and twistyness just fell away when we rearranged the subscription model. Lots of wierdly complex ownership tangles fell to the floor when we wrapped up the collections into classes so that they weren't just std:: containers spread thinly around other classes. Now they're wrapped up they can own their contents and an infinite amount of bollocks just vanishes.

I feel better now.

Some people shouldn't be allowed to write code.

Leave a comment