-
Notifications
You must be signed in to change notification settings - Fork 23
Logbook 3
A Logbook entry written by @tpierrain
The mob got back to work today (but without Ozgur this time).
As I was out of the office the week before for a business trip (and still a little bit jet-lagged I guess ;-) I tried to resynch myself with the ongoing work. After this brief catch up for me, Mendel took the keyboard and we continued a previous refactoring initiated to make our code more coherent (by replacing every .NET events by callbacks transmitted as argument to methods). Indeed, part of the code was raising .NET events, other part was expecting callbacks. After few refactoring steps, we realized that we were not all completely enthusiastic with this decision to replace .NET events with callbacks (the ones given to the IOrder.Route() method for instance).
Since we didn't have enough pros/cons arguments on that topic, we then decided to postpone this design decision until we have enough information to take it. We also started to instruct this design debate with pros/cons:
# Events vs. callbacks transmitted to methods
## .NET Events pros
+ More visible / prominent part of the domain
+ Appropriated for fan-out notification (one to many)
## Callbacks pros
+ Forces the Registration (not optional like .NET event subscriptions)
+ Per-method-call registration; no need to rely on closure to match callback instance with the proper context
TBC...
One of the pro-callback argument was also to be more aligned with the possible F# implementation of the SOR. But we finally agreed that we rather go full F# or full C# (i.e. leveraging on C# concepts such as events) rather than having a kind of lowest common denominator implementation for the SOR.
As a consequence, we frozen our refactoring efforts, and we temporary stick to the ol' .NET event option by exposing a new Failed event on the InvestorInstruction type.
Because we also chat a while about the semantic of the InvestorInstruction. Should we keep it or replace it with the notion of ExecutionStrategy that traders seem to refer also instead? Proper timing for "BA" Cyrille to join us, and to explain that every-time traders were talking about strategies, they were referencing to algorithms (e.g. WAP, T-WAP, V-WAP).
Concretely speaking, that means that our Investor Instruction is still a valuable name (e.g. as an investor, I intent to cut my position on ...), but that we'll also have to add an execution strategy type as part of the Investor Instruction arguments.
Fortunately for us Mendel had the keyboard (cause he's blazzing fast with it ;-). Unfortunately for him, my default Resharper settings was set to Visual Studio scheme (not the Resharper 2.x/IntelliJ he was used to). I said unfortunately, because even when he changed the keyboard scheme for the session, it seemed that some of the short-cuts were still disabled (probably due to some collision with VS.NET existing short-cuts). Sad Panda...
"We shouldn't need such kind of tools... and NCrunch too BTW"" (smile)
I still don't know whether he's really serious or not with those statements... but that makes us tease ourselves gently and adds some fun to our sessions (from a personal point of view, I really enjoy running gags)
We finished this short session by allowing partial executions to be specified at the investor level so that our ShouldFaileWhenOrderExceedsAllMarketCapacityAndPartialExecutionNotAllowed acceptance test finally passes, using this brand new Failed event.
From a personal point of view, I hope that I'll be more active/efficient next time. That's all folks!
A Logbook entry written by @tpierrain
Today's session was mainly between Mendel and I. Cyrille was among us at the beginning, but he quickly left the room to attend another meeting after having contributed to our topic-of-the-day-debate (i.e. what's the next step for us).
And the winner was... to make the SOR able to success -when requested liquidity is available across the markets- despite the fact that one or more limit orders are rejected by a market (remember? we previously implemented a Market sweep strategy. i.e.: a weight average execution on the markets).
Mendel and I started then to introduce the concept of ExecutionState. A kind of "fil-rouge" for the state of the InvestorInstruction during its complete life-cycle. After few extract methods and refactoring steps, we were able to keep the execution history of any investor instruction in its corresponding ExecutionState (transmitted to the Solver for instance).
In that new design, the retry attempts are initiated directly from the SmartOrderRoutingEngine registered callback (for the OrderFailed event of the OrderBasket). Now, the SOR engine asks the Solver for another round to complete total execution if needed.
To prevent from facing infinite loops on limit orders retry (due to rounding issues with the markets weight average strategy, or if it's really impossible to execute all the InvestorInstruction for instance), we introduced 2 things:
- A goodTill option (nullable DateTime) on the investor instruction.
- A mechanism to prevent the SOR from sending limit orders to a Market after it send us 3 order rejections.
In fact, the bell rang while we were trying to green a test demonstrating the second mechanism (i.e.: ShouldStopSendingOrdersToAMarketAfter3Rejects() acceptance test). But Mendel and I have seen few things that we may improve on our code design once we'll make our test green.
TBC...
A Logbook entry written by @tpierrain
Cyrille and Ozgur having good excuses to cut school today ;-) we were 3 developers: Tomasz, Mendel and I.
Before our session, Tomasz teased us with a code-review he sent us by mail. It was about what we produced during our previous session (the one he couldn't attend). Not very surprisingly, it was very much inspired by FP concepts. Mendel and I were ok with most of Tomasz proposal, but before refactoring anything, we needed to stick to our TDD hygiene: Red, Green, Refactor
Yes. Time to Green the test we've just added few days before (remember: we are only mob-ing at lunch, in 1 or 2 hours long sessions. In that condition, hard to always finish our tasks right in time).
This test was checking the SOR ability to disqualify a given market after 3 missing attempts to execute Limit-Orders on it (a kind of naive implementation to avoid infinite loop for our investor instructions).
We fixed it (by introducing a GetValidMarkets(price) method to the MarketSweepSolver). We also fixed the other test demonstrating the SOR ability to execute an investor instructions even when one market was disqualified (the 3 failures rule I was just talking about).
All right sir: all green!
"Why don't we use the Null Object pattern here and there?", "Are we properly implementing CQS within our code?", "Why wouldn't we return some kind of monadic constructions?", ... were some of the questions we talked about (strongly inspired by Tomasz code review).
Ok, but first things first: we wanted to make our InvestorInstruction class immutable. To be completely honest, we were very close to start this refactoring while we were fixing the test at the beginning of the session. But in a mob -like when we pair- there is always someone that is enforcing our discipline. Here: "baby steps guys! baby steps!"
Our InvestorInstruction was pretty much immutable already, excepting its ExecutedQuantity property that was updated every time a LimitOrder was executed (notification being made through the corresponding ExecutionState). Time for us to reassign the responsibility to follow what has been executed to the ExecutionState instead of our now immutable InvestorInstruction.
Yes. And that's what we led into after discussions, and few intermediate renamings (e.g. ExecutionContext). In fact, this recent concept was involved in almost all refactorings today. But I'll let you check the code if you are interested in.
Indeed. It took us at least 10-15 minutes to talk about that at the end of the session. With questions such as:
-
Question: Should we introduce our first performance tests BEFORE we change our market connectivity API into an asynchronous one (in order to compare those 2 approaches)?
-
Answer: No. We are still on our Journey 1. Also, the market connectivity API will be asynchronous because it is the way they are in the real world. I.e.: it's a constraint, not a particular design choice on our hands.
-
Q: Should we extract our (hexagonal architecture) ports and adapters first, and then transforming our simple Market proxy into real APIs? (one API for feed reception, the other for order routing)
-
A: Probably yes.
-
Q: Should we merge those two steps into one?
-
A: Probably not. Baby steps again, to avoid a tunnel effect
Ding-dong! Time to get back to (real) work.