What happens to using statement when I move to dependency injection

Question

I am currently using the following code:

public class MyProvider
{
    public MyProvider()
    {
    }

    public void Fetch()
    {
        using (PopClient popClient = new PopClient())
        {
            ....
        }
    }
}

Because I want to be able to unit test the Fetch method and due to the fact that I can't mock PopClient, I created an interface and a wrapper class that calls into PopClient. My updated code looks like:

public class MyProvider
{
    private readonly IPopClient popClient;

    public MyProvider(IPopClient popClient)
    {
        this.popClient = popClient;
    }

    public void Fetch()
    {
        using (var pop3 = popClient)
        {
            ....
        }
    }
}

I am using Ninject for dependency injection and I am not quite sure what kind of effect the using statement will have in the updated code since Ninject already created an instance of PopClient and injected it into the constructor.

Will the using statement dispose of pop3 object and leave the popClient object alone so Ninject can handle it or will the using statement interfere with Ninject?

What is the proper approach in this case? Any insight would be very helpful.

Fagaly

If you dispose popClient, what happens to MyProvider object ?! Shouldn't it be disposed as well ?! Because there is left an only disposed readonly member in the class. If it is so, I think it is better to put a dispose method for MyProvider class. – Reincarnate

Answer 1

The pop3 variable will be given the same reference to an IPopClient object that popClient has, so when the using statement is over, the object referred to by both the local and instance variables will be Dispose()d, probably placing it in an inconsistent state for further use.

If you want to use multiple instances of IPopClient, one per Fetch() call, what you should do is inject a "factory method":

public class MyProvider
{
    private readonly Func<IPopClient> createPopClient;

    public MyProvider(Func<IPopClient> popClientFactory)
    {
        this.createPopClient = popClientFactory;
    }

    public void Fetch()
    {
        using (var pop3 = createPopClient())
        {
            ....
        }
    }
}

Now, when you call Fetch(), it will execute the factory method which will return a new reference to an IPopClient, which can be used and then disposed of without affecting any other call to that method.

AutoFac supports injecting factory methods for registered types without any additional setup (hence it's name, I think); I believe when configuring a Ninject container you are required to explicitly register a "getter" as the factory method for a given return type (which can be as simple as a lambda ()=>new PopClient() or it can use a call to the container's resolution method).

Latchkey

better to create a factory interface. The intent is more clear. +1 either way for factory pattern – Cracking

As long as you have Ninject.Extensions.Factory.dll in your AppDomain.BaseDirectory, Func<T> is autogenerated - see the Ninject.Extensions.Factory wiki (i.e. no need to register anything) – Errata

I will second the comment made by @Cracking above. A factory interface is a bit more work, but it will pay off in the long run. The intent will be clearer, and mocking is also easier. – Serve

Answer 2

When setting up your bindings, declare the scope:

https://github.com/ninject/ninject/wiki/Object-Scopes

Ninject will call dispose on the objects it created for you, so make sure you write up your dispose methods in any objects you give to Ninject to handle.

Sanyu

The problem is that with it set up the way it currently is, MyProvider only ever gets one instance of an IPopClient object to use; that means that if code makes multiple calls to Fetch() using one instance of MyProvider, this will fail no matter what the registered scope of IPopClient is. – Latchkey

Yes, the Fetch method will be called multiple times so in the current code (no DI) I was always guaranteed a new pop instance because of the using statement. In the DI code I believe transient scope will only create one instance that will be used by multiple calls and the using statement will dispose of that instance after the first call is done. – Fagaly

@Fagaly - that is exactly correct - see my answer. In short, if you want to keep the using statement, you don't want to inject a single instance, but instead a factory method that you can call to produce as many instances as you like. – Latchkey

Answer 3

I like the answer from @KeithS but I would like to add some updates with ConfigureServices, .NET Core dependency injection and MailKit.Net.Smtp.

https://learn.microsoft.com/en-us/dotnet/core/extensions/dependency-injection

Given the class below:

public class MailService
{
    private readonly SmtpSettings _settings;
    private readonly Func<ISmtpClient> _smtpClientFactory;

    public MailService(SmtpSettings settings, Func<ISmtpClient> smtpClientFactory)
    {
        _settings = settings;
        _smtpClientFactory = smtpClientFactory;
    }

    public async Task SendEmailAsync(string to, string subject, MimeEntity body, CancellationToken cancellationToken = default)
    {
        var message = new MimeMessage();
        message.From.Add(MailboxAddress.Parse(_settings.UserName));
        message.To.Add(MailboxAddress.Parse(to));
        message.Subject = subject;
        message.Body = body;

        using (var smtpClient = _smtpClientFactory())
        {
            await smtpClient.ConnectAsync(_settings.Server, _settings.Port, SecureSocketOptions.StartTls, cancellationToken);
            await smtpClient.AuthenticateAsync(_settings.UserName, _settings.Password, cancellationToken);
            await smtpClient.SendAsync(message, cancellationToken);
            await smtpClient.DisconnectAsync(true, cancellationToken);
        }
    }
}

It can be added like this to ConfigureServices(IServiceCollection services):

services.AddTransient<ISmtpClient, SmtpClient>();
services.AddSingleton(provider =>
                 new Func<ISmtpClient>(() => provider.GetService<ISmtpClient>()));

Complete Program.cs example for Azure Function:

public static void Main()
{
    var host = new HostBuilder()
        .ConfigureFunctionsWorkerDefaults()
        .ConfigureAppConfiguration((hostContext, builder) =>
        {
            if (hostContext.HostingEnvironment.IsDevelopment())
            {
                builder.AddJsonFile("local.settings.json");
                //This will override any values added to local.settings.json - We will however follow the recommended approach for keeping secrets in dev 
                //https://learn.microsoft.com/en-us/aspnet/core/security/app-secrets?view=aspnetcore-5.0&tabs=windows#register-the-user-secrets-configuration-source
                //builder.AddJsonFile("secret.settings.json");
                builder.AddUserSecrets<Program>();
            }

        })
        .ConfigureServices((hostContext, services) =>
        {
            var connectionString = hostContext.Configuration.GetConnectionString("DefaultConnection");
            services.AddDbContext<ApplicationDbContext>(options =>
                    options.UseSqlServer(
                    connectionString,
                    sqlServerOptions => sqlServerOptions.CommandTimeout(600)));

            services.AddHttpClient();

            var configuration = hostContext.Configuration;

            var smtpSettings = new SmtpSettings();
            configuration.Bind("Smtp", smtpSettings);
            services.AddSingleton(smtpSettings);

            services.AddTransient<ISmtpClient, SmtpClient>();

            services.AddSingleton(provider =>
                 new Func<ISmtpClient>(() => provider.GetService<ISmtpClient>()));

            services.AddTransient<MailService>();
        })
        .Build();

    host.Run();
}

Source to get Func<T> registered and resolved:

https://mcmap.net/q/498660/-how-to-use-func-lt-t-gt-in-built-in-dependency-injection

Ennius