Suggest design: almost every object in app has log

Question

I'm writing an application. I use NLog for logging. In this application almost every object can write to log. I define protected member for that:

protected Logger logger;

protected virtual Logger Logger
{
     get { return logger ?? (logger = LogManager.GetLogger(this.GetType().ToString())); }
}

In that case I need to copy/paste this code for every base class in application. Or I see other option: define app-specific root object with logger in it and subclass it. But semantically this sounds wrong because it is not true "is-a" situation for me.

are there any better options?

Answer 1

I would recommend you use a static logger so that you get a per-class logger. This stops the overhead of creating a logger for each instance (the logger is thread safe):

class MyClass
{
    static readonly Logger logger = LogManager.GetCurrentClassLogger();
}

GetCurrentClassLogger will save you from having to name the logger explicitly, but the drawback is additional overhead as it has to figure the name of the logger out for you from the stack trace at runtime.

This is likely not a big deal in most cases, but otherwise, it's a bit faster to do this:

class MyClass
{
    static readonly Logger logger = LogManager.GetLogger("MyClass");
}

I would stick with GetCurrentClassLogger until / unless the slight overhead turns out to be an issue in your solution.

The impression I'm getting is that you're trying to cut down on typing, and/or you're addressing the (good) natural aversion to copying and pasting duplicate code.

However, this pattern of having a logger per class with a static initializer is well accepted and works best in most cases. You might be best served sticking with that, and perhaps setting up a code snippet to save you some typing.

If you're still not happy with that, Dependency Injection (either through constructor or property injection) or even Aspect Oriented logging management could be other things for you to investigate.

Answer 2

Sometimes I really wish C# would support multiple inheritance or mixins....

You could write an extension method:

public static Logger Logger(this object obj) {
    return LogManager.GetLogger(obj.GetType());
}

The drawback is that it would be a bit slower as the created instance is not cached (except internally in NLog, which is an implementation detail), but you could do it yourself:

public static Logger Logger(this object obj) {
    Logger logger;
    Type type = obj.GetType();
    // s_loggers is static Dictionary<Type, Logger>
    if (!s_loggers.TryGetValue(type, out logger)) { // not in cache
        logger = LogManager.GetLogger(type);
        s_loggers[type] = logger;  // cache it
    }
    return logger;
}

Than you can call it like this:

this.Logger.Log(...)

The obvious drawback is that any object could write to logger of any other object.

With regard to comment about memory leak (now deleted):

The first implementation solves that. However, it is not a leak more than any static object. It would be a leak if you could not access these objects. As an alternative you could cache WeakReference to logger instead of logger itself, but I see no point, as I believe that there already is some caching in NLog itself. Otherwise, NLog would always have to create a new instance of logger for each type.