Return to Revisions

configuration is usually accessed statically

I have a different impression. Configuration is accessed in a way that is convenient in terms of maintainability and testing. And static references do not help in those areas. It is common to register settings in DI containers and either inject them directly or using some sort of wrapper (probably what you have with IOptions interface). There is nothing wrong with that.

constructor has a lot of parameters, but I don't think that it has too much responsibility

IMHO, it has everything to do with responsibility. Your class manages too many entities. I can think of two patterns that can be useful here:

• A decorator pattern. Take a look at how streams are organized. How you can wrap a buffering stream around a file stream to get a file stream with a large buffer. Don't you think that, for example, encryption can be added to existing connection by wrapping around it? It will require a change in signature, but maybe it is a good thing?
• A pipeline pattern. Why should connection know every step which needs to be taken to process a message. First decrypt it, then create reader, then pass a handler, then continue processing inside a handler... That sounds like a pipeline to me. Wouldn't it be nicer if your connection was only responsible for reading an array of bytes? Once you have the binary packet you could just pass it to some IMessagePipeLine.Process(byte[]) method. Or maybe just fire an event? P.S. If you are interested in this approach check out TPL.Dataflow library, it has some neat classes for building an async pipeline, and I believe it is available under .Net Core.

a lot of code just to build up a bunch of objects using config values

Is there a reason why you can't inject the settings directly in your component? Something along those lines:

//register settings inside container
services.AddSingleton<CryptoSettings>(...);
//register crypto service wich has a dependency on settings class
services.AddSingleton<ICryptor, Cryptor>();

class Cryptor : ICryptor
{
     public Cryptor(CryptoSettings settings) {...}
     ...
}

The point of DI containers is that you let container resolve dependencies for you. You on the other hand do most of the heavy lifting manually.

That being said, complex application can require hundreds or even thousands of lines just to register all the components. That can be tedious, but it is a price that you have to pay for DI container. However it is worth mentioning, that containers provide some powerful features when it comes to registration. For example, most containers allow you to automatically register all services that implement an interface / located in certain namespace / marked with an attribute. Some containers has build-in support for configuration files and can inject configuration values into registered components directly from xml file, as long as the file structure follows specified format.

A few other things that I noticed:

1. What is this infinite loop doing inside your program class? Apart from periodically stealing processor core from other threads? Why not run your listener inside the main thread synchronously?

2. There is no clean up on Program close. Connections are never terminated and DI container is never disposed.

3. _handler.Handle(this, reader); - this should rise red flags. Circular dependencies are almost always an indication of some poor design choices. So whenever you have to pass a this reference to a method, try to figure out why it is necessary. Maybe instead what you actually need is another service, that both classes ("handler" and "connection") can depend on.