Is it a good practice to have logger as a singleto

Question

I had a habit to pass logger to constructor, like:

public class OrderService : IOrderService {
     public OrderService(ILogger logger) {
     }
}

But that is quite annoying, so I've used it a property this for some time:

private ILogger logger = NullLogger.Instance;
public ILogger Logger
{
    get { return logger; }
    set { logger = value; }
}

This is getting annoying too - it is not dry, I need to repeat this in every class. I could use base class, but then again - I'm using Form class, so would need FormBase, etc. So I think, what would be downside of having singleton with ILogger exposed, so veryone would know where to get logger:

    Infrastructure.Logger.Info("blabla");

UPDATE: As Merlyn correctly noticed, I've should mention, that in first and second examples I am using DI.

Answer 1

A singleton is a good idea. An even better idea is to use the Registry pattern, which gives a bit more control over instantiation. In my opinion the singleton pattern is too close to global variables. With a registry handling object creation or reuse there is room for future changes to instantiation rules.

The Registry itself can be a static class to give simple syntax to access the log:

Registry.Logger.Info("blabla");

Answer 2

Another solution I personally find the easiest is to use a static Logger class. You can call it from any class method without having to change the class, e.g. add property injection etc. Its pretty simple and easy to use.

Logger::initialize ("filename.log", Logger::LEVEL_ERROR); // only need to be called once in your application

Logger::log ("my error message", Logger::LEVEL_ERROR); // to be used in every method where needed