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?

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. — Fabricio Koch, Nov 09 '16 at 18:40
You mean pass the LoggerFactory in MyService Constructor and don't add it to the .net core services — Hussein Salman, Nov 09 '16 at 18:50
Yes. Like this: `services.AddSingleton();`. 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) {....}` — Fabricio Koch, Nov 09 '16 at 18:55

score 19 · Accepted Answer · edited Feb 14 '19 at 18:02

19

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>());

edited Feb 14 '19 at 18:02

Brandon

984
1
11
26

answered Nov 09 '16 at 19:08

Steven

166,672
24
332
435

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 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. – Fabricio Koch Nov 09 '16 at 19:28
2

@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. – Steven Nov 09 '16 at 19:31
@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? – Hussein Salman Nov 10 '16 at 03:18
2

@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](https://stackoverflow.com/q/5646820/264697) and implement an adapter to MS.Extensions.Logging as described [here](https://stackoverflow.com/a/39610057/264697). [This documentation](https://simpleinjector.readthedocs.io/en/latest/advanced.html#context-based-injection) 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. – Steven Nov 10 '16 at 06:59
@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. – Hussein Salman Nov 10 '16 at 14:24
1

@h.salman: You should not build your object graphs based on runtime data, just as [you should not construct components using runtime data](https://www.cuttingedge.it/blogs/steven/pivot/entry.php?id=99). 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. – Steven Nov 10 '16 at 14:28
@Steven, Can you please provide Pseudo code for this or any example – Hussein Salman Nov 10 '16 at 14:35
OK, I will post another question and link it to this one with my real code. – Hussein Salman Nov 10 '16 at 14:44
@Steven, I asked new question and linked it to this one. – Hussein Salman Nov 10 '16 at 16:06
@Steven 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`. – Alisson Reinaldo Silva Feb 12 '19 at 18:05

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

1 Answers1

Linked