Architectural re-think needed to remove tight coupling to un-managed resources such as the web socket
SimonGeering opened this issue · 4 comments
Related to #15
Further progress adding unit tests may be limited by the current tight coupling of the Connection Manager to its ClientWebSocket in the _Socket field which is preventing mocking of the un-managed resource in a unit test.
This will need to be addressed through some form of IOC; either with the introduction of a DI container or some other design pattern such as the injection of a ClientWebSocket factory object alongside the existing logger factory
Is there any news on a decision in relation to this, please? I would like to add more unit tests but that depends on the ability to mock the web socket. I don't feel it is my place to implicitly impose an arbitrary decision on this important architectural change by bundling any particular solution as part of such an un-related UnitTest PR.
Additionally to indirection in the creation of the ClientWebSocket and management of its lifetime, we may also want to wrap it in a facade allowing the API to be simplified to only those operations needed by the codebase. At the same time, this would allow a way to introduce an Interface which underlying type lacks, which will help implement any indirection method mentioned in the original comment.
Happy for you to do on stream and I'll pick up the testing after. It will be mid-afternoon on sunday for me so I will try and be in the chat if I can.