As an Agile Coach, Miško is responsible for teaching his co-workers to maintain the highest level of automated testing culture, allowing frequent releases of applications with high quality. He is very involved in Open Source community and an author of several open source projects. Recently his interest in Test Driven Developement turned into http://TestabilityExplorer.org with which he hopes will change the testing culture of the open source community. Misko is a DZone MVB and is not an employee of DZone and has posted 38 posts at DZone. You can read more from them at their website. View Full User Profile

To Assert or Not to Assert

02.10.2009
| 5701 views |
  • submit to reddit

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.

From http://misko.hevery.com/

Published at DZone with permission of Misko Hevery, author and DZone MVB.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Mark Thornton replied on Tue, 2009/02/10 - 3:58am

Perhaps your problem is that the House has too many properties being passed in the constructor?

 

Note also that in places where single storey houses are common, the roof is a frequent route for illicit access.

Jeroen Wenting replied on Tue, 2009/02/10 - 4:38am

Correct. And why didn't you consider the special case of a windowless house (say an Eskimo's igloo or a Mongolian yurt? Why can't I define a house with more than one bedroom? More than one livingroom? Where can I put my study, my library? The fact that you have trouble testing your House class is not caused by your parameter checking, but by the rigidity of that House class. It's an indication of poor class design.

Franz Wong replied on Tue, 2009/02/10 - 4:51am

If you cannot change the signature of the constructor, why don't you write a function to create House instance? I think that can help a little bit.

House createHouse(Door door, ....) {

    if (door == null) {

         door = new Door(....);

    }

    ...

    return new House(door, ...);

}

Michael Schnell replied on Tue, 2009/02/10 - 7:40am

What you're complaining about has nothing to do with "defensive programming". It's simply the problem everyone doing "Test Driven Design" (TDD) will face: You're forced to test objects that need other objects as arguments. A common solution is to create some factories in your test code that do the creation of "dummy objects" for you.

Rafael Naufal replied on Tue, 2009/02/10 - 9:03am in response to: Michael Schnell

Sometimes ago I blogged about ways of indicating the absence of an object:

http://rnaufal.livejournal.com/10017.html

 One interesting comment I received about using Assert to validate input: 

 "There's a document in the JDK docs titled Programming With Assertions that says:

Do not use assertions for argument checking in public methods"

Mark Thornton replied on Tue, 2009/02/10 - 9:06am in response to: Rafael Naufal

You supposed to throw an exception instead of using an assert. However this doesn't change the problem with respect to testing --- you have to provide a valid value.

Kamil Hark replied on Tue, 2009/02/10 - 9:10am

@michael-schnell

"There's a document in the JDK docs titled Programming With Assertions that says:

Do not use assertions for argument checking in public methods"

 But this is about java assertions which can be (and always are) disable, so you can't rely on them.

@misko

I agree that in the internal part of API shouldn't be any assertions ,only developers use this API and they should know where they can or cannot pass nulls;

Tracy Nelson replied on Tue, 2009/02/10 - 9:41am

I have a concern with your assertion (hmmm...):

Since the secure() method only needs to operate on door, and window, those are the only objects which I should have to create.

If someone later changes secure() to:

public void secure() {
door.lock();
for(Room room : this.getRooms()) {
if(room.hasWindows()) {
for(Window w : room.getWindows()) {
w.lock();
}
}
}
}

 then you'll need rooms as well.  Your test will fail because it's incomplete, not because of a problem with secure().  I second the suggestion that you (or more appropriately the person who created the House class) create a TestableHouseFactory to provide a mock House object suitable for testing.  That's the best way to ensure that logical invariants are met.

 

Walter Laan replied on Tue, 2009/02/10 - 10:15am

I would just use the HouseFactory in the test as well.

Also your test should check if the door and window are unlocked before the house.secure() call, in case they start locked and secure() does nothing ;).

Greg Brown replied on Tue, 2009/02/10 - 11:38am in response to: Kamil Hark

> I agree that in the internal part of API shouldn't be any assertions ,only developers > use this API and they should know where they can or cannot pass nulls; IMO, internal methods are exactly where you *should* be using assertions. They are meant to help identify errors in your code itself, not in how someone is calling your code (exceptions are used for that). -Greg

Casper Bang replied on Tue, 2009/02/10 - 2:53pm

Assertions can be enabled and disabled (at least in Java) with runtime flags, so it's not really a problem for the ones using a build environment as Ant or Maven is it? Having said that, I prefer preconditions over assertions, and non-nullable types whenever possible.

Rafael Naufal replied on Tue, 2009/02/10 - 4:26pm

 @MarkThornton

"You supposed to throw an exception instead of using an assert. However this doesn't change the problem with respect to testing --- you have to provide a valid value."

What if the unnecessary values for testing would be substituted by "NullObjects"?

Then, the code would look like this:

testSecureHouse() {
Door door = new Door();
Window window = new Window();
House house = new House(door, window,
HouseTest.createRoofForTest(), HouseTest.createKitchenForTest(),
HouseTest.createLivingRoomForTest(),
HouseTest.createBedRoomForTest());

house.secure();

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

If Roof, Kitchen, LivingRoom and BedRoom were interfaces, the null objects can have the same contract (implementing the same interface). The null objects can have only one instance because there is no need to have a lot of instances of the same NullObject. Of course they don't need to be hard-coded, they can be configured and created with Test Data Builders. Each of the collaborators instantiate Null Objects that it depends.

Michael Schnell replied on Wed, 2009/02/11 - 3:12am in response to: Kamil Hark

@kamil.hark

> Do not use assertions for argument checking in public methods

I personally think Java "assertions" are a (very) minimal attempt to achieve what is characterized as "Design By Contract". If you use something like Contract4J that works with AOP the class will look like this:

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

@Pre("door != null")
@Pre("window != null")
@Pre("roof != null")
@Pre("kitchen != null")
@Pre("livingRoom != null")
@Pre("bedRoom != null")
House(Door door, Window window,
Roof roof, Kitchen kitchen,
LivingRoom livingRoom,
BedRoom bedRoom){
this.door = door;
this.window = window;
this.roof = roof;
this.kitchen = kitchen;
this.livingRoom = livingRoom;
this.bedRoom = bedRoom;
}

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

}

You can easily turn the checking of any "assertions" (Preconditions, Postconditions and Invariants) off. This way Misko can test the class the way wants to do it.

Kamil Hark replied on Wed, 2009/02/11 - 4:17am in response to: Greg Brown

 Of cource you have to do assertions but you have two options:

1. you can use assertions in your productions code

2 you can use tests out of your production code

I prefere the 2cd option if I want check nulls etc.

I use the 1st option when I check some state of object or params and in this case somethimes use exceptions

Liam Knox replied on Wed, 2009/02/11 - 8:07am

A few points. I think JSR 308 really deals with Null checking in a far more powerfull way and this can be performed at compile time and is immediately intuitive reading the code. Without this I think you would need to favor code that asserts rather than code that is lacking in safety. You cannot test all scenarios. I think there is massive naievity that test driven approach makes it somehow safer to short cut. I think your argument that they confuse the reader is lacking. Presumably if you have static creational methods that are imported the test will look alot more clean. I would never say substitute safety for 'beauty' Interested how these view sit with your chief Java Architect ;-)

Adrian Engler replied on Thu, 2009/02/19 - 11:45am

If in this example, a house really needs a roof and a kitchen (and the other objects passed to the constructor), I don't think it is good to allow null just to make unit testing easier. What you need is a stub, and null is not necessarily a good substitute for a stub. Getting a stub can be quite easy, you can use a mock-generating framework like EasyMock - if you don't call any methods on the mock objects, they are just used as if they were simple stubs. Creating a mock object can be done in one line (createMock(...), if the type is a class type rather than an interface, classextension is needed for EasyMock).

 Null - if it is acceptable as an argument, at all, often has a special meaning. I would not generally use null when I don't care about an argument because then, it may also fail to work if there is no null check because the conditions for null aren't fulfilled. If you want to test a method call with null, write a unit test in which null is passed to the method, if you want to test a method call in which a non-null value is passed, but you don't care about the value for all the arguments, use stubs or mock objects.

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.