User Interface Programming

Is any code so trivial that testing it is a waste of time? In theory, possibly. In real life, no code is ever that trivial, as I hope the following will show.

Let’s make a Person class – the sort of thing any half-decent programmer can do dead drunk, downwind of an elephant with halitosis.  Blindfolded.  (The programmer, that is; not the elephant.)

public class Person {

    private String name;

    public void setName( String name ) {

        this.name = name;

    }

    public String getName() {

        return name;

    }

}

This is impossible to get wrong, right?  Right.  If this class were guaranteed never to change until Hell froze over.  But no such guarantees exist in Real Life; let’s test properly and see where the journey takes us.

Eclipse’s “New JUnit Test” wizard suggests testSetName() and testGetName() methods.  As testing the getter without testing the setter (or vice versa) is just silly, we’ll combine them.

We should write the test first and the code afterwards – to write new code, you need a broken test. That’s the law, and you break it at your peril.  But in any case: We run the test, and (unsurprisingly) it passes.

After using the Person class a while, we discover that it’s tedious and error prone to check for null names all over the code; we’d like to change it so that the name is never null. 

• We could go ahead and change the code, watch the test break, then fix the test.
• We could change the test to reflect the new behavior, watch the red bar indicate broken code, then fix the code.

The latter is best.  After all, the test embodies the functional specs. The tests should drive the code, not the other way around. Remember the TDD Primary Directive:  To write new code, you need a broken test.

You should never ever do both changes at once, without testing in bet­ween. That brief moment of red-bar-ness is where you gain confidence that the test code catches the bugs it purports to catch.

Anyway. There are two obvious ways of effecting our goal – either change getName() to return a blank string if name == null, or ensure that name is never null in the first place. Our gut likes the second option best, so let’s do that.

We must make two changes:

• Initialize name to a blank string;
• ensure that setName() is never called with a null argument.

Here’s the updated test, which we expect to be broken in two places.

To fix the first failure we add an initializer to name.

Person

private String name = "";

To fix the second failure we check for a null argument to setName().

Person

public void setName( String name ) {

    if ( null == name ) {

        throw new NullPointerException( "null name not allowed" );

    }

    this.name = name;

}

And we’re back to the green bar.  Huzzah.

Now we want to show some Persons in a list box, and figure it’d be nice if the list box got to know about it when persons change. Let’s introduce a plain-vanilla notification mechanism.

Person

public interface Listener {

    public void personChanged( Person person );

}

private Collection< Listener > listeners = new HashSet< Listener >();

public void addListener( Listener listener ) {

    listeners.add( listener );

}

protected void fireNotification() {

    for ( Listener listener : listeners ) {

        listener.personChanged( this );

    }

}

(We don’t need removeListener() for this toy pro­g­ram. (And if this were C# we wouldn't have to bother with those tedious methods; we’d just use events instead. (And if I go one level deeper, this will turn into Lisp.)))

To test the notification mechanism, we need to create a listener in PersonTest. Its only job is to register whether it’s been called (by setting its person member to non-null).

PersonTest

private static class TestListener implements Listener {

    Person person = null;

    public void personChanged( Person changedPerson ) {

        person = changedPerson;

    }

}

Next, we must install the listener and verify that it gets or doesn’t get called in the app­ro­p­ria­te places.  We could stuff these new tests into the existing testSetAndGetName() method, but even though the notification mechanism is clearly related to setting, it has nothing to do with getting.  Instead, let’s create a new test method with a suitably descriptive name.

Aside from the naming issue, another reason to split a test method into two is the need for tests to run independently of one another.  We’ll revisit that aspect later; it’s one that can burn you badly.

The last test is broken, since the Person class doesn’t actually send any notifications.  We can fix that easily enough; we’re still living in Trivial City.

Person

public void setName( String name ) {

    if ( null == name ) {

        throw new NullPointerException(

                "null name not allowed" );

    }

    this.name = name;

    fireNotification();

}

Whops!  We fixed the broken test, but broke the one above – by sending a notification when nothing actually changed.  Let’s try again.

Person

public void setName( String name ) {

    if ( null == name ) {

        throw new NullPointerException(

                "null name not allowed" );

    }

    if ( !name.equals( this.name ) ) {

        this.name = name;

        fireNotification();

    }

}

And we’re back to the green bar.

Invoking equals() on a null name provokes exactly the NullPointerException we want, so we can dispense with the explicit check.

Person

public void setName( String name ) {

    if ( null == name ) {

        throw new NullPointerException( "null name not allowed" );

    }

    if ( !name.equals( this.name ) ) {

        this.name = name;

        fireNotification();

    }

}

The bar is still green.

Now management, out of the goodness of its collective heart, has decided to supply this poor, hitherto homeless Person with an address.  The semantics of the address field follow that of the name field, so we’ll just copy the relevant bits, both in the test class and the functional code.

We’re still living in Trivial City, right?  Writing that test was a waste of time.  But look what happens:

No change expected:<null> but was:<testdemo.Person@55571e>

    at PersonTest.testSetAddressNotification(PersonTest.java:73)

Woot!  Tie my face to a pig and drag me through the mud – ¿que pasa? (As an exercise, you might go back, read the code and see if you can spot the bug.)

Here is the broken test.

Even though the address passed to setAddress() was the same as the previous address, we still got a notification.

Finding the bug is easy enough; it’s a typical copy & paste error.

Person

public void setAddress( String address ) {

    if ( !address.equals( this.name ) ) {

        this.address = address;

        fireNotification();

    }

}

The upshot of this is that the address field is set (and a notification sent) even if the new address is the same as the old one. The extra notifications are unlikely to bring the pro­g­ram to its knees, but the flip side of the problem is much worse.  So instead of just fixing the bug, let’s write another test to expose the problem to the bright light of day.

PersonTest

@Test

public void testSetAddressToSameAsName() {

    Person person = new Person();

    person.setName( "Duckburg" );

    assertEquals( "Name", "Duckburg", person.getName() );

    person.setAddress( "Duckburg" );

    assertEquals( "Address", "Duckburg", person.getAddress() );

}

The test fails exactly the way we expect:

Address expected:<[Duckburg]> but was:<[]>

A person with name == address is not a common occurrence, so years might pass before this par­ti­cu­lar bug ever bit anyone.  But a real-life Person class would have fields galore.  Some of them might be expected to hold sometimes-overlapping values.  And the chance of a copy & paste error is proportional to the number of fields.  We’d have on our clammy little hands a pro­g­ram that failed occasionally for ob­scu­re reasons.

It seems we’re not in Trivial City anymore.

(Yes, we also used copy & paste on the test code, and might conceivably have made a corresponding error there.  But the chances of two such errors matching up are an order of magnitude lower.)

The whole object, and nothing but

If we’re feeling paranoid (as we sometimes should), the test for setAddress() might also verify that we didn’t accidentally set the name instead.  Consider a different copy & paste bug from the one we had above:

Person

public String getAddress() {

    return name;

}

This would be caught at once with the tests we have in place.  But consider instead a sloppily written test.

The test above would not catch the bug, for two reasons:

• We are using the same test string for both name and address;
• the tests are interdependent – if we were to set/get address first, the result would have been different.

Our suspenders are broken. Could we have been saved by the belt?  We could, by verifying the value of all the fields instead of only getting the one we just set.  Adding lots of verifications for individual fields is tedious and error-prone, though; it’s easier and safer to consider the whole person.

Person

@Override

public String toString() {

    return name + ", " + address;

}

That little trick would have caught the bug after all, even in an otherwise sloppy test.

(Oops.  Forgot to write tests for Person.toString()...)

Conclusion

With a solid PersonTest in place, we are free to change and refactor Person at will, confident that the tests will catch any mistakes. But if we don’t start with (and build on) the trivialities, something is bound to slip through the cracks.

I mean, it stands to reason that you are perfect and never make mistakes.  But I make mistakes all the time.  When I make a mistake hacking your code, wouldn’t you want to know about it?

In case you’re wondering, this is not a contrived example.  The code has been stripped to the bone, but I recently found a bug just like this one while retrofitting unit tests onto an old class.

You really, really, really want a solid test suite for your code base.

 

---

(Start bottom menu)

Top • Home • Articles • Book • Resources
Win­dows De­vel­oper Maga­zine • R&D Books • CMP Books
Amazon.com • Amazon.co.uk • Contact Petter Hesselberg

(End bottom menu)