-
Notifications
You must be signed in to change notification settings - Fork 23
Logbook 6
A Logbook entry written by @tpierrain
Alone with Tomasz, we took our time to eat, to discuss, and to continue our SOR code refactoring.
We started by commenting our erratic HarnessTests.ShouldReturnALatency() test. Always successful with my fellow NCrunch runner, this test was failing almost every time when executed via the R# runner (what Tomasz uses at home).
The reason was this assertion:
Check.That<double>(runner.AverageLatency).IsPositive();
NFluent expecting a strictly positive value, the 0 millisecond AverageLatency usually found with the R# runner was leading to a RED indicator (whereas NCrunch takes more than 1 millisecond to execute this test).
We could have fixed it with something like:
Check.That<double>(runner.AverageLatency).Not.IsNegative();
But we decided to get back to more urgent refactoring steps, and to see afterwards with the rest of the mob what to do with that embryonic harness test.
But for sure, this made me think to rename this NFluent check to a more explicit: .IsStrictlyPositive()
Following some of our previous hexagonal architecture discussion, we started to rename the SmartOrderRoutingRawInproc Port type to a more appropriated: SmartOrderRoutingRawInproc Adapter.
By the way, was it relevant for this Adapter class to expose an interface? Not really. => ciao ISmartOrderRoutingRawInprocPort...
Every time we were simplifying our code, I could see Tomasz more and more pleased. Ok we were still not coding in F# ;-) but I could feel him more in peace with himself & this first implementation for the SOR.
Ok. If our first Adapter was relying on a middleware instead of a pure & simple raw C# interaction, its code would probably be callbacks-based (for every kind of incoming message events). But since we were building this investor-side Adapter for our Test Harness purpose, there was no need to simulate an artificial event-based paradigm for it. For sure, we don't want to measure any kind of middleware performance here. All we wanted is to measure our SOR implementation.
We don't want to rely on an external middleware solution. But isn't awkward to have a Send() method accepting multiple DTOs as argument like this:
public void Send(InvestorInstructionIdentifierDto instructionIdentifierDto, InvestorInstructionDto instruction)
Since we usually received one DTO per method call, it seemed more relevant to us to modify the Send() method like this:
public void Send(InvestorInstructionDto instruction)
with the instruction identifier embedded within the instruction.
This was what we did then, continuing our refactoring session. Refactoring? Nah! It was much more a Re-design session here. Mos def: we were impacting the contract/interaction model of the SOR within all our acceptance tests...
Tomasz wanted to simplify even more by replacing the InvestorInstructionIdentifierDto type with a simple long value, but I advocated to keep it for a while in order to maintain our baby steps, but also because this class was today responsible to maintain the uniqueness of the investor's requests identifiers.
In day 15 , we all identified the need for a session helper/manager. A kind of logic to prevent any Investor from being flooded by the SOR with other investors' execution events.
Also: the more Tomasz & I were observing our SOR API, the more we wanted it to embrace a Functional approach rather than an Object Oriented one.
Last but not least, we were not very pleased to expose our InvestorInstruction type outside the SOR domain landscape.
Those 3 points led us to replace our previous .NET events on the InvestorInstruction instances (Executed & Failed) by a callback-based approach.
We changed from this former SOR usage:
var identifierDto = sor.RequestUniqueIdentifier();
var instruction = sor.CreateInvestorInstruction(identifierDto, ...details...);
instruction.Executed += lambdaWhenExecuted;
instruction.Failed += lambdaWhenFailed;
sor.Route(investorInstruction);
To this more functional one:
var instructionDto = new InvestorInstructionDto(new identifierDto(), ...details...);
sor.Route(instructionDto , lambdaWhenExecuted, lambdaWhenFailed);
But since we realized this re-design would be impactful, we decided to introduce a temporary sor.Subscribe(identifier, lambdaWhenExecuted, lambdaWhenFailed); method as an intermediate step.
In fact, we had to stop before all our acceptance tests were fixed. We just had enough time to make it build, but with some // TODO scattered in the process.
One-hour-and-a-half sessions are really short... We will end this major change during the next session.
We also said to ourselves that it would be important to timebox our Journey1 so that we can re-focus on priorities once this code simplification achieved. Let's try to finish our test harness before XMas...
A Logbook entry written by @tpierrain
I'm warning you: this will probably the shortest SOR logbook entry ever.
Yes tonight I'm sick. I'm tired. And today session only lasted 1 hour and 15 minutes (lunch included). So let's do it quickly! ;-)
No. Today we started our session by watching "WAT", a funny 4 min talk by Gary Bernhardt that Cyrille found on twitter this morning. Excellent warm-up for the cheeks ;-)
Then, we fixed all the broken tests.
Indeed, Tomasz and I didn't finished our redesigning session last monday. We also let tons of broken tests in the process (developping at lunch time is really short).
Thus today we fixed. Ozgur, Cyrille, Tomasz and I fixed the whole tests. But it took us some times to troubleshoot...
Following Murphy's law, my laptop also experienced new and unexpected lags that slowed down us in almost every move ;-(
Yes. In the mean time we continued to talk and to sketch about Adapters, but mostly about where you should (and shouldn't) find DTO types in your hexagonal architecture.
Now I like to split the hexagonal architecture in 3 parts:
-
the inside (application use cases + core domain)
-
the outside
-
and the Adapters' limbo...
Data Transfer Object (DTO) pattern is really about network optimization. Instead of using chatty interactions with lots of small data structures, we reduce the number of calls. To do so, we are using "an object (the DTO) that aggregates all the data that would have been transferred by the several calls (in one call only)." (source: Wikipedia)
For me DTO should belong to the outside world, but also to the Adapters' limbo... Not the inside world! In some cases, you may even have different kinds of DTOs for the same system, depending on middleware technologies you are using.
It's the Adapters responsibility to make the link and to map the DTOs world of the outside, with the POCOs/POJOs world of the inside.
My (little) concern for today was to find DTOs within "the inside" (i.e. our application use cases layer => ISmartOrderRoutingEntryPoint and the core domain).
But after discussion with the crew, I admitted that adding a mapping here was probably not today's priority.
Cyrille also highlighted that we will sill need something like a session helper for every Adapter to be able to connect the proper socket/context/client when events or callbacks will be called by "the inside" of the hexagon. Something that we clearly not implemented yet (YAGNI). But something to keep in mind for the future.
Damn! Didn't I said that this logbook entry would be short?!? Time to conclude.
Our last action for this short session was to realize that our SmartOrderRoutingRawInprocAdapter were not really tested anymore. The only test usage for it is from our TestHarness.
To be improved next thus, among other things...