Mark Needham

Thoughts on Software Development

Coding: Assertions in constructors

with 8 comments

While browsing through the ASP.NET MVC source I noticed that they use an interesting pattern on the constructors to ensure that an exception will be thrown if an object is not instantiated correctly.

public ControllerContext(HttpContextBase httpContext, RouteData routeData, ControllerBase controller)
    : base(httpContext, routeData) {
    if (controller == null) {
        throw new ArgumentNullException("controller");
    }
    Controller = controller;
}

If you pass in a null Controller you shall go no further!

We have discussed the merits of putting assertions in constructors several times in our Domain Driven Design book club but without ever really coming to a conclusion with respect to whether or not it is a good pattern.

Spring is an example of a framework which does internal assertions on the values being passed to its objects and will throw assertions appropriately. There is even a custom Assert class to do so.

The benefit of this approach is that our code will fail early if there is a problem. The disadvantage is that we may end up putting Asserts all over our code to cater for every place where the input could be invalid. If we were to use the Spring Assert class we would then have a coupling to that in our production code.

The approach I am more familiar with is that we would validate any input from the UI or from systems that we don’t have control over, but apart from these boundary cases we would assume that our classes are good citizens and will call each other with valid data.

My understanding of the implicit contract of a constructor is that we should never be passing null values into them. If we are contemplating doing so then we are probably doing something wrong and either need to reconsider or create a new constructor and then default the value for the extra parameters in other constructors.

In a way I suppose this is a limitation of the Java/C# type system that you can’t prevent null values being passed into a constructor or that you can even have a null value in the first place.

Be Sociable, Share!

Written by Mark Needham

February 14th, 2009 at 1:32 am

Posted in Coding

Tagged with

  • My opinion is that the null ref argument checks are just unwieldy code noise. I think the MVC team is doing that because of a coding standard more than a real need. On any project that I do, all dependencies are generally filled by an IoC tool, so the threat of an NRE is effectively nil. In that case, the null checks do more damage to readability than good in terms of error prevention/detection.

    I think the MVC code is a poster child for the negative impact of defensive coding. I find that code to be very hard to follow because of the overwhelming defensive code checks.

    Besides, it’s nice to be able to throw in nulls in unit tests when a dependency isn’t really in play.

  • Pingback: Arjan`s World » LINKBLOG for February 14, 2009()

  • Pingback: Reflective Perspective - Chris Alcock » The Morning Brew #287()

  • “we would assume that our classes are good citizens”

    Hhhmmm … does the server-rule about client data submission apply here?

    We always tell our server-side developers not to trust the client input, to always validate it: The client is always either stupid or malicious.

    Without validating the inputs, we’re assuming that both current and future callers will be “good citizens” — but how many developers are on the team? How many maintenance developers will fix defects and add features to this codebase over the next five or ten years?

    Will they all develop code that meets the “good citizen”-test and what the potential consequences of them not doing so? Just a null-pointer exception or something more serious?

    I don’t think the readability argument holds water, because the potential consequences if there’s a situation where null results in an unexpected result, rather than an exception, could be high.

  • “Hhhmmm … does the server-rule about client data submission apply here?”

    Good point, I guess the boundary of client/server would be an area where I wouldn’t trust the data coming in.

    Re: Developers on the team etc – that was exactly the discussion being used in favour of having the assertions when we talked about it in the book club.

    I’m not sure what the answer is – I’ve not worked on a project where we put these types of assertions throughout the codebase, it would be interesting to see how it worked out.

  • Jacob Stanley

    Checking constructor arguments can be done in a fairly non intrusive way with extension methods.

    I use an extension method in the form – T NotNull(this T, string) – which allows for the following syntax:

    public ControllerContext(HttpContextBase httpContext, RouteData routeData, ControllerBase controller)
    : base(httpContext, routeData)
    {
    Controller = controller.NotNull(“controller”);
    }

    Tagged with the right attributes (from JetBrains.Annotations), ReSharper will even give you intellisense on the string which represents the parameter name.

    [DebuggerHidden]
    [AssertionMethod]
    public static T NotNull(
    [AssertionCondition(AssertionConditionType.IS_NOT_NULL)] this T argument,
    [InvokerParameterName] string argumentName) where T : class
    {
    if (argument == null)
    {
    throw new ArgumentNullException(argumentName);
    }

    return argument;
    }

  • Jacob Stanley

    Looks like the <generics> and code formatting were stripped out of the example, but you get the idea 🙂

  • Pingback: Coding: Checking invariants in a factory method at Mark Needham()