We should centralise logging

[MDoerner]

Currently, we sprinkle LogManager.GetCurrentClassLogger() all over the code base and, thus, spread a direct dependency to NLog all over the solution. As was discussed in chat several times, we should probably take a nore structurised approach to this.

One concern in the past was that when deriving from a type GetCurrentClassLogger in the base gets the logger with the message setup for the base class. However, this can be circumvented using LogManager.GetLogger(GetType()FullName).

Using this, I would like to suggest to introduce two abstract base classes for logging providing a protected logger, one generic and one not. The non-generic one will be used for classes at the top of an inheritance hierarchy and the generic one for derived class. The latter will derive from its generic parameter in order to circumvent the need for (illegal) multiple-inheritance. (I hope this is actually legal in C#.)

[Vogel612]

Shouldn't we solve this problem by using DI? It seems like a much cleaner solution to just configure static property injection for a Logger interface. Decoupling that interface from NLog seems too much of a hassle TBH.

Since Rubberduck is not a library, we don't really need to be logger-agnostic. I also don't see us changing logger implementations anywhere down the line...

[MDoerner]

Personally, I have two problems with using DI to get the loggers, in particular property injection.

As far as I understand, to property inject the logger, we need a public or at least internal settable property on the classes supporting logging. Consequently, the logging would be part of all kinds of public interfaces. I thhink that should be avoided since the logger is an implementation detail.

My second concern is of a moer technical nature: is there a way to DI the correct logger for the class it gets injected into?

[retailcoder]

@MDoerner loggers can be named anything - if CW can setup the logger name to match the name of the class it's injected into, then it can work. Or the logger name can be hard-coded, but then we would lose the ability to configure per-namespace logger targets (which we're not using afaik)