To Assert or Not To Assert

To Assert or Not To Assert

by Miško Hevery

Some of the strongest objections I get from people is on my stance on what I call "defensive programming". You know all those asserts you sprinkle your code with. I have a special hate relationship against null checking. But let me explain.

At first, people wrote code, and spend a lot of time debugging. Than someone came up with the idea of asserting that some set of things should never happen. Now there are two kinds of assertions, the ones where you assert that an object will never get into on inconsistent state and the ones where you assert that objects never gets passed a incorrect value. The most common of which is the null check.

Than some time later people started doing automated unit-testing, and a weird thing happened, those assertions are actually in the way of good unit testing, especially the null check on the arguments. Let me demonstrate with on example.

class House {
  Door door;
  Window window;
  Roof roof;
  Kitchen kitchen;
  LivingRoom livingRoom;
  BedRoom bedRoom;

  House(Door door, Window window,
            Roof roof, Kitchen kitchen,
            LivingRoom livingRoom,
            BedRoom bedRoom){
    this.door = Assert.notNull(door);
    this.window = Assert.notNull(window);
    this.roof = Assert.notNull(roof);
    this.kitchen = Assert.notNull(kitchen);
    this.livingRoom = Assert.notNull(livingRoom);
    this.bedRoom = Assert.notNull(bedRoom);
  }

  void secure() {
    door.lock();
    window.close();
  }
}

Now let's say that i want to test the secure() method. The secure method needs door and window. Therefore my ideal would look like this.

testSecureHouse() {
  Door door = new Door();
  Window window = new Window();
  House house = new House(door, window,
             null, null, null, null);

  house.secure();

  assertTrue(door.isLocked());
  assertTrue(window.isClosed());
}

Since the secure() method only needs to operate on door, and window, those are the only objects which I should have to create. For the rest of them I should be able to pass in null. null is a great way to tell the reader, "these are not the objects you are looking for". Compare the readability with this:

testSecureHouse() {
  Door door = new Door();
  Window window = new Window();
  House house = new House(door, window,
    new Roof(),
    new Kitchen(),
    new LivingRoom(),
    new BedRoom());

  house.secure();

  assertTrue(door.isLocked());
  assertTrue(window.isClosed());
}

If the test fails here you are now sure where to look for the problem since so many objects are involved. It is not clear from the test that that many of the collaborators are not needed.

However this test assumes that all of the collaborators have no argument constructors, which is most likely not the case. So if the Kitchen class needs dependencies in its constructor, we can only assume that the same person who put the asserts in the House also placed them in the Kitchen, LivingRoom, and BedRoom constructor as well. This means that we have to create instances of those to pass the null check, so our real test will look like this:

testSecureHouse() {
  Door door = new Door();
  Window window = new Window();
  House house = new House(door, window,
    new Roof(),
    new Kitchen(new Sink(new Pipes()),
           new Refrigerator()),
    new LivingRoom(new Table(), new TV(), new Sofa()),
    new BedRoom(new Bed(), new Closet()));

  house.secure();

  assertTrue(door.isLocked());
  assertTrue(window.isClosed());
}

Your asserts are forcing you to create so many objects which have nothing to do with the test and only confuse the reader and make the tests hard to write. Now I know that a house with a null roof, livingRoom, kitchen and bedRoom is an inconsistent object which would be an error in production, but I can write another test of my HouseFactory class which will assert that it will never happen.

Now there is a difference if the API is meant for my internal consumption or is part of an external API. For external API I will often times write tests to assert that appropriate error conditions are handled, but for the internal APIs my tests are sufficient.

I am not against asserts, I often use them in my code as well, but most of my asserts check the internal state of an object not wether or not I am passing in a null value. Checking for nulls usually goes against testability, and given a choice between well tested code and untested code with asserts, there is no debate for me which one I chose.