Logging as a decorator vs. Dependency Injection - what if I need to log inside the class?

Question

(I originally asked this question in this comment, but Mark Seemann asked me to create a new question instead.)

I'm starting a new app (.NET Core, if that matters), and right now I'm trying to decide how exactly to do logging.

The general consensus seems to be that logging is a cross-cutting concern, so the logger shouldn't be injected directly into the class that is supposed to log.

Often, there's an example like the following class how not to do it:

public class BadExample : IExample
{
    private readonly ILogger logger;

    public BadExample(ILogger logger)
    {
        this.logger = logger;
    }

    public void DoStuff()
    {
        try
        {
            // do the important stuff here
        }
        catch (Exception e)
        {
            this.logger.Error(e.ToString());
        }
    }
}

Instead, the class with the business logic shouldn't know about the logger (SRP) and there should be a separate class which does the logging:

public class BetterExample : IExample
{
    public void DoStuff()
    {
        // do the important stuff here
    }
}

public class LoggingBetterExample : IExample
{
    private readonly IExample betterExample;
    private readonly ILogger logger;

    public LoggingBetterExample(IExample betterExample, ILogger logger)
    {
        this.betterExample = betterExample;
        this.logger = logger;
    }

    public void DoStuff()
    {
        try
        {
            this.betterExample.DoStuff();
        }
        catch (Exception e)
        {
            this.logger.Error(e.ToString());
        }
    }
}

Whenever an IExample is needed, the DI container returns an instance of LoggingBetterExample, which uses BetterExample (which contains the actual business logic) under the hood.

Some sources for this approach:

Blog posts by Mark Seemann:

• Instrumentation with Decorators and Interceptors
• Dependency Injection is Loose Coupling

Blog post and SO answer by Steven:

• Meanwhile... on the command side of my architecture
• Windsor - pulling Transient objects from the container

---

My question:

Obviously, the LoggingBetterExample approach only works as long as the logging can be done outside the actual class.
(like in the example above: catch any exceptions thrown by BetterExample from outside)

My problem is that I'd like to log other things inside the actual class.
Mark Seemann suspected here that if someone needs to do this, maybe the method in question is doing too much.

As I said before, I'm in the planning phase for a new application, so I don't have much code to show, but the use case I'm thinking right now is something like this:

My app will have a config file with some optional values.
The user may decide to omit the optional values, but it's an important decision to do this.
So I'd like to log a warning when some of the optional values are missing, just in case it happened by error.
(omitting the values is perfectly fine though, so I can't just throw an exception and stop)

This means that I will have a class which reads config values and needs to do something like this (pseudocode):

var config = ReadConfigValues("path/to/config.file");

if (config.OptionalValue == null)
{
    logger.Warn("Optional value not set!");
}

No matter if ReadConfigValues is in this class or a different one, I don't think this class would violate the SRP.

When I'm not able to log outside the actual class by using a decorator, is there a better solution than to inject the logger?

I know I could read the config file in the inner class, but check the values (and log the warning) in the decorator. But IMO checking the value is business logic and not infrastructure, so to me it belongs in the same class where the config file is read.

Astro

You could have a logger factory available in a lower layer so that it is available everywhere. The factory could be configured in the Composition Root so that the actual implementation of loggers is not important to client classes. In other words, I suggest you don't inject but rather make a dependency to a factory. – Sillimanite

Answer 1

checking the value is business logic and not intfastructure, so to me it belongs in the same class where the config file is read.

Obviously, I don't know your domain well enough to dispute the truth of that assertion, but that logging is part of the domain model sounds strange to me. Anyway, for the sake of argument, let's assume that this is the case.

What ought not to be the case, though, is that reading a configuration file is domain logic. While reading and manipulating the data from a file could easily be domain logic, reading a file is I/O.

The most common approach to Inversion of Control in application architecture is to employ the Ports & Adapters architecture. The entire point of such an architecture is to decouple the domain model from I/O, and other sources of non-determinism. The poster example is to show how to decouple the domain model from its database access, but file access falls squarely in that category as well.

What this ought to imply in this particular case is that you're going to need some IConfigurationReader interface anyway. This means that you can apply a Decorator:

public class ValidatingConfigurationReader : IConfigurationReader
{
    private readonly IConfigurationReader reader;
    private readonly ILogger logger;

    public ValidatingConfigurationReader(IConfigurationReader reader, ILogger logger)
    {
        this.reader = reader;
        this.logger = logger;
    }

    public MyConfiguration ReadConfigValues(string filePath)
    {
        var config = this.reader.ReadConfigValues(filePath);

        if (config.OptionalValue == null)
        {
            this.logger.Warn("Optional value not set!");
        }

        return config;
    }
}

This ValidatingConfigurationReader class can be implemented in the domain model, even if the underlying, file-reading IConfigurationReader implementation belongs in some I/O layer.

Farrison

Concerning the part you quoted: I'm afraid you misunderstood me - I meant that checking the value is domain logic, not logging. But after re-reading my question and your answer, I agree with you that reading the config file is not domain logic. I still have a question about your answer though: The ValidatingConfigurationReader contains business logic AND gets a logger injected. I thought the point of using a Decorator was to avoid having both in the same class? Or is there no better solution in this case? – Astro

That's a keen observation. It still makes me wonder how the Domain Logic can possibly care about logging. Could it be that there's some other concern in play here that looks like logging, but ought not to be conflated with logging? Could it be, for example, that the concern was better modelled as Domain Events? – Farrison

Regarding Dependency Injection and events, see: blog.ploeh.dk/2013/09/06/dependency-injection-and-events – Farrison

I'm afraid we're still misunderstanding each other. I never meant to say that logging is part of my domain, but the point is: I have to check for missing config values somewhere, and I have to notify my user somewhere. I chose to use logging for notification, because I intended to have a CompositeLogger which outputs on the console and into a log file (it's a console app that I expect to run unattended most of the time, and admins/devs will look into the log file). – Astro

I understood that because logging is a cross-cutting concern, the logger shouldn't simply be injected into the domain class that does the check and one should use a decorator instead. That's why I asked this question - and you answered it with the ValidatingConfigurationReader, which is a decorator, does domain logic and gets a logger injected. That's where my confusion comes from :-) – Astro

Oh, I see. In that case, I'd solve it in a different way. 1. Load the configuration from file. 2. Pass it to a validation function that returns validation results. 3. Log warnings, if any. You can have the validation function implemented in the Domain Logic if you wish, but you don't need to inject anything into it. You can easily model it as a pure function. – Farrison

Answer 2

Don't take SRP so seriously, otherwise you'll end up with functional programming. If you afraid of getting your class cluttered by putting log statements inside it, then you have two options. The first one you already mentioned which is using a Decorator class but you can't access/log the private stuff. The second option is using partial classes and putting the logging statements in a separate class.

Teakettle

You can also use IL Weaving for logging method calls but that also comes with its own limitations. – Teakettle

"otherwise you'll end up with functional programming" That... isn't a bad thing. In fact, I would say it's a good thing. If you read Seemann's blog, you'll notice that he discusses FP often. – Diabolo

Regarding IL weaving, Seemann mentions an alternative in the original post: https://mcmap.net/q/128331/-logging-aspect-oriented-programming-and-dependency-injection-trying-to-make-sense-of-it-all – Diabolo

There is no good thing or bad thing about FP. It all depends on the context. For instance, if you've ever done any game programming, you already know that you should not use a FP approach like LINQ in the game update loop because it creates a lot of memory garbage. Yes, it looks much cleaner but it puts pressure on the GC and eventually you will end up with performance hiccups. – Teakettle

I agree everything gets thrown out the window when it comes to performance tuning. However, I think you should take SRP very seriously, and only consider violating it after weighing the pros and cons... and possibly benchmarking. Devs love to prematurely optimize. – Diabolo