Search code examples
c#.net-corenullable-reference-types

ILogger<T>? defaulting to null vs ILogger<T> defaulting to NullLogger<T>


Take the following two classes:

public class PersonA
{
    private readonly ILogger<PersonA> _logger;

    public PersonA() : this(new NullLogger<PersonA>())
    {}
    
    public PersonA(ILogger<PersonA> logger)
    {
        _logger = logger;
    }

    public void SomeMethod()
    {
        _logger.LogDebug("SomeMethod invoked");
    }
}

public class PersonB
{
    private readonly ILogger<PersonB>? _logger;

    public PersonB(ILogger<PersonB>? logger = null)
    {
        _logger = logger;
    }

    public void SomeMethod()
    {
        _logger?.LogDebug("SomeMethod invoked");
    }
}

The way that ILogger<T> is passed is very similar in both and both approaches work, but is there any reason why you'd favour ILogger<T>? defaulting to null over two ctors one defaulting to NullLogger<T> and ensuring that _logger is never null or vice-versa?

Are any differences in performance?


Solution

  • As pointed out by one comment, there is so little difference in tearms of performance and object allocation that I would actually avoid to compare these two approaches in tearms of performance.

    I would instead suggest the following considerations:

    • having two constructors is worst in terms of design. That will make creating your object harder, because the object creator has to choose between the two and have to understand the different semantic. It is much better having one and only one way to create an object: that's simpler and there is no ambiguity about the way the object should be created
    • as long as possible I would avoid handling null values. Code using objects that can potentially be null is harder to maintain, because if you accidentally forget the ? operator when you refactor your code you will get a NullReferenceException at runtime. You would need to write and maintain additional unit tests for the null case, in order to be sure you didn't forget a ? somewhere in your code.

    Based on these two points, a better design is the following:

    1. have a class with only one constructor
    2. do not allow the _logger field to be nullable, that is it's type should be ILogger<Person> instead of ILogger<Person>?. In cases when you don't care about logging, use the null object pattern and inject an instance of NullLogger<Person> in the constructor (instead of using a null reference).

    As a final note do not instantiate NullLogger<T>. Use, instead, the NullLogger<T>.Instance static field. By doing so, you will access to a shared cached instance of the class and you will avoid to allocate a new object (it's fine to do so for stateless classes like NullLogger<T>). This is good in tearms of memory usage and garbage collector pressure.

    Final design of the class is the following:

    public class Person
    {
        private readonly ILogger<Person> _logger;
    
        public Person(ILogger<Person> logger)
        {
            // guard against null
            _logger = logger ?? throw new ArgumentNullException(nameof(logger));
        }
    
        public void SomeMethod()
        {
            _logger.LogDebug("SomeMethod invoked");
        }
    }
    
    // if you don't care about logging
    var person = new Person(NullLogger<Person>.Instance);