Is constructing objects with null parameters in unit tests OK?

Question

I started writing some unit tests for my current project. I don't really have experience with it though. I first want to completely "get it", so I am currently using neither my IoC framework nor a mocking library.

I was wondering if there is anything wrong with providing null arguments to objects' constructors in unit tests. Let me provide some example code:

public class CarRadio
{...}


public class Motor
{
    public void SetSpeed(float speed){...}
}


public class Car
{
    public Car(CarRadio carRadio, Motor motor){...}
}


public class SpeedLimit
{
    public bool IsViolatedBy(Car car){...}
}

Yet Another Car Code Example(TM), reduced to only the parts important to the question. I now wrote a test something like this:

public class SpeedLimitTest
{
    public void TestSpeedLimit()
    {
        Motor motor = new Motor();
        motor.SetSpeed(10f);
        Car car = new Car(null, motor);

        SpeedLimit speedLimit = new SpeedLimit();
        Assert.IsTrue(speedLimit.IsViolatedBy(car));
    }
}

The test runs fine. SpeedLimit needs a Car with a Motor in order to do its thing. It is not interested in a CarRadio at all, so I provided null for that.

I am wondering if an object providing correct functionality without being fully constructed is a violation of SRP or a code smell. I have this nagging feeling that it does, but speedLimit.IsViolatedBy(motor) doesn't feel right either - a speed limit is violated by a car, not a motor. Maybe I just need a different perspective for unit tests vs. working code, because the whole intention is to test only a part of the whole.

Is constructing objects with null in unit tests a code smell?

Answer 1

In the case of the example above, it is reasonable that a Car can exist without a CarRadio. In which case, I'd say that not only is it acceptable to pass in a null CarRadio sometimes, I'd say that it's obligatory. Your tests need to ensure that the implementation of the Car class is robust, and does not throw null pointer exceptions when no CarRadio is present.

However, let's take a different example - let's consider a SteeringWheel. Let's assume that a Car has to have a SteeringWheel, but the speed test doesn't really care about it. In this case, I wouldn't pass a null SteeringWheel as this is then pushing the Car class into places where it isn't designed to go. In this case, you'd be better off creating some sort of DefaultSteeringWheel which (to continue the metaphor) is locked in a straight line.

Answer 2

Is constructing objects with null in unit tests a code smell?

Yes. Note the C2 definition of code smell: "a CodeSmell is a hint that something might be wrong, not a certainty."

The test runs fine. SpeedLimit needs a Car with a Motor in order to do its thing. It is not interested in a CarRadio at all, so I provided null for that.

If you are trying to demonstrate that speedLimit.IsViolatedBy(car) does not depend on the CarRadio argument passed to the constructor, then it would probably be clearer to the future maintainer to make that constraint explicit by introducing a test double that fails the test if it is invoked.

If, as is more likely, you are just trying to save yourself the work of creating a CarRadio that you know isn't used in this case, you should notice that

• this is a violation of encapsulation (you are letting the fact that CarRadio isn't used leak out into the test)
• you have discovered at least one case where you want to create a Car without specifying a CarRadio

I strongly suspect that the code is trying to tell you that you want a constructor that looks like

public Car(IMotor motor) {...}

and then that constructor can decide what to do with the fact that there is no CarRadio

public Car(IMotor motor) {
    this(null, motor);
}

or

public Car(IMotor motor) {
    this( new ICarRadio() {...} , motor);
}

or

public Car(IMotor motor) {
    this( new DefaultRadio(), motor);
}

etc.

Its about if passing null to something else while testing SpeedLimit. You can see that the test is called "SpeedLimitTest" and the only Assert is checking a method of SpeedLimit.

That's a second interesting smell - in order to test SpeedLimit, you have to build an entire car around a radio, even though the only thing that the SpeedLimit check probably cares about is the speed.

This might be a hint that SpeedLimit.isViolatedBy should be accepting a role interface that car implements, rather than requiring an entire car.

interface IHaveSpeed {
    float speed();
}

class Car implements IHaveSpeed {...}

IHaveSpeed is a really lousy name; hopefully your domain language will have an improvement.

With that interface in place, your test for SpeedLimit could be much simpler

public void TestSpeedLimit()
{
    IHaveSpeed somethingTravelingQuickly = new IHaveSpeed {
        float speed() { return 10f; }
    }

    SpeedLimit speedLimit = new SpeedLimit();
    Assert.IsTrue(speedLimit.IsViolatedBy(somethingTravelingQuickly));
}

Answer 3

Is constructing objects with null in unit tests a code smell?

No

Not at all. To the contrary, if NULL is a value that is available as an argument (some languages may have constructs that disallow passing of a null pointer), you should test it.

Another question is what happens when you pass NULL. But that is exactly what a unit test is about: making sure a defined result is happening when for each possible input. And NULL is a potential input, so you should have a defined result and you should have a test for that.

So if your Car is supposed to be constructable without a radio, you should write a test for that. If it's not and is supposed to throw an exception, you should write a test for that.

Either way, yes you should write a test for NULL. It would be a code smell if you left it out. That would mean you have an untested branch of code that you just assumed would magically be bug-free.

---

Please note that I do agree with the other answers that your code under test could be improved. Nonetheless, if the improved code has parameters that are pointers, passing NULL even for the improved code is a good test. I would want a test to show what happens when I pass NULL as a motor. That's not a code smell, that's the opposite of a code smell. It's testing.

Answer 4

I personally would be hesitant to inject nulls. In fact, whenever I inject dependencies into a constructor, one of the first things I do is raise an ArgumentNullException if those injections are null. That way I can safely use them inside my class, without dozens of null-checks spread around. Here's a very common pattern. Note the use of readonly to ensure those dependencies set in constructor can not be set to null (or anything else) afterwards:

class Car
{
   private readonly ICarRadio _carRadio;
   private readonly IMotor _motor;

   public Car(ICarRadio carRadio, IMotor motor)
   {
      if (carRadio == null)
         throw new ArgumentNullException(nameof(carRadio));

      if (motor == null)
         throw new ArgumentNullException(nameof(motor));

      _carRadio = carRadio;
      _motor = motor;
   }
}

With the above, I could perhaps have a unit test or two, where I inject nulls, just to make sure my null checks work as expected.

Having said that, this becomes a non-issue, once you do two things:

1. Program to interfaces instead of classes.
2. Use a mocking framework (like Moq for C#) to inject mocked dependencies instead of nulls.

So for your example you'd have:

interface ICarRadio
{
   bool Tune(float frequency);
}

interface Motor
{
   bool SetSpeed(float speed);
}

...
var car = new Car(new Moq<ICarRadio>().Object, new Moq<IMotor>().Object);

More details on using Moq can be found here.

Of course, if you want to understand it without mocking first, you can still do it, as long as you are using interfaces. Simply create a dummy CarRadio and Motor as following, and inject those instead:

class DummyCarRadio : ICarRadio
{
   ...
}
class DummyMotor : IMotor
{
   ...
}

...
var car = new Car(new DummyCarRadio(), new DummyMotor())

Answer 5

Let's take a step back to first principles.

A unit test tests some aspect of a single class.

There will however be constructors or methods that take other classes as parameters. How you handle these will vary from case to case but setup should be minimal since they're tangential to the objective. There may be scenarios where passing a NULL parameter is perfectly valid - such as a car having no radio.

I'm assuming here, that we're just testing the racing line to start with (to continue the car theme), but you may also want to pass NULL to other parameters to check that any defensive code and exception handing mechanisms are functioning as required.

So far so good. However, what if NULL isn't a valid value. Well, here you're into the murky world of mocks, stubs, dummies and fakes.

If all this seems a bit much to take on, you're actually already using a dummy by passing NULL in the Car constructor since it a value that potentially won't be used.

What should become clear pretty quickly, is that you're potentially shotgunning your code with a lot of NULL handling. If you replace class parameters with interfaces, then you also pave the way for mocking and DI etc which make these far more straightforward to handle.

Answer 6

It’s not just okay, it’s a well known dependency breaking technique for getting legacy code into a test harness. (See “Working Effectively with Legacy Code” by Michael Feathers.)

Does it smell? Well...

The test doesn’t smell. The test is doing its job. It’s declaring “this object can be null and the code under test still works correctly”.

The smell is in the implementation. By declaring a constructor argument, you’re declaring “this class needs this thing in order to properly function”. Obviously, that’s untrue. Anything untrue is a bit smelly.

Answer 7

Taking a look at your design, I would suggest that a Car is composed of a set of Features. A Feature maybe a CarRadio, or EntertainmentSystem which may consist of things like a CarRadio, DvdPlayer etc.

So, at a minimum, you would have to construct a Car with a set of Features. EntertainmentSystem and CarRadio would be implementers of the interface (Java, C# etc) of the public [I|i]nterface Feature and this would be an instance of the Composite design pattern.

Therefore, you would always construct a Car with Features and a unit test would never instantiate a Car without any Features. Although you might consider mocking a BasicTestCarFeatureSet that has a minimal set of functionality required for your most basic test.

Just some thoughts.

John