7

I have an old guard class - it constisted or static methods, typical of a utilty class.

However, recently I've started to use NLog - so my guards can now log as well as throw. The thing is with NLog that each calling class (where the guard resides) creates its own logger, so instead of a method like this:

public static void NotNull<T>(T obj, string param)
{
    if (obj.Equals(null))
        throw new ArgumentNullException(param);
}

I have a method with a signature like this:

public static void NotNull<T>(T obj, string param, Logger logger, LogLevel logLevel)
{
}

Now all my methods contain the two same parameters relating to the logger, so I've almost decided that dependency injection would be a better approach, passing the logger into the constructor, and obj into the method.

The question I have is based on my inexperience - my new class won't be static, but should I leave the methods inside as static?

Markus Safar
  • 6,324
  • 5
  • 28
  • 44
John Ohara
  • 2,821
  • 3
  • 28
  • 54
  • You do not have to create a new instance of logger per class instance. Have a look at [this answer](http://stackoverflow.com/a/9071875/728795) for example, it is ok to have a static logger field, or some log provider – Andrei Nov 26 '15 at 13:25
  • 2
    Your `NotNull` method throws a `NullReferenceException` if `obj` is null, not an `ArgumentNullException`. – Lee Nov 26 '15 at 13:26
  • @Andrei If you don't use a logger per class, the logs aren't as detailed with regard to the namespace. – John Ohara Nov 26 '15 at 13:27
  • @JohnOhara, per class != per class instance (that is per object) – Andrei Nov 26 '15 at 13:29
  • @Andrei - i get you, but I'd still need to pass it into my static method wouldnt I? – John Ohara Nov 26 '15 at 13:32
  • @JohnOhara, nope. If you have your logger as a static field, it will be available across all your static methods within the class – Andrei Nov 26 '15 at 13:33
  • @Andrei - sorry I'm confused. If I have 2 classes, class A and class B then they will each have a logger. When I call my static method, I'll still need to pass the appropriate logger won't I? If not please can you explain? – John Ohara Nov 26 '15 at 13:36
  • @JohnOhara, posted an answer with code sample, hope it makes it clear – Andrei Nov 26 '15 at 13:40

1 Answers1

6

It does not seem like you need to pass in logger at all. It is fine and not against the common practice to have a static logger field (look at this answer for details), so that it is shared across all instances of the class. Consider:

public static class Utils
{
    private static readonly ILog Log = LogManager.GetLogger(typeof(Utils));

    public static void NotNull<T>(T obj, string param)
    {
        Log.Debug("Huston, we got a null.");
        if (obj.Equals(null))
            throw new ArgumentNullException(param);
    }
}
Community
  • 1
  • 1
Andrei
  • 55,890
  • 9
  • 87
  • 108
  • I get what you are referring to now Andrei - but in this scenario, as far as I'm aware, with just one logger, all the errors get logged to the log in the log file with the same namespace, as MyNameSpace.Utils | MyObj is null – John Ohara Nov 26 '15 at 13:48
  • 1
    Just to add to the above - it's an issue because the null didn't orginate in utils, it was either class A or B – John Ohara Nov 26 '15 at 13:49
  • All right, so you want logger to log an entry under namespace of the class `T`? – Andrei Nov 26 '15 at 14:00
  • Yep, you got it Andrei - param MyVar in class My Class rather than MyVar in Utils – John Ohara Nov 26 '15 at 14:02
  • Alright. First I have to say I doubt it is a sensible approach, but you know your use case obviously better. I would just add a type `T` in text for to the log message in the code I wrote above. However you could also consider creating a logger instance every time the method is called, making sure it is of correct type. Or yes, you could use DI and register factory for loggers, and then methods cannot be static either, as you cannot access non-static fields from static methods – Andrei Nov 26 '15 at 14:08
  • Thanks for the answers Andrei. – John Ohara Nov 26 '15 at 14:22