What are the options for getting a normally injected logger to a class that is created outside of DI container?

Question

I know that Service Locator pattern is out of favour. When you have something like Global.Kernel.Get<IService>() in your code the consultant will detect a code smell. So where possible, we should be using constructor/property or method injection.

Now let's consider a common task of logging with Ninject. There is a couple of extensions such as ninject.extensions.logging and Ninject.Extensions.Logging.Log4net that "just work". You reference them in your project and then whenever you inject ILogger you get a wrapped instance of log4net logger that is named after the type you are injecting into. You don't even have a dependency on log4net anywhere in you code, just in the place where you provide it with the configuration. Great!

But what if you need a logger in a class that is not created in a DI container? How do you get it there? It can't be auto injected because the class is created outside of the DI, you can do Global.Kernel.Inject(myObject) because it would be wrong to depend on the DI not in the composition root as explained in the article linked above, so what options do you really have?

Below is my attempt at it, but I do not like it because it feels mighty awkward. I'm actually injecting ILoggerFactory instead of ILogger to the class that creates outside-of-DI object and then pass the ILoggerFactory to the outside-of-DI object in the constructor.

Another way of achieving the same would be to introduce the log4net dependency into the outside-of-DI class, but that also feels backward.

How would you solve this?

And here is a code example:

using System.Threading;
using Ninject;
using Ninject.Extensions.Logging;

namespace NjTest
{
    // This is some application configuration information
    class Config
    {
        public string Setting1 { get; set; }
    }

    // This represent messages the application receives
    class Message
    {
        public string Param1 { get; set; }
        public string Param2 { get; set; }
        public string Param3 { get; set; }
    }

    // This class represents a worker that collects and process information
    // in our example this information comes partially from the message and partially
    // driven by configuration
    class UnitCollector
    {
        private readonly ILogger _logger;
        // This field is not present in the actual class it's
        // here just to give some exit condition to the ProcessUnit method
        private int _defunct;
        public UnitCollector(string param1, string param2, ILoggerFactory factory)
        {
            _logger = factory.GetCurrentClassLogger();
            // param1 and param2 are used during processing but for simplicity we are
            // not showing this
            _logger.Debug("Creating UnitCollector {0},{1}", param1, param2);
        }

        public bool ProcessUnit(string data)
        {
            _logger.Debug("ProcessUnit {0}",_defunct);
            // In reality the result depends on the prior messages content
            // For simplicity we have a simple counter here
            return _defunct++ < 10;
        }
    }

    // This is the main application service
    class Service
    {
        private readonly ILogger _logger;
        private readonly Config _config;

        // This is here only so that it can be passed to UnitCollector
        // and this is the crux of my question. Having both
        // _logger and _loggerFactory seems redundant
        private readonly ILoggerFactory _loggerFactory;

        public Service(ILogger logger, ILoggerFactory loggerFactory, Config config)
        {
            _logger = logger;
            _loggerFactory = loggerFactory;
            _config = config;
        }

        //Main processing loop
        public void DoStuff()
        {
            _logger.Debug("Hello World!");
            UnitCollector currentCollector = null;
            bool lastResult = false;
            while (true)
            {
                Message message = ReceiveMessage();
                if (!lastResult)
                {
                    // UnitCollector is not injected because it's created and destroyed unknown number of times
                    // based on the message content. Still it needs a logger. I'm here passing the logger factory
                    // so that it can obtain a logger but it does not feel clean
                    // Another way would be to do kernel.Get<ILogger>() inside the collector
                    // but that would be wrong because kernel should be only used at composition root
                    // as per http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern/
                    currentCollector = new UnitCollector(message.Param2, _config.Setting1,_loggerFactory);
                }
                lastResult = currentCollector.ProcessUnit(message.Param1);

                //message.Param3 is also used for something else
            }
        }
        private Message ReceiveMessage()
        {          
            _logger.Debug("Waiting for a message");
            // This represents receiving a message from somewhere
            Thread.Sleep(1000);
            return new Message {Param1 = "a",Param2 = "b",Param3 = "c"};
        }
    }

    class Program
    {
        static void Main()
        {
            log4net.Config.XmlConfigurator.Configure();
            IKernel kernel = new StandardKernel();
            kernel.Bind<Config>().ToMethod(ctx => new Config {Setting1 = "hey"});
            Service service = kernel.Get<Service>();
            service.DoStuff();

        }
    }
}

App.Config:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <configSections>
    <section name="log4net" type="log4net.Config.Log4NetConfigurationSectionHandler,log4net, Version=1.2.13.0, Culture=neutral, PublicKeyToken=669e0ddf0bb1aa2a" />
  </configSections>
  <startup> 
      <supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.5" />
  </startup>
  <log4net>
    <appender name="ConsoleAppender" type="log4net.Appender.ConsoleAppender">
      <layout type="log4net.Layout.PatternLayout">
        <conversionPattern value="%date [%thread] %-5level %logger - %message%newline" />
      </layout>
    </appender>
    <root>
      <level value="ALL" />
      <appender-ref ref="ConsoleAppender" />
    </root>
  </log4net>
  <runtime>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="log4net" publicKeyToken="669e0ddf0bb1aa2a" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.2.13.0" newVersion="1.2.13.0" />
      </dependentAssembly>
    </assemblyBinding>
  </runtime>

</configuration>

Answer 1

You should have a look at ninject.extensions.factory. You could either create an interface factory with corresponding binding:

internal interface IUnitCollectorFactory
{
    IUnitCollector Create(Param2Type param2);
}

Bind<IUnitCollectorFactory>().ToFactory();

or inject and use a Func-Factory Func<Param2Type>.

This way the IUnitCollectorFactory will be instantiated by the IoC and you can also inject the Config into it instead of passing a parameter.

An alternative would be to write your own factory which is doing the new()-ing, but i personally think the untestable code is often not really worth it. You're writing component- or specification-tests, aren't you? :) However, that's what Mark Seeman is basically recommending (see Can't combine Factory / DI) - which you had seen if you'd read all the comments of the blog post. The advantage of the abstract-factory is that it gets you closer to a true composition-root where you're instantiating as much as possible "on application startup" instead of deferring the instantiation of parts of the application to some later time. The drawback of late-instantiation is that starting the program will not tell you whether the whole object tree can be instantiated - there may be binding issues which only occur later.

Another alternative would be to adjust your design so that IUnitCollector is stateless - i.E. does not depend on getting Param2 as ctor parameter but rather as method parameter. That may, however, have other, more significant, drawbacks so you may choose to keep your design as is.

To apply the use of Func-Factory to your example change the beginning of Service class like this:

private readonly Func<string, string, UnitCollector> _unitCollertorFactory;        
public Service(ILogger logger, Config config, Func<string, string, UnitCollector> unitCollertorFactory)
{
    _logger = logger;
    _config = config;
    _unitCollertorFactory = unitCollertorFactory;
}

Note that you no longer need the _loggerFactory field and the ILoggerFactory constructor parameter. Then, instead of newing up UnitCollector, instantiate it like this:

currentCollector = _unitCollertorFactory(message.Param2, _config.Setting1);

Now you can substitute ILoggerFactory in the UnitCollector constructor to ILogger and it will get happily injected.

And don't forget that for this to work you need to reference ninject.extensions.factory.

Answer 2

Your specific case is explained clearly here in the Ninject contextual binding documentation. You can use the following registration:

Bind<ILog>().ToMethod(context => LogFactory.CreateLog(context.Request.Target.Type));

This will inject an ILog abstraction based on the type it is injected into, which is precisely what you seem to need.

This allows you to keep the calling of the factory inside your Composition Root and keeps your code free from any factories or the evil Service Location anti-pattern.

UPDATE

After taking a closer look at all your code I now see the root design 'flaw' that causing these problems. The root problem is that in the constructor of your UnitCollector you are mixing runtime data with dependencies and configuration data this is a bad thing and explained here, here and here.

If you move the runtime parameter param1 out of the constructor into the method, you completely solve the problem, simplify your configuration and allow your configuration to be verified at startup. This will look as follows:

kernel.Bind<UnitCollector>().ToMethod(c => new UnitCollector(
    c.Get<Config>().Setting1,
    LogFactory.CreateLog(typeof(UnitCollector))));

class UnitCollector {
    private readonly string _param2;
    private readonly ILogger _logger;
    public UnitCollector(string param2, ILogger logger) {
        _param2 = param2;
        _logger = logger;
    }

    public bool ProcessUnit(string data, string param1) {
        // Perhaps even better to extract both parameters into a
        // Parameter Object: https://bit.ly/1AvQ6Yh
    }
}

class Service {
    private readonly ILogger _logger;
    private readonly Func<UnitCollector> _collectorFactory;

    public Service(ILogger logger, Func<UnitCollector> collectorFactory) {
        _logger = logger;
        _collectorFactory = collectorFactory;
    }

    public void DoStuff() {
        UnitCollector currentCollector = null;
        bool lastResult = false;
        while (true) {
            Message message = ReceiveMessage();
            if (!lastResult) {
                currentCollector = this.collectorFactory.Invoke();
            }
            lastResult = currentCollector.ProcessUnit(message.Param1, message.Param2);
        }
    }
}    

Do note that in the Service there is now a factory (a Func<T>) for the UnitCollector. Since your whole application keeps looping in that DoStuff method, each loop can be seen as a new request. So on each request you will have to do a new resolve (which you are already doing, but now with the manual creation). Furthermore, you would often need to create some scope around such request to make sure that some instances that are registered with a 'per request' or 'per scope' lifestyle are created just once. In that case you will have to extract that logic out of this class to prevent mixing infrastructure with business logic.