Should we enforce constructor args?!

 Context

Say we have a Car object:

public class Car
{
    protected Engine engine;
    protected Door[] doors;
    protected Wheel[] wheels;

    public Car(Engine engine, Door[] doors, Wheel[] wheels)
    {
        Check.PreCondition(engine != null, "engine must not be null");
        Check.PreCondition(doors != null, "doors must not be null");
        Check.PreCondition(wheels != null, "wheels must not be null");

        this.engine = engine;
        this.doors = doors;
        this.wheels = wheels;
    }

    public void EnableCentralLocking()
    {
        doors.ForEach(door => door.Lock());
    }

    public void Start()
    {
        engine.TurnOver();
    }
}

The code above is used to demonstrate a context wereby we have an object that takes a number of other collabarators which are then delegated to when certain behaviour is required this is a common event in OO, You can see I use a custom assertion class to make sure the collabarators are not null.

One thing we pick up on is that different behaviours that the Car object is instructed to perform may effect only one collabarator or may effect them all for instance EnableCentralLocking effects the doors collabarating object but has no effect on the wheels or engine.

Unit testing the above

Ok so now we want to unit test the Car object:

[TestFixture]
public class when_central_locking_enabled
{
    [Test]
    public void should_tell_all_doors_to_lock()
    {
        var engine = new Engine();
        var wheels = new Wheel[4];
        var doors = createStubDoors();

        var sut = new Car(engine, doors, wheels);
        sut.EnableCentralLocking();
        doors.All(door => door.Locked);
    }
}

Here I use a state based way of testing the doors have all been locked. One thing is apparent in order to create the Car due to the assertion checks at the beginning I have to create all the collabarators now if I was testing a behaviour that required all of them this is a must however to test the central locking the doors is the only collabarator I need, the creation of the others is just a wasted overhead.

Now this is how I currently create my objects that require collabarators and I was ok with this however recently I was told by a friend to check out a blog by Misko Hevery I was astonished by how well written and the information surrounding good OO and unit testing was, anyhow I came across this post which made me think about whether we should be asserting the collabarators in the constructor or not.

If we remove the assertions our test becomes:

[TestFixture]
public class when_central_locking_enabled
{
    [Test]
    public void should_tell_all_doors_to_lock()
    {
        var doors = createStubDoors();
        var sut = new Car(null, doors, null);
        sut.EnableCentralLocking();
        doors.All(door => door.Locked);
    }
}

hmmm...
We removed the overhead of creating the collabarators not needed for the test however our code is open to NullReferenceExceptions being thrown if were passed a null collabarator, hmmm… a real head scratcher! 

What are the options?

  1. Make the responsibility of making sure objects are created be assigned to a factory object or on an IoC container
  2. Initialize the fields with NullObject values, and use a property to access the field so checks can be made (this would not be enforcable)
  3. Keep the assertions but allow your assertion library to be context aware, at runtime in your unit tests allow it to be in a fall through mode
  4. Keep the assertions and accept the overhead, use Test Data Builder pattern / Object Mother to cut down on repeated code
  5. Move the assertions into the methods, this could cause a lot of DRY violation and extra overhead.

As with most of these things it’s a balance between acheiving 100% defence and overhead required in our unit tests, after looking at the options I could come up with I’m inclined to favour 1 or 3.

What would you use? Do you have any different options about how to balance the 2 out?

Advertisements

2 thoughts on “Should we enforce constructor args?!

  1. I think the clean solution is to put the responsibility of creating a correct object with all dependencies satisfied in the factory or container. I use php so my constructor will be:
    public __construct(Engine $engine = null, array $doors = null, array $wheels = null)
    The problem here is that the factory could use new operator forgetting to pass all parameters (as to allow null parameters with the class type enforcer I have to add them a default value). Testing the factory is something I have not achieved yet.

  2. Thanks for the comment, I tend to favour IoC container creation as well they also tend to blow up at app startup if a dependency is not met which is preferable to a null reference exception halfway through an operation.

Comments are closed.