Passing Services using Dependency Injection and Factory Pattern in ASP.NET

Question

I am using ASP.NET Core, I know that such Logging mechanism is already provided by the framework, but using this to illustrate my problem.

I am using kind of Factory pattern to build the Logger class, since I don't know the type of logging (because it is stored in DB).

The ILogger Contract

Log(string msg)

Then LoggerFactory will return an ILogger after creating a Logger based on param passed from DB:

public class LoggerFactory
{
    public static Contracts.ILogger BuildLogger(LogType type)
    {
        return GetLogger(type);
    }

//other code is omitted, GetLogger will return an implementation of the related logger

Now, when I need to use the Logger I have to do it in this way:

  public class MyService
{
    private ILogger _logger
    public MyService()
    {
        _logger = LoggerFactory.BuildLogger("myType");
    }

But, I intend to keep my classes without any instantiation, I need to use Constructor DI in MyService and I need to inject all the dependencies on Startup:

    services.AddTransient<Contracts.ILogger, LoggerFactory.BuildLogger("param") > ();

But this will not work this we need to pass a concrete implementation. How to make that work using DI, is there a better approach for implementing that?

Gadhelic

One option is to inject the LoggerFactory, instead of a ILogger. This way, you get the LoggerFactory, get the Type from your DB and build your ILogger. If you want to inject the ILogger you would have to know the Type of your ILogger in the moment of the injection. But you may not have this info yet. – Information

You mean pass the LoggerFactory in MyService Constructor and don't add it to the .net core services – Gadhelic

Yes. Like this: services.AddSingleton<LoggerFactory>();. Singleton means you'll create a LoggerFactory object only in the first time you need it. After this, you'll use only the same. Then, you use like this: public MyService(LoggerFactory loggerFactory) {....} – Information

Answer 1

There are a couple of errors in your approach:

• Your services depend on the concrete LoggerFactory type which is a Dependency Inversion Principle violation.
• Doing this extra initialization can make building the object graph unreliable, while injection constructors should be simple.
• It hides the fact that the ILogger is the real service that your consumer depends upon. This makes the system harder to test, harder to maintain, and complicates object graph analysis.
• The use of a factory is a smell, since factories are hardly ever the right solution.

Instead, your service should look as follows:

public class MyService
{
    private ILogger _logger;
    public MyService(ILogger logger)
    {
        _logger = logger;
    }
}

This dramatically simplifies all consumers that depend upon ILogger. This also means that getting the right ILogger for MyService becomes a responsibility of the Composition Root, which is the correct place to have this knowledge.

It does mean however that you might need to move away from the built-in DI container of ASP.NET Core to a more feature rich DI library, because the built-in container is not capable of making a context aware registration for ILogger while having the library auto-wire other constructor dependencies as well.

With the ASP.NET Core DI container, you can only hand-wire your services using a delegate. For instance:

services.AddTransient<MyService>(c => new MyService(
    BuildLogger(typeof(MyService).Name),
    c.GetRequiredService<ISomeOtherDependency>(),
    c.GetRequiredService<IYetAnotherOne>());

Heinie

Steven, with your example, he can't inject the correct ILogger, right? If he knows the type before instantiate MyService, then he can change the constructor to public MyService(ILogger<TypeA> logger). But since he doesn't know that, he must inject a interface that can do that for him. Correct me if I'm wrong. – Information

@FabricioKoch: I'm not sure I follow you. Besides this, I would not advise introducing a generic abstraction here, because MyService is completely uninterested in this generic type; it just wishes to log. It is therefore a problem of the Composition Root to supply the correct type, not of the consumer. – Heinie

@Steven, I understand from you that this is not doable with ASP Core DI Container since it doesn't provide the Composition Root? If the answer was Yes, how to do that using other Libraries like Ninject or AutoFac? – Gadhelic

@h.salman Every application contains a Composition Root. Best is to hide the framework's ILogger abstraction behind an application-specific abstraction, as described here and implement an adapter to MS.Extensions.Logging as described here. This documentation explains how to wire up this adapter using Simple Injector. If you will have to Google if you want to know how to wire this up using Ninject or Autofac. – Heinie

@Steven, I definitely agree with what you are saying and very thanks to your clear explanations. But, If I want to use DI, I would pass the Interface and the implementation to the container. However, in my case I don't know the Implementation since its a db parameter. This means I have just one way to do this , by writing `if else statement' check the db param type and then pass the concrete implementation. If there is another way to do that, please let me know. – Gadhelic

@h.salman: You should not build your object graphs based on runtime data, just as you should not construct components using runtime data. Instead create a proxy implementation for your ILogger abstraction that can call the database and forward the call to the real implementation. This way you can add this without complicating consumers of ILogger and prevent complicating your Composition Root. – Heinie

@Steven, Can you please provide Pseudo code for this or any example – Gadhelic

OK, I will post another question and link it to this one with my real code. – Gadhelic

@Steven, I asked new question and linked it to this one. – Gadhelic

@Heinie if I understand your idea, the ILogger proxy would do the job of the factory, without the consumer knowing it, right? So the consumer would still rely on ILogger (rather than ILoggerFactory), and internally the proxy would forward the call to the proper concrete ILogger. That sounds much better than making consumers relying on some sort of ILoggerFactory. – Methodius