One time, or N?

Consider

It’s fairly easy to suggest that candidate should just be set once, outside of the loop. In real life however it’s not always so easy, particularly for newer developers, to notice that. Consider

There are clearly multiple things going on even in this simplified example. A slightly more complicated instance of this came up recently after a colleague and I noticed that portfolio preparation for a specific (admittedly) counterparty was taking 8 hours to run after a release into dev.

Of all the things I picked out, it was the counterpartyDataService being passed into the netting rule evaluation that was the most important. Underneath, it turned out that many of the netting rules needed to look up counterparty data through the counterpartyDataService in the database as part of determining whether they were applicable. This code is doing that for every netting rule, for every one of tens of thousands of trades.

A bank’s position with one counterparty is quite likely to be represented by multiple trades. The counterparty data request was a clear candidate for extraction from the netting rules and instead executed once, before entering the trade loop. Doing so, so that the code looked something like:

resulted in portfolio prep time dropping to less than an hour, and decreased the load on the database besides. Added benefit: all counterparties with multiple trades also saw improved times.

Being aware of the granuality and independence of the data you’re working on is important. The smell associated with the original code is from passing a high level service deep down the stack. Although you might argue that this code is correct, in that a test to determine whether it was correctly netting trades would pass, it’s clearly not correct when considering efficiency and base level performance. I don’t think this is an optimisation; I think this is just getting it right.

In this case it was just an experience mistake: the code arguably worked after all. However, the junior dev who wrote the code wasn’t used to looking for signs like “why am I passing in services that let me do a high level query inside a low level loop?” and was under a certain amount of time pressure, so we had a decent chat about it which led to some more imrpovements as well.