Dependency injection - Place logic in the overloaded constructor?

Question

Today I've got this class:

public class SmtpEmailProvider : IMessageProvider
{
    private readonly SmtpClientWrapper _smtpClientWrapper;

    public SmtpEmailProvider(SmtpClientWrapper smtpClientWrapper)
    {
        _smtpClientWrapper = smtpClientWrapper;
    }

To be able to mock the SmtpClient, I've wrapped it like this:

public class SmtpClientWrapper
{
    private readonly SmtpClient _smtpClient;

    public SmtpClientWrapper(SmtpClient smtpClient)
    {
        _smtpClient = smtpClient;
    }

    public virtual void Send(MailMessage msg)
    {
        if (_smtpClient == null) throw new InvalidOperationException("SmtpClient must be passed to the constructor before calling Send.");

        _smtpClient.Send(msg);
    }
}

Right now I can do this to initiate the SmtpEmailProvider class, and place the SmtpClient logic there:

public IMessageProvider LocateProviderByName(string providerName)
{
    var client = new SmtpClient
                     {
                         Host = "127.0.0.1",
                         Port = 25
                     };
    client.Credentials = new NetworkCredential("...", "...");
    return new SmtpEmailProvider(new SmtpClientWrapper(client));
}

But I want to replace that with:

public IMessageProvider LocateProviderByName(string providerName)
{
    return IoC.Resolve<IMessageProvider>(providerName);
}

Then I need to place the logic in the constructor with no parameters. But I get the feeling that I'm doing to much in the contructor then.

Is there any other way to do it?

Answer 1

I'm a little confused about what, exactly, you are trying to accomplish. If I am to assume you need to provide Smtp emailers, and you are using IoC, then you should be creating and wiring up your entire object graph with the IoC framework. By that, I mean that you would configure your IoC framework to create the SmtpClient, which it then creates the SmtpClientWrapper with, finally creating the SmtpEmailProvider with. You should not need to put any dependency-creation logic in the SmtpEmailProvider constructor.

Here is an example with Castle Windsor, given the code you provided:

<configuration>
  <component id="smtpClient" type="System.Net.Mail.SmtpClient, System">
    <parameters>
      <Host>127.0.0.1</Host>
      <Port>25</Port>
    </parameters>
  </component>
  <component id="smtpClientWrapper" type="Naespace.SmtpClientWrapper, Assembly">
    <parameters>
      <smtpClient>${smtpClient}</smtpClient>
    </parameters>
  </component>
  <component id="smtpProvider" service="Namespace.IMessageProvider, Assembly" type="Namespace.SmtpEmailProvider, Assembly">
    <parameters>
      <smtpClientWrapper>${smtpClientWrapper}</smtpClientWrapper>
    </parameters>
  </component>
</configuration>

With the above Windsor configuration, you can simply create your IMessageProvider like so:

public IMessageProvider LocateProviderByName(string providerName)
{
    return IoC.Resolve<IMessageProvider>(providerName);
}

var messageProvider = LocateProviderByName("smtpProvider");

The key point here is to use the IoC container for what it is: a dependency creation and management system that can, and should, create full object graphs for you. This alleviates the problem you have with too much dependency management logic in a constructor.