In C#, what is the best way handle dependency inject a class which consumes multiple interfaces?

Question

I have a class which consumes Renci.SshNet's SftpClient which implements BaseClient, ISftpClient.

So, my constructor looks like:

    public CollendaSftpClient(
            CollendaCsvWriter collendaCsvWriter,
            CollendaSftpConfig collendaSftpConfig,
            ILogger<CollendaSftpClient> logger,
            ISftpClient sftpClient
        )
    {
        _collendaCsvWriter = collendaCsvWriter ?? throw new ArgumentNullException(nameof(collendaCsvWriter));
        _collendaSftpConfig = collendaSftpConfig ?? throw new ArgumentNullException(nameof(collendaSftpConfig));
        _logger = logger ?? throw new ArgumentNullException(nameof(logger));
        _sftpClient = sftpClient ?? throw new ArgumentNullException(nameof(sftpClient));
    }

Annoyingling, ISftpClient does not include either the Connect() or Disconnect() methods, both of which are coming from BaseClient and included in IBaseClient (which BaseClient implements.

Thus, I have an upload method which looks like:

    private void Upload(MemoryStream memoryStream)
    {
        _logger.InterpolatedInformation($"Attempting to upload to {_collendaSftpConfig.Host:SftpHost}");
        try
        {
            SftpClient? sftpClient = _sftpClient as SftpClient;
            if (sftpClient != default)
            {
                sftpClient.Connect();
            }

            string filePath = _collendaSftpConfig?.FilePath
                ?.InsertTimestamp()
                ?? throw new InvalidOperationException("CollendaSftpConfig configuration is missing FilePath.");

            // Renci.SshNet's Sftp Client seems to have some async support, but it seems much more complicated to consume.
            // There is no clear benefit to using it at this time.
            _sftpClient.UploadFile(memoryStream, filePath);
            if (sftpClient != default)
            {
                sftpClient.Disconnect();
            }
        }
        catch (Exception ex)
        {
            CollendaSftpException sftpException = new($"Failed to upload to {_collendaSftpConfig.Host:SftpHost}", ex);
            _logger.InterpolatedError(sftpException, $"Upload failure: {ex.Message:ErrorMessage}");
            throw sftpException;
        }
    }

Needing to check whether I've injected an instance of something else looks ugly and not especially testable... Nor does it seem like a great idea to add it to my services and inject it twice.

Short of asking Renci to fix their interace, what is the best way to deal with this?

Answer 1

May I suggest a small redesign of your application? Prefer to let application code depend only on application-specific abstractions, conforming to the Dependency Inversion Principle.

In your case CollendaSftpClient depends on the third-party ISftpClient abstraction and SftpClient class. These types are not designed specifically for your application, which causes testability issues, as you already noticed.

Instead, hide SftpClient behind an application-specific abstraction, for instance:

public interface IFileUploader
{
    void Upload(string path, MemoryStream data);
}

This abstraction contains solely an Upload method. It lacks Connect and Disconnect methods, because the consumers of IFileUpload will (or should) not be concerned in making and breaking the connection. This can be done in the background, by the implemention of IFileUploader that adapts to SftpClient. For instance:

public sealed class SftpClientFileUploaderAdapter : IFileUploader
{
    public SftpClientFileUploaderAdapter(SftpClient sftpClient) ...
    
    public void Upload(string path, MemoryStream data)
    {
        _sftpClient.Connect();
        
        try
        {
            _sftpClient.UploadFile(data, filePath);
        }
        finally
        {
            sftpClient.Disconnect();
        }
    }
}

This adapter depends directly on the SftpClient class—not the interface. It is allowed to do this, because this adapter will be part of your Composition Root or Anti-Corruption Layer (depending on which terminology you follow). This allows it to connect and disconnect.

Note that this class lacks logging. That's because logging is a cross-cutting concern and cross-cutting concerns are best placed in their own class. For instance, you can create this Decorator for IFileUploader:

public sealed class LoggingFileUploader : IFileUploader
{
    public LoggingFileUploader(
        CollendaSftpConfig collendaSftpConfig,
        ILogger logger,
        IFileUploader decoratee) ...
    
    public void Upload(string path, MemoryStream data)
    {
        _logger.InterpolatedInformation(
            $"Attempting to upload to {_collendaSftpConfig.Host:SftpHost}");
        
        try
        {
            _decoratee.Upload(path, data);
        }
        catch (Exception ex)
        {
            CollendaSftpException sftpException = new(
                $"Failed to upload to {_collendaSftpConfig.Host:SftpHost}", ex);
            _logger.InterpolatedError(sftpException,
                $"Upload failure: {ex.Message:ErrorMessage}");
            throw;
        }        
    }
}

This Decorator implements IFileUploader while it also wraps and uses an IFileUploader itself. This allows to 'extend' the behavior of the SftpClientFileUploaderAdapter while using Composition over Inheritance.

Note that this piece of catching and logging of exceptions would raise a red flag when I do a code review over such piece of code. In general, when you let the exception bubble up the call stack (using the throw keyword) there's no need to log that exception in the catch block. Instead, you typically let the exception bubble up and log the exception at the top of the call stack.

In this case I, therefore, find it more reasonable to add more details to the exception message (as you are doing) and simply throw that new message. For instance:

catch (Exception ex)
{
    throw new CollendaSftpException(
        $"Failed to upload to {_collendaSftpConfig.Host:SftpHost}", ex);
}        

This leaves the following code in the CollendaSftpClient class:

public CollendaSftpClient(
    CollendaSftpConfig collendaSftpConfig,
    IFileUploader fileUploader) ...

private void Upload(MemoryStream memoryStream)
{
    string filePath = _collendaSftpConfig?.FilePath?.InsertTimestamp();
    
    if (filePath is null) throw new InvalidOperationException(
        "CollendaSftpConfig configuration is missing FilePath.");

    _fileUploader.UploadFile(filePath, memoryStream);
}

Inside your Composition Root, you can now construct this new object structure as follows:

var config = new CollendaSftpConfig();
var ftpClient = new SftpClient { ... };
new CollendaSftpClient(
    config,
    new LoggingFileUploader(
        config,
        new Logger<LoggingFileUploader>(),
        new SftpClientFileUploaderAdapter(ftpClient)));

Or in case you're using MS.DI as your DI Container:

services.AddTransient<CollendaSftpClient>();
services.AddTransient<IFileUploader>(sp =>
    ActivatorUtilities.CreateInstance<LoggingFileUploader>(sp,
        sp.GetRequiredService<ILogger<LoggingFileUploader>>(),
        ActivatorUtilities.CreateInstance<SftpClientFileUploaderAdapter>(sp)));

Please note that the suggestions above are written with just the information extracted from your question, with no knowledge about your application and domain. Please adapt this code to fit your application or use the ideas presented to improve your application design. Chances are slim that these code samples are the best-fit your application.