Is logging next to an implementation a SRP violation?

Question

When thinking of agile software development and all the principles (SRP, OCP, ...) I ask myself how to treat logging.

Is logging next to an implementation a SRP violation?

I would say yes because the implementation should be also able to run without logging. So how can I implement logging in a better way? I've checked some patterns and came to a conclusion that the best way not to violate the principles in a user-defined way, but to use any pattern which is known to violate a principle is to use a decorator pattern.

Let's say we have a bunch of components completely without SRP violation and then we want to add logging.

• component A
• component B uses A

We want logging for A, so we create another component D decorated with A both implementing an interface I.

• interface I
• component L (logging component of the system)
• component A implements I
• component D implements I, decorates/uses A, uses L for logging
• component B uses an I

Advantages: - I can use A without logging - testing A means I don't need any logging mocks - tests are simpler

Disadvantage: - more components and more tests

I know this seem to be another open discussion question, but I actually want to know if someone uses better logging strategies than a decorator or SRP violation. What about static singleton logger which are as default NullLogger and if syslog-logging is wanted, one change the implementation object at runtime?

Answer 1

I would say you're taking SRP far too seriously. If your code is tidy enough that logging is the only "violation" of SRP then you are doing better than 99% of all other programmers, and you should pat yourself on the back.

The point of SRP is to avoid horrific spaghetti code where code that does different things is all mixed up together. Mixing logging with functional code doesn't ring any alarm bells for me.

Answer 2

No, it is not a violation of SRP.

The messages you send to the log should change for the same reasons as the surrounding code.

What IS a violation of SRP is using a specific library for logging directly in the code. If you decide to change the way of logging, SRP states that it should not impact your business code.

Some kind of abstract Logger should be accessible to your implementation code, and the only thing your implementation should say is "Send this message to the log", with no concerns wrt how it's done. Deciding about the exact way of logging (even timestamping) is not your implementation's responsibility.

Your implementation then should also not know whether the logger it is sending messages to is a NullLogger.

That said.

I would not brush logging away as a cross-cutting concern too fast. Emitting logs to trace specific events occurring in your implementation code belongs to the implementation code.

What is a cross-cutting concern, OTOH, is execution tracing: logging enters and exits in each and every method. AOP is best placed to do this.

Answer 3

As logging is often considered a cross-cutting concern I'd suggest using AOP for separating logging from implementation.

Depending on the language you'd use an interceptor or some AOP framework (e.g. AspectJ in Java) to perform this.

The question is if this is actually worth the hassle. Note that this separation will increase the complexity of your project while providing very little benefit.

Answer 4

This sounds fine. You're describing a fairly standard logging decorator. You have:

component L (logging component of the system)

This has one responsibility: logging information that is passed to it.

component A implements I

This has one responsibility: providing an implementation of interface I (assuming I is properly SRP-compliant, that is).

This is the crucial part:

component D implements I, decorates/uses A, uses L for logging

When stated that way, it sounds complex, but look at it this way: Component D does one thing: bringing A and L together.

• Component D does not log; it delegates that to L
• Component D does not implement I itself; it delegates that to A

The only responsibility that component D has is to make sure that L is notified when A is used. The implementations of A and L are both elsewhere. This is completely SRP-compliant, as well as being a neat example of OCP and a pretty commonplace use of decorators.

An important caveat: when D uses your logging component L, it should do so in a way that lets you change how you're logging. The simplest way to do this is to have an interface IL that is implemented by L. Then:

• Component D uses an IL to log; an instance of L is provided
• Component D uses an I to provide functionality; an instance of A is provided
• Component B uses an I; an instance of D is provided

That way, nothing depends directly on anything else, making it easy to swap them out. This makes it easy to adapt to change, and easy to mock parts of the system so you can unit test.

Answer 5

Of course it's a violation of SRP as you have a cross cutting concern. You can however create a class that is responsible for composing the logging with execution of any action.

example:

class Logger {
   ActuallLogger logger;
   public Action ComposeLog(string msg, Action action) {
      return () => {
          logger.debug(msg);
          action();
      };
   }
}

Answer 6

Yes it is a violation of SRP as logging is a cross cutting concern.

The correct way is to delegate logging to a logger class (Interception) which sole purpose is to log - abiding by the SRP.

See this link for a good example: https://msdn.microsoft.com/en-us/library/dn178467%28v=pandp.30%29.aspx

Here is a short example:

public interface ITenantStore
{
    Tenant GetTenant(string tenant);
    void SaveTenant(Tenant tenant);
}

public class TenantStore : ITenantStore
{
    public Tenant GetTenant(string tenant)
    {....}

    public void SaveTenant(Tenant tenant)
    {....}
} 

public class TenantStoreLogger : ITenantStore
{
    private readonly ILogger _logger; //dep inj
    private readonly ITenantStore _tenantStore;

    public TenantStoreLogger(ITenantStore tenantStore)
    {
        _tenantStore = tenantStore;
    }

    public Tenant GetTenant(string tenant)
    {
        _logger.Log("reading tenant " + tenant.id);
        return _tenantStore.GetTenant(tenant);
    }

    public void SaveTenant(Tenant tenant)
    {
        _tenantStore.SaveTenant(tenant);
        _logger.Log("saving tenant " + tenant.id);
    }
}

Benefits include

• You can test this without logging - true unit testing
• you can easily toggle logging on / off - even at runtime
• you can substitute logging for other forms of logging, without ever having to change the TenantStore file.