Is there an easier way to test argument validation and field initialization in an immutable object?

Question

My domain consists of lots of simple immutable classes like this:

public class Person
{
    public string FullName { get; }
    public string NameAtBirth { get; }
    public string TaxId { get; }
    public PhoneNumber PhoneNumber { get; }
    public Address Address { get; }

    public Person(
        string fullName,
        string nameAtBirth,
        string taxId,
        PhoneNumber phoneNumber,
        Address address)
    {
        if (fullName == null)
            throw new ArgumentNullException(nameof(fullName));
        if (nameAtBirth == null)
            throw new ArgumentNullException(nameof(nameAtBirth));
        if (taxId == null)
            throw new ArgumentNullException(nameof(taxId));
        if (phoneNumber == null)
            throw new ArgumentNullException(nameof(phoneNumber));
        if (address == null)
            throw new ArgumentNullException(nameof(address));

        FullName = fullName;
        NameAtBirth = nameAtBirth;
        TaxId = taxId;
        PhoneNumber = phoneNumber;
        Address = address;
    }
}

Writing the null checks and property initialization is already getting very tedious but currently I write unit tests for each of these classes to verify that argument validation works correctly and that all properties are initialized. This feels like extremely boring busywork with incommensurate benefit.

The real solution would be for C# to support immutability and non-nullable reference types natively. But what can I do to improve the situation in the meantime? Is it worth writing all these tests? Would it be a good idea to write a code generator for such classes to avoid writing tests for each one of them?

---

Here is what I have now based on the answers.

I could simplify the null checks and property initialization to look like this:

FullName = fullName.ThrowIfNull(nameof(fullName));
NameAtBirth = nameAtBirth.ThrowIfNull(nameof(nameAtBirth));
TaxId = taxId.ThrowIfNull(nameof(taxId));
PhoneNumber = phoneNumber.ThrowIfNull(nameof(phoneNumber));
Address = address.ThrowIfNull(nameof(address));

Using the following implementation by Robert Harvey:

public static class ArgumentValidationExtensions
{
    public static T ThrowIfNull<T>(this T o, string paramName) where T : class
    {
        if (o == null)
            throw new ArgumentNullException(paramName);

        return o;
    }
}

Testing the null checks is easy using the GuardClauseAssertion from AutoFixture.Idioms (thanks for the suggestion, Esben Skov Pedersen):

var fixture = new Fixture().Customize(new AutoMoqCustomization());
var assertion = new GuardClauseAssertion(fixture);
assertion.Verify(typeof(Address).GetConstructors());

This could be compressed even further:

typeof(Address).ShouldNotAcceptNullConstructorArguments();

Using this extension method:

public static void ShouldNotAcceptNullConstructorArguments(this Type type)
{
    var fixture = new Fixture().Customize(new AutoMoqCustomization());
    var assertion = new GuardClauseAssertion(fixture);

    assertion.Verify(type.GetConstructors());
}

Answer 1

I created a t4 template exactly for this kind of cases. To avoid writing lots of boilerplate for Immutable classes.

https://github.com/xaviergonz/T4Immutable T4Immutable is a T4 template for C# .NET apps that generates code for immutable classes.

Specifically talking about non null tests then if you use this:

[PreNotNullCheck, PostNotNullCheck]
public string FirstName { get; }

The constructor will be this:

public Person(string firstName) {
  // pre not null check
  if (firstName == null) throw new ArgumentNullException(nameof(firstName));

  // assignations + PostConstructor() if needed

  // post not null check
  if (this.FirstName == null) throw new NullReferenceException(nameof(this.FirstName));
}

Having said this, if you use JetBrains Annotations for null checking, you can also do this:

[JetBrains.Annotations.NotNull, ConstructorParamNotNull]
public string FirstName { get; }

And the constructor will be this:

public Person([JetBrains.Annotations.NotNull] string firstName) {
  // pre not null check is implied by ConstructorParamNotNull
  if (firstName == null) throw new ArgumentNullException(nameof(firstName));

  FirstName = firstName;
  // + PostConstructor() if needed

  // post not null check implied by JetBrains.Annotations.NotNull on the property
  if (this.FirstName == null) throw new NullReferenceException(nameof(this.FirstName));
}

Also there are a few more features than this one.

Answer 2

You can get a bit of improvement with a simple refactoring that can ease the problem of writing all those fences. First, you need this extension method:

internal static T ThrowIfNull<T>(this T o, string paramName) where T : class
{
    if (o == null)
        throw new ArgumentNullException(paramName);

    return o;
}

You can then write:

public class Person
{
    public string FullName { get; }
    public string NameAtBirth { get; }
    public string TaxId { get; }
    public PhoneNumber PhoneNumber { get; }
    public Address Address { get; }

    public Person(
        string fullName,
        string nameAtBirth,
        string taxId,
        PhoneNumber phoneNumber,
        Address address)
    {
        FullName = fullName.ThrowIfNull(nameof(fullName));
        NameAtBirth = nameAtBirth.ThrowIfNull(nameof(nameAtBirth));
        TaxId = taxId.ThrowIfNull(nameof(taxId));
        PhoneNumber = phoneNumber.ThrowIfNull(nameof(fullName));
        Address = address.ThrowIfNull(nameof(address));
    }
}

Returning the original parameter in the extension method creates a fluent interface, so you can extend this concept with other extension methods if you wish, and chain them all together in your assignment.

Other techniques are more elegant in concept, but progressively more complex in execution, such as decorating the parameter with a [NotNull] attribute, and using Reflection like this.

That said, you may not need all these tests, unless your class is part of a public-facing API.

Answer 3

In the short term, there's not much you can do about the tediousness of writing such tests. However, there is some help coming with throw expressions due to be implemented as part of the next release of C# (v7), likely due in the next few months:

public class Person
{
    public string FullName { get; }
    public string NameAtBirth { get; }
    public string TaxId { get; }
    public PhoneNumber PhoneNumber { get; }
    public Address Address { get; }

    public Person(
        string fullName,
        string nameAtBirth,
        string taxId,
        PhoneNumber phoneNumber,
        Address address)
    {
        FullName = fullName ?? throw new ArgumentNullException(nameof(fullName));
        NameAtBirth = nameAtBirth ?? throw new ArgumentNullException(nameof(nameAtBirth));
        TaxId = taxId ?? throw new ArgumentNullException(nameof(taxId)); ;
        PhoneNumber = phoneNumber ?? throw new ArgumentNullException(nameof(phoneNumber)); ;
        Address = address ?? throw new ArgumentNullException(nameof(address)); ;
    }
}

You can experiment with throw expressions via the Try Roslyn webapp.

Answer 4

I'm surprised no one mentioned NullGuard.Fody yet. It's available via NuGet and will automagically weave those null checks into the IL during compile time.

So your constructor code would simply be

public Person(
    string fullName,
    string nameAtBirth,
    string taxId,
    PhoneNumber phoneNumber,
    Address address)
{
    FullName = fullName;
    NameAtBirth = nameAtBirth;
    TaxId = taxId;
    PhoneNumber = phoneNumber;
    Address = address;
}

and NullGuard will add those null checks for you transforming it into exactly what you wrote.

Note though, that NullGuard is opt-out, that is, it will add those null checks to every method and constructor argument, property getter and setter and even check method return values unless you explicitly allow a null value with the [AllowNull] attribute.

Answer 5

Is it worth writing all these tests?

No, probably not. What is the likelihood that you're going to screw that up? What is the likelihood that some semantics will change out from under you? What is the impact if someone does screw it up?

If you're spending a bunch of time making tests for something that will rarely break, and is a trivial fix if it did... maybe not worth it.

Would it be a good idea to write a code generator for such classes to avoid writing tests for each one of them?

Maybe? That sort of thing could be done easily with reflection. Something to consider is doing code generation for the real code, so you don't have N classes that may have human error. Bug prevention > bug detection.

Answer 6

Is it worth writing all these tests?

No.
Because I am pretty sure you have tested those properties through some other tests of logic where those classes are used.

For example, you can have tests for Factory class which have assertion based on those properties (Instance created with properly assigned Name property for example).

If those classes are exposed to the public API which used by some third part/end user (@EJoshua thanks for noticing), then tests for expected ArgumentNullException can be useful.

While waiting for C#7 you can use extension method

public MyClass(string name)
{
    name.ThrowArgumentNullExceptionIfNull(nameof(name));
}

public static void ThrowArgumentNullExceptionIfNull(this object value, string paramName)
{
    if(value == null)
        throw new ArgumentNullException(paramName);
}

For testing you can use one parameterized test method which use reflection to create null reference for every parameter and assert for expected exception.

Answer 7

You could always write a method like the following:

// Just to illustrate how to call this
private static void SomeMethod(string a, string b, string c, string d)
    {
        ValidateArguments(a, b, c, d);
        // ...
    }

    // This is the one to use as a utility function
    private static void ValidateArguments(params object[] args)
    {
        for (int i = 0; i < args.Length; i++)
        {
            if (args[i] == null)
            {
                StackTrace trace = new StackTrace();
                // Get the method that called us
                MethodBase info = trace.GetFrame(1).GetMethod();

                // Get information on the parameter that is null so we can add its name to the exception
                ParameterInfo param = info.GetParameters()[i];

                // Raise the exception on behalf of the caller
                throw new ArgumentNullException(param.Name);
            }
        }
    }

At a minimum, that'll save you some typing if you have several methods that require this kind of validation. Of course, this solution assumes that none of your method's parameters can be null, but you can modify this to change that if you so desire.

You can also extend this to perform other type-specific validation. For example, if you have a rule that strings can't be purely whitespace or empty, you could add the following condition:

// Note that we already know based on the previous condition that args[i] is not null
else if (args[i].GetType() == typeof(string))
            {
                string argCast = arg as string;

                if (!argCast.Trim().Any())
                {
                    ParameterInfo param = GetParameterInfo(i);

                    throw new ArgumentException(param.Name + " is empty or consists only of whitespace");
                }
            }

The GetParameterInfo method I refer to basically does the reflection (so that I don't have to keep typing the same thing over and over, which would be a violation of the DRY principle):

private static ParameterInfo GetParameterInfo(int index)
    {
        StackTrace trace = new StackTrace();

        // Note that we have to go 2 methods back to get the ValidateArguments method's caller
        MethodBase info = trace.GetFrame(2).GetMethod();

        // Get information on the parameter that is null so we can add its name to the exception
        ParameterInfo param = info.GetParameters()[index];

        return param;
    }

Answer 8

I do not suggest throwing exceptions from the constructor. The problem is you will have a hard time to test this as you have to pass valid parameters even they are irrelevant for your test.

For example: If you want to test if the third parameter throws an exception you have to pass in the first and the second correctly. So your test is not isolated anymore. when the constraints of the first and second parameter change this test case will fail even it tests the third parameter.

I suggest to use the java validation API and externalize the validation process. I suggest to have four responsibilities (classes) involved:

1. A suggestion object with Java validation annotated parameters with a validate method that returns a Set of ConstraintViolations. One advantage is you can pass around this objects without the assertion to be valid. You can delay validation until it is neccessary without having a try to instantiate the domain object. The validation object can be used in different layers as it is a POJO with no layer specific knowledge. It can be part of you public API.

2. A factory for the domain object that is responsible for creating valid objects. You pass in the suggestion object and the factory will call the validation and create the domain object if everything is fine.

3. The domain object itself which should be mostly inaccessible for instantiation for other developers. For testing purposes I suggest to have this class in package scope.

4. A public domain interface to hide the concrete domain object and make misusage hard.

I do not suggest to check for null values. As soon as you get into the domain layer you have get rid of passing in null or returning null as long as you do not have object chains that have ends or search functions for single objects.

One other point: writing as less code as possible is not a metric for code quality. Think about 32k games competitions. That code is the most compact but also the messiest code you can have because it does only care about technical issues and does not care about semantics. But semantics are the points that make things comprehensive.