Inject same scoped dependency inside itself-CodePudding

I'm learning dependency injection, because I don't want my BE to look spaghety no more. I have a good understanding of Asp.Net Core and EF Core. I just never learned dependecy injection properly. I'm playing around with an idea. Let's say, that I create an EmailSenderService (and IEmailSenderService with it). I do the same for CustomLogger and WeatherRepository. Here are the implementations:

Program.cs:

public static void Main(string[] args)
{
    var builder = WebApplication.CreateBuilder(args);

    // Add services to the container.
    builder.Services.AddControllers();
    builder.Services.AddScoped<ICustomLogger, CustomLogger>();
    builder.Services.AddScoped<IEmailSenderService, EmailSenderService>();
    builder.Services.AddScoped<IWeatherRepository, WeatherRepository>();

    // Add swagger
    builder.Services.AddEndpointsApiExplorer();
    builder.Services.AddSwaggerGen();

    var app = builder.Build();

    // Configure the HTTP request pipeline.
    if (app.Environment.IsDevelopment()) {
        app.UseSwagger();
        app.UseSwaggerUI();
    }

    app.UseHttpsRedirection();
    app.UseAuthorization();
    app.MapControllers();
    app.Run();
}

CustomLogger.cs

public interface ICustomLogger
{
    public void Log(string logText);
}

public class CustomLogger : ICustomLogger
{
    public void Log(string logText) => System.Diagnostics.Debug.WriteLine(logText);
}

EmailSenderService.cs

public interface IEmailSenderService
{
    public void SendMail(string email, string text);
}

public class EmailSenderService : IEmailSenderService
{
    public void SendMail(string email, string text) => System.Diagnostics.Debug.WriteLine($"TO: {email}, TEXT: {text}");
}

WeatherForecastModel.cs

public struct WeatherForecastModel
{
    public DateTime Date { get; set; }

    public int TemperatureC { get; set; }

    public int TemperatureF => 32   (int)(TemperatureC / 0.5556);

    public string? Summary { get; set; }
}

WeatherRepository.cs

public interface IWeatherRepository
{
    public WeatherForecastModel[] GetRandomSample();
}

public class WeatherRepository : IWeatherRepository
{
    private static readonly string[] Summaries = new[]
    {
        "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"
    };

    public WeatherForecastModel[] GetRandomSample() =>
        Enumerable.Range(1, 5).Select(index => new WeatherForecastModel
        {
            Date = DateTime.Now.AddDays(index),
            TemperatureC = Random.Shared.Next(-20, 55),
            Summary = Summaries[Random.Shared.Next(Summaries.Length)]
        }).ToArray();
}

WeatherForecastController.cs

[ApiController]
[Route("[controller]")]
public class WeatherForecastController : ControllerBase
{
    private readonly ICustomLogger _customLogger;
    private readonly IEmailSenderService _emailSenderService;
    private readonly IWeatherRepository _weatherRepository;

    public WeatherForecastController(ICustomLogger customLogger, IEmailSenderService emailSenderService, IWeatherRepository weatherRepository)
    {
        _customLogger = customLogger;
        _emailSenderService = emailSenderService;
        _weatherRepository = weatherRepository;
    }

    [HttpGet(Name = "GetWeatherForecast")]
    public IEnumerable<WeatherForecastModel> Get()
    {
        _customLogger.Log("Started function GetWeatherForecast");
        WeatherForecastModel[] results = _weatherRepository.GetRandomSample();
        _customLogger.Log("Started sending mail.");
        _emailSenderService.SendMail("[email protected]", $"Summary of the first result: {results[0].Summary}");
        _customLogger.Log("Ended sending mail.");
        _customLogger.Log("Ended function GetWeatherForecast");
        return results;
    }
}

Now, with the whole implementation, in place, I don't like it. Like visually. I do not want to see logging and email sending logic inside of my controller. This is the fundamentaly issue, I'm trying to solve with this question. I could (I implemented it for testing) inject logger inside the EmailSenderService and inside the WeatherRepository and log there, howerver, I do not like that either. I do not want to see logging inside of my logic. So, I thought about something I called LogAwareEmailSenderService. Here is impelementation:

public class LogAwareEmailSenderService : IEmailSenderService
{
    private readonly ICustomLogger _customLogger;
    private readonly IEmailSenderService _emailSenderService;

    public LogAwareEmailSenderService(ICustomLogger customLogger, IEmailSenderService emailSenderService)
    {
        _customLogger = customLogger;
        _emailSenderService = emailSenderService;
    }

    public void SendMail(string email, string text)
    {
        _customLogger.Log($"Started sending email to: {email}, containing text: {text}");
        _emailSenderService.SendMail(email, text);
        _customLogger.Log($"Done sending email to: {email}, containing text: {text}");
    }
}

Basically, what I'm trying to achieve, is: Take my original EmailSenderService, then inject it into my LogAwareEmailSenderService. The idea is, that now, I should be able to inject this LogAwareEmailSenderService into my controller without the need to change my controller at all (just remove my previous logging logic), right? And If I achieve this, I can go on and continue, to make something like LogAndEmailAwareWeatherRepository, that will inject LogAwareEmailSenderService and instead of sending mail and logging function start inside of the controller. I will just call LogAndEmailAwareWeatherRepository, that will log these things, and send the email, resulting in controller only calling the important, _weatherRepository.GetRandomSample() -- This call will do the logging and sending mail, using the previously described abstractions.

However, in the first place, I am unable to inject the EmailSenderService inside the LogAwareEmailSenderService. I want them both to be scoped. I trid this approach (in my Program.cs):

builder.Services.AddScoped<IEmailSenderService, EmailSenderService>();
builder.Services.AddScoped<IEmailSenderService, LogAwareEmailSenderService>();

however I got circular dependency error:

'Some services are not able to be constructed (Error while validating the service descriptor 'ServiceType: DependencyInjectionExample.Services.EmailSenderService.IEmailSenderService Lifetime: Scoped ImplementationType: DependencyInjectionExample.Services.EmailSenderService.LogAwareEmailSenderService': A circular dependency was detected for the service of type 'DependencyInjectionExample.Services.EmailSenderService.IEmailSenderService'.
DependencyInjectionExample.Services.EmailSenderService.IEmailSenderService(DependencyInjectionExample.Services.EmailSenderService.LogAwareEmailSenderService) -> DependencyInjectionExample.Services.EmailSenderService.IEmailSenderService)'

So, I got some questions:

Am I going about this right? Like, is what I described above, the normal approach to things?
Where should I put my Logging logic? When doing this, I also thought about caching things, meaning, that I would have something like CacheAwareWeatherRepository, that would only care about the caching implementation and then call the WeatherRepository to get data and return them, while caching them.
How to implement my solution?
I still don't understand some parts of dependecy injection, are there any articles/books that helped you personally understand it?

If you've got here, thank you, I know it is long, however I wanted to describe my problem, possible solutions, and questions clearly.

If you have any questions, about anything, please feel free to ask me in comments, or email me (if it's long question) to [email protected]. I would really like to get to the bottom of this.

PS: This is not about implementation of bussiness logic, or anything like this, this is only for getting data, logging it, caching it and doing abstractions above data access. I implemented this, with idea that you would have one interface and then layers of abstractions. One for getting the actual data (fAb), One for logging the fact (sAb) implementing fAb, One for caching data (tAb) implementing sAb, One for logging the fact of caching (qAb) implementing tAb. And so on.

CodePudding user response：

Am I going about this right? Like, is what I described above, the normal approach to things?

There's not a right/wrong, but what you're describing is an accepted pattern, called the decorator pattern. One service adds behaviors around another one while implementing the same interface.

Where should I put my Logging logic?

Depends on what you mean by "logging logic." The true logic of your logging (opening files and writing to them, e.g.) is already abstracted away behind the logger interface, as it should be.

If you like to have generic messages logged every time a public method is entered or exited, then you can do that with an aspect-oriented Fody Weaver to avoid repetitive code.

The lines of code that decide what to output as a log message, on the other hand, are mostly going to be specific to your implementation. The most useful diagnostic messages are probably going to be ones that need to know contextual information about your specific implementation, and that code needs to be embedded within the implementation code itself. The fact that you "visually don't want to see" calls to the logging service in your controller code is something you should get over. Diagnostic logging is a cross-cutting concern: a responsibility inherent to every class regardless of what their "single responsibility" is supposed to be.

How to implement my solution?

The DI registration method can take a delegate that lets you be more specific about how the type is resolved.

builder.Services.AddScoped<EmailSenderService>(); // allow the next line to work
builder.Services.AddScoped<IEmailSenderService>(p => new LogAwareEmailSenderService(
    p.GetRequiredService<ICustomLoggerService>(),
    p.GetRequiredService<EmailSenderService>()));

I still don't understand some parts of dependecy injection, are there any articles/books that helped you personally understand it?

Technically not the sort of question we're supposed to be asking on StackOverflow, but I learned from Mark Seeman's Dependency Injection in .NET (affiliate link). It's kind of old now, so library-specific details are outdated, but the principles are enduring.

CodePudding user response：

I thought it might be worth mentioning the Scrutor library which lets you easily specify the decoration setup. For example, you could have:

builder.Services.AddScoped<IEmailSenderService, EmailSenderService>();
builder.Services.Decorate<IEmailSenderService, LogAwareEmailSenderService>();

Edit: the Decorator pattern is a great way to honour Single Responsibility while enabling the Open/Closed principle: you're able to add new decorators to extend the capabilities of your codebase without updating the existing classes.

However, it comes with the cost of a somewhat more involved setup. The Scrutor library takes care of the decoration so that you don't need to manually code how the services are composed. If you don't have many decorators, or you don't have many levels of decoration, then that advantage might not be useful to you.

The scanning capability is not related to the decorator setup: it's simply another capability that allows one to add classes to the service collection without manually the classes (sort of auto-discovery). You can do this via reflection.

Here's an example of the decorator setup if you also had caching:

builder.Services.AddScoped<IWeatherRepository, WeatherRepository>();
builder.Services.Decorate<IWeatherRepository, DiagnosticsWeatherRepositoryDecorator>();
builder.Services.Decorate<IWeatherRepository, CachedWeatherRepositoryDecorator>();

You could do this manually:

builder.Services.AddScoped<WeatherRepository>();
builder.Services.AddScoped(provider => new DiagnosticsWeatherRepositoryDecorator(provider.GetRequiredService<WeatherRepository>()));
builder.Services.AddScoped<IWeatherRepository>(provider => new CachedWeatherRepositoryDecorator(provider.GetRequiredService<DiagnosticsWeatherRepositoryDecorator>)));

It becomes a bit more involved if the constructors take other parameters. It's completely possible, but really verbose.

I thought I'd also share some advice regarding your 5th question; my experience of dependency injection frameworks is that:

The frameworks are simply a key-value map; if a class needs interface A, then create class B.
Sometimes it's just a key; that happens when you only need to let the framework know that class C exists.
Whenever a class needs to be created, the map of all classes and all interface/class to class mappings are consulted, and whatever the map lists is created.
It seems like you're aware, but these frameworks also manage the lifetime of the objects it creates. For example, a scoped lifetime is linked to the duration of the http request. This means that an IDisposible object created by the framework, will be deposed once the request ends.

In your case:

builder.Services.AddScoped<IEmailSenderService, EmailSenderService>();
builder.Services.AddScoped<IEmailSenderService, LogAwareEmailSenderService>();

The second statement actually overwrites the mapping of the first statement: you can only have one key (in this case IEmailSenderService) in the collection. So when the framework tried to create LogAwareEmailSenderService it saw that LogAwareEmailSenderService needs an IEmailSenderService but the only one it knows about is LogAwareEmailSenderService.

This is why we only list the interface once when we manually tied up the decorated classes. When the Scrutor library is used, it re-maps the types allowing you to list the interface multiple times.