Блог

Хорошо, технически они не являются «неиспользуемыми», но они никогда не используются классом, который их содержит, так что, я думаю, в некотором смысле они есть. Вот гипотетическая ситуация, на которую я пытаюсь найти ответ.

У меня есть класс NetworkListener ,

 public NetworkListener(IPAddress ipAddress, int port, NetworkClientPacketRepository packetRepository)
  

NetworkListener иногда приходится создавать новый класс, который находится вне корня композиции:

 new NetworkClient(await _listener.AcceptTcpClientAsync(), _packetRepository, _logger);
  

Это плохая практика? Похоже на это, похоже на это, так и должно быть, верно? Я не уверен,

Несколько вещей, которые выделяются для меня, показывают, что это так…

A. NetworkListener должен принимать NetworkClientRepository зависимость только потому NetworkClient , что она нужна.

B. NetworkListener должен принимать Logger зависимость только потому NetworkClient , что она нужна.

Это кажется неправильным, но я не знаю лучшего способа, поэтому я делаю это таким образом.

Как я могу избежать использования неиспользуемых полей только для передачи в дочерний класс?

Вот мой корень композиции:

 services.AddSingleton<NetworkEventArguments>();
services.AddSingleton<NetworkClientRepository>();
services.AddSingleton<Dictionary<int, IClientPacket>>();
services.AddSingleton<ClientPacketRepository>();
services.AddSingleton(provider => new NetworkListener(
    IPAddress.Parse(config.GetValue<string>("Networking:Host")),
    config.GetValue<int>("Networking:Port"),
    provider.GetService<NetworkClientRepository>()
));
  

Класс NetworkListener:

 public class NetworkListener : IDisposable
{
    private readonly NetworkClientRepository _clientRepository;
    private readonly ClientPacketRepository _packetRepository;
    private readonly TcpListener _listener;

    public NetworkListener(IPAddress ipAddress, int port, NetworkClientRepository clientRepository, ClientPacketRepository packetRepository)
    {
        _clientRepository = clientRepository;
        _packetRepository = packetRepository;
        _listener = new TcpListener(ipAddress, port);
    }

    public void Start(int backlog = 100)
    {
        _listener.Start(backlog);
    }

    public async Task ListenAsync()
    {
        while (true)
        {
            HandleIncomingConnection(await _listener.AcceptTcpClientAsync());
        }
    }
    
    public event EventHandler<NetworkEventArguments> ClientConnected;
    
    private void HandleIncomingConnection(TcpClient client)
    {
        var networkClient = new NetworkClient(client, _packetRepository);
        
        _clientRepository.AddClient(networkClient);
        
        ClientConnected?.Invoke(this, new NetworkEventArguments
        {
            Client = networkClient
        });
    }

    public void Dispose()
    {
        _listener.Server.Close();
        _listener.Server.Dispose();
    }
}
  

Класс NetworkClient:

 public class NetworkClient : IDisposable
{
    public TcpClient TcpClient { get; }
    
    private readonly ClientPacketRepository _packetRepository;
    private readonly ILogger _logger;
    private readonly NetworkStream _networkStream;

    public NetworkClient(TcpClient tcpClient, ClientPacketRepository packetRepository, ILogger logger)
    {
        TcpClient = tcpClient;
        
        _packetRepository = packetRepository;
        _logger = logger.ForContext<NetworkClient>();
        _networkStream = tcpClient.GetStream();

        ProcessDataAsync();
    }
    
    private void ProcessDataAsync()
    {
        var thread = new Thread(() =>
        {
            // TODO: Read from tcpClient
        });
        
        thread.Start();
    }

    public void Dispose()
    {
        TcpClient.Dispose();
    }
}
  

Это плохая практика? Похоже на это, похоже на это, так и должно быть, верно? Я не уверен,

Это, безусловно, так. Ваши классы должны создавать экземпляры только классов данных или сторонних сервисов, которые невозможно внедрить. Создание экземпляра TcpListener является хорошим примером последнего.

Ваша проблема возникает отсюда:

 public NetworkClient(TcpClient tcpClient, ClientPacketRepository packetRepository, ILogger logger)
  

Старайтесь избегать смешивания зависимостей и «данных» в конструкторе. Вместо этого позвольте конструктору иметь только параметры, которые можно вводить, и поместите остальные параметры в соответствующие методы.

Вот как NetworkClient должно выглядеть:

 public interface INetworkClient 
{
    void ProcessDataAsync(TcpClient tcpClient);
}

public class NetworkClient : INetworkClient, IDisposable
{
    private readonly ClientPacketRepository _packetRepository;
    private readonly ILogger _logger;

    public NetworkClient(ClientPacketRepository packetRepository, ILogger logger)
    {
        _packetRepository = packetRepository;
        _logger = logger.ForContext<NetworkClient>();
    }
    
    // this Async in the name and the Thread being created internally looks smelly
    private void ProcessDataAsync(TcpClient tcpClient)
    {
        var networkStream = tcpClient.GetStream();
        // ...
    }

    // ...
}
  

Тогда у вас мог NetworkClientFactory бы быть класс, который просто генерирует новый экземпляр NetworkClient , и вы бы внедрили эту фабрику NetworkListener .

 public interface INetworkClientFactory
{
    INetworkClient Create();
}

public class NetworkClientFactory : INetworkClientFactory
{
    private readonly IServiceProvider _serviceProvider;

    public NetworkClientFactory(IServiceProvider serviceProvider)
    {
        _serviceProvider = serviceProvider;
    }

    // getting a specific service like this is normally considered an anti-pattern, 
    // however, it's generally accepted in factories when the DI system doesn't provide better ways
    public INetworkClient Create() => 
       _serviceProvider.GetRequiredService<INetworkClient>();
}
  

Итак, вы бы использовали это с:

 private void HandleIncomingConnection(TcpClient client)
{
    var networkClient = _networkClientFactory.Create();
    netowrkClient.ProcessDataAsync(client);
    
    _clientRepository.AddClient(networkClient);
    
    ClientConnected?.Invoke(this, new NetworkEventArguments
    {
        Client = networkClient
    });
}