CODEWITHSUNDEEP
c#asp.netdependency-injection
James Andrew Smith
James Andrew Smith

Reputation: 1568

Better way of checking null in dependency injection

When using dependency injection through a constructor I always need to check for nulls before passing the instance to an internal property. e.g.

public UserManager(User user, IStateManager stateManager)
{
    if(user == null) throw new ArgumentNullException;
    if(statemanager == null) throw new ArgumentNullException("stateManager");

    _user = user;
    _stateManager = statemanager;
}

repeating this pattern on every controller / class seems repetitive. Is there a better way to handle this? btw different controllers will have different constructor initialisers. I am using Simple Injector for my DI.

Upvotes: 7

Views: 7663

Answers (4)

KUTlime
KUTlime

Reputation: 8117

If you really insist on checks of injected dependencies, in .NET 6 here is a new method in .NET API ArgumentNullException.ThrowIfNull(someParameter).

My recommendation would be either configure your DI properly or use PostSharp.

Upvotes: 0

Steven
Steven

Reputation: 172816

It's repetitive code, but it will hardly ever be a problem, because can this cause sweeping changed through your code base? Will you ever need to change many of those checks? Hardly. Take a look at this blog post that goes in more details into this.

To be honest, when it comes to my injection constructors, I almost never add those null checks anymore, because I know my DI container will not inject null references into my constructors when auto-wiring those types. This saves me from writing all these null checks at all.

Some people might argue that I now write my code with my DI container in mind, but I would argue against that. I'm just writing the minimal amount of code required that solves my problems. Adding those null checks doesn't help me in my case.

But do note that in case I'm writing code for a reusable library, I absolutely do write those null checks, because I have no idea who is calling that code. For constructors that are not used as injection constructors (messages, entities, value types, DTOs) I actually DO add those checks. But here are some ideas how to make this a little bit nicer:

You can add a nice helper method like this:

public UserManager(User user, IStateManager stateManager)
{
    Requires.IsNotNull(user, "user");
    Requires.IsNotNull(statemanager, "statemanager");

    _user = user;
    _stateManager = statemanager;
}

This however, doesn't really help with reducing duplicate code, although it does reduce the actual size of machine code that is generated (but this hardly ever an issue). So, you could make this method return a value like this:

public UserManager(User user, IStateManager stateManager)
{
    _user = Requires.IsNotNull(user, "user");
    _stateManager = Requires.IsNotNull(statemanager, "statemanager");
}

Or... using C# 6.0:

public UserManager(User user, IStateManager stateManager)
{
    _user = Requires.IsNotNull(user, nameof(user));
    _stateManager = Requires.IsNotNull(statemanager, nameof(statemanager));
}

You can implement this method as follows:

public static class Requires {
    public static T IsNotNull<T>(T instance, string paramName) where T : class {
        // Use ReferenceEquals in case T overrides equals.
        if (object.ReferenceEquals(null, instance)) {
            // Call a method that throws instead of throwing directly. This allows
            // this IsNotNull method to be inlined.
            ThrowArgumentNullException(paramName);
        }

        return instance;
    }

    private static void ThrowArgumentNullException(paramName) {
        throw new ArgumentNullException(paramName);
    }
}

With C# 8 non-nullable reference types, reference types can be made non-nullable by default:

public UserManager(User user, IStateManager stateManager)
{
    _user = user;
    _stateManager = statemanager;
}

Note that this is only causes a compile-time enforcement, not a runtime enforcement. So no exceptions are thrown.

This might change with C# 9. There is a proposal for added the runtime checks using the ! symbol:

public UserManager(User user!, IStateManager stateManager!)
{
    _user = user;
    _stateManager = statemanager;
}

Upvotes: 20

Borys Generalov
Borys Generalov

Reputation: 2345

In fact such checks produce 0 business value, but create extra noisiness in your code. A fellow developer will be confused and will have to spend extra effort trying to understand why you have all these checks. Normally IoC container will throw an error when the dependency could not be resolved. Even if your container did not throw, you'll have null reference exception later during execution. Better add Debug.Assert statement, which is sort of indication that you are not sure what exactly is happening here, but you've got null sometimes

Upvotes: 3

Scope Creep
Scope Creep

Reputation: 881

I use a static method, .ThrowIfNull, which will throw an ArgumentNullException with the correct argument name, if null.

public MyClass 
{
  public MyClass(DependencyType1 firstDependency, DependencyType2 secondDependency)
  {
     Arguments.ThrowIfNull(firstDependency, secondDependency);

     _firstDependency = firstDependency;
     _secondDependency = secondDependency;
  }
}

public static class Arguments
    {
        public static void ThrowIfNull(params object[] args)
        {
            for (var i = 0; i < args.Length; i++)
            {
                if (args[i] != null
                    && args[i].GetType() == typeof(ArgumentAction)
                    && (ArgumentAction)args[i] == ArgumentAction.Skip) continue;
                if (args[i] == null) throw GetArgumentNullException(i);
            }
        }

        private static ArgumentNullException GetArgumentNullException(int argIndex)
        {
            var frame = new StackFrame(2, false);
            var method = frame.GetMethod();
            var args = method.GetParameters();
            var name = args[argIndex].Name;
            return new ArgumentNullException(name);
        }

        public enum ArgumentAction
        {
            Undefined = 0,
            Skip
        }
    }

Upvotes: 4

Related Questions