Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

Tags:

(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.

like image 870
Christian Specht Avatar asked Jan 06 '16 14:01

Christian Specht


1 Answers

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.

like image 82
Mark Seemann Avatar answered Sep 29 '22 15:09

Mark Seemann