This graph, intended to show the differences in outcome between TDD and not, is a sketch of my observations combined with information extrapolated from other people's anecdotes. One datapoint is backed by third party research: Studies show that it takes about 15% longer to produce an initial solution using TDD. Hence I show in the graph the increased amount of time under TDD to get to "done."

The tradeoff mentioned in the title is that TDD takes a little longer to get to "done" than code 'n' fix. It requires incremental creation of code that is sometimes replaced with incrementally better solutions, a process that often results in a smaller overall amount of code.

When doing TDD, the time spent to go from "done" to "done done" is minimal. When doing code 'n' fix, this time is an unknown. If you're a perfect coder, it is zero time! With some sadness, I must report that I've never encountered any perfect coders, and I know that I'm not one. Instead, my experience has shown that it almost always takes longer for the code 'n' fixers to get to "done done" than what they optimistically predict.

You'll note that I've depicted the overall amount of code in both graphs to be about the same. In a couple cases now, I've seen a team take a TDD mentality, apply legacy test-after techniques, and subsequently refactor a moderate-sized system. In both cases they drove the amount of production code down to less than half the original size, while at the same time regularly adding functionality. But test code is usually as large, if not a bit larger, than the production code. In both of these "legacy salvage" cases, the total amount of code ended up being more or less a wash. Of course, TDD provides a large bonus--it produces tests that verify, document, and allow further refactoring.

Again, this graph is just a rough representation of what I've observed. A research study might be useful for those people who insist on them.

Labels: TAD, TDD, test-driven development

This simple file monitor class notifies listeners when a watched file is modified:

public class FileMonitor {
 private Set<FileChangeListener> listeners =
    new HashSet<FileChangeListener>();
 private int duration;
 private File file;
 private long lastModified;

 public FileMonitor(String filename, int duration) {
     this.file = new File(filename);
     lastModified = file.lastModified();
     this.duration = duration;
 }

 public void start() {
     boolean isDaemon = true;
     long initialDelay = duration;
     new Timer(isDaemon).schedule(
        new FileMonitorTask(), initialDelay, duration);
 }

 public void addListener(FileChangeListener listener) {
     listeners.add(listener);
 }

 class FileMonitorTask extends TimerTask {
     @Override
     public void run() {
         if (file.lastModified() > lastModified) {
             lastModified = file.lastModified();
             for (FileChangeListener listener: listeners) {
                 listener.modified();
             }
         }
     }
 }
}

A FileMonitor schedules a TimerTask that just checks the modified date from time to time, and compares it to the last modified date. The code above seems like a typical and reasonable implementation. FileMonitorTask could be an anonymous inner class, of course.

I spent more time than I would have preferred test-driving this solution, since I made the mistake of test-driving it as a single unit. These tests had to deal with the vagaries of threading due to the Timer scheduling.

import java.io.*;
import java.util.*;
import org.junit.*;
import static org.junit.Assert.*;

public class FileMonitorTest {
 private static final String FILENAME = "FileMonitorTest.properties";
 private static final int MONITOR_DURATION = 1;
 private static final int TIMEOUT = 50;
 private FileMonitor monitor;
 private File file;
 private List<FileChangeListener> fileChangeNotifications;
 private Thread waitThread;

 @Before
 public void initializeListenerCounter() {
     fileChangeNotifications = new ArrayList<FileChangeListener>();
 }

 @Before
 public void createFileAndMonitor() throws IOException {
     FileUtils.writeFile(FILENAME, "property=originalValue");
     file = new File(FILENAME);
     monitor = new FileMonitor(FILENAME, MONITOR_DURATION);
 }

 @After
 public void deleteFile() {
     FileUtils.deleteIfExists(FILENAME);
 }

 @Test
 public void shouldNotifyWhenFileModified() throws Exception {
     monitor.addListener(createCountingFileChangeListener());
     waitForFileChangeNotifications(1);

     monitor.start();

     alterFileToChangeLastModified();
     joinWaitThread();
     assertEquals(1, numberOfFileChangeNotifications());
 }

 @Test
 public void shouldSupportMultipleListeners() throws Exception {
     monitor.addListener(createCountingFileChangeListener());
     monitor.addListener(createCountingFileChangeListener());
     waitForFileChangeNotifications(2);

     monitor.start();

     alterFileToChangeLastModified();
     joinWaitThread();
     assertEquals(2, numberOfFileChangeNotifications());
     assertAllListenersDistinctlyNotified();
 }

 @Test
 public void shouldOnlyReportOnceForSingleModification()
         throws InterruptedException {
     // slow--must wait on timeout. Better way?
     monitor.addListener(createCountingFileChangeListener());
     waitForFileChangeNotifications(2);

     monitor.start();

     alterFileToChangeLastModified();

     joinWaitThread();
     assertEquals(1, numberOfFileChangeNotifications());
 }

 @Test
 public void shouldReportMultipleTimesForMultipleModification()
     throws InterruptedException {
     monitor.addListener(createCountingFileChangeListener());
     waitForFileChangeNotifications(1);

     monitor.start();

     alterFileToChangeLastModified();
     joinWaitThread();

     waitForFileChangeNotifications(2);
     alterFileToChangeLastModified();
     joinWaitThread();

     assertEquals(2, numberOfFileChangeNotifications());
 }

 private FileChangeListener createCountingFileChangeListener() {
     return new FileChangeListener() {
         @Override
         public void modified() {
             fileChangeNotifications.add(this);
         }
     };
 }

 private int numberOfFileChangeNotifications() {
     return fileChangeNotifications.size();
 }

 private void waitForFileChangeNotifications(final int expected) {
     waitThread = new Thread(new Runnable() {
         @Override
         public void run() {
             while (numberOfFileChangeNotifications() != expected) {
                 sleep(1);
             }
         }
     });
     waitThread.start();
 }

 private void sleep(int ms) {
     try {
         Thread.sleep(ms);
     } catch (InterruptedException exception) {
         fail(exception.getMessage());
     }
 }

 private void alterFileToChangeLastModified() {
     file.setLastModified(file.lastModified() + 1000);
 }

 private void joinWaitThread() throws InterruptedException {
     waitThread.join(TIMEOUT);
 }

 private void assertAllListenersDistinctlyNotified() {
     Set<FileChangeListener> notifications =
         new HashSet<FileChangeListener>(
);
     notifications.addAll(fileChangeNotifications);
     assertEquals(fileChangeNotifications.size(), notifications.size());
 }
}

Wow, that's a bit longer than I would have hoped. Yes, there are better ways to test the threading. And there may be better threading mechanisms to implement the monitor, although I think the Timer is pretty simple. I don't do enough threading, so I usually code a "familiar" solution (i.e. something I already know well) initially, and then refactor into a more expressive and concise solution.

I'm pretty sure this test is flawed. But rather than invest in fixing it, I reconsidered my design (by virtue of wanting an easier way to test it). The SUT is really two classes, the monitor and the task. Each one can easily be tested separately, in isolation, as a unit. Sure, I want to see the threading in action, but perhaps that's better relegated to an integration test (which might also double as an acceptance test).

I decided to change the solution to accommodate more focused and more robust tests. The file monitor tests are pretty much what was there before (with some small additional refactorings), except that there's now no concern over threading--I just test the run method:

import java.io.*;
import org.junit.*;
import static org.mockito.Mockito.*;
import org.mockito.*;

public class FileMonitor_TaskTest {
 private static final String FILENAME = "FileMonitorTest.properties";
 private FileMonitor.Task task = new FileMonitor.Task(FILENAME);
 @Mock
 private FileChangeListener listener;

 @Before
 public void initializeMockito() {
     MockitoAnnotations.initMocks(this);
 }

 @BeforeClass
 public static void createFile() throws IOException {
     FileUtils.writeFile(FILENAME, "property=originalValue");
 }

 @AfterClass
 public static void deleteFile() {
     FileUtils.deleteIfExists(FILENAME);
 }

 @Before
 public void createTask() {
     task = new FileMonitor.Task(FILENAME);
 }

 @Test
 public void shouldNotNotifyWhenFileNotModified() {
     task.addListener(listener);
     task.run();
     verify(listener, never()).modified();
 }

 @Test
 public void shouldNotifyWhenFileModified() throws Exception {
     task.addListener(listener);
     changeFileLastModifiedTime();

     task.run();

     verify(listener, times(1)).modified();
 }

 @Test
 public void shouldSupportMultipleListeners() throws Exception {
     task.addListener(listener);
     FileChangeListener secondListener = mock(FileChangeListener.class);
     task.addListener(secondListener);

     changeFileLastModifiedTime();
     task.run();

     verify(listener, times(1)).modified();
     verify(secondListener, times(1)).modified();
 }

 @Test
 public void shouldOnlyReportOnceForSingleModification() throws InterruptedException {
     task.addListener(listener);

     task.run();
     changeFileLastModifiedTime();
     task.run();
     task.run();

     verify(listener, times(1)).modified();
 }

 @Test
 public void shouldReportMultipleTimesForMultipleModification() throws InterruptedException {
     task.addListener(listener);

     task.run();
     changeFileLastModifiedTime();
     task.run();
     changeFileLastModifiedTime();
     task.run();

     verify(listener, times(2)).modified();
 }

 private void changeFileLastModifiedTime() {
     File file = new File(FILENAME);
     file.setLastModified(file.lastModified() + 1000);
 }
}

I introduced Mockito to provide a simple verifying stub for the listener. I suppose I should also stub out interactions with File, but I'll incur the speed penalty for now.

Next, I needed to test the remaining FileMonitor code. To do this involved proving that it creates and schedules a task appropriately using a timer, and that it handles listeners appropriately.

import xxx.CollectionUtil;
import java.util.Timer;
import org.junit.*;
import org.mockito.*;
import static org.mockito.Mockito.*;

public class FileMonitorTest {
 @Mock
 private Timer timer;

 @Before
 public void initializeMockito() {
     MockitoAnnotations.initMocks(this);
 }

 @Test
 public void shouldScheduleTimerWhenStarted() {
     String filename = "filename";
     long duration = 10;
     FileMonitor monitor = new FileMonitor(filename, duration) {
         @Override
         protected Timer createTimer() {
             return timer;
         }
     };
     monitor.start();
     verify(timer).schedule(any(FileMonitor.Task.class), eq(duration), eq(duration));
 }

 @Test
 public void shouldDelegateListenersToTask() {
     FileChangeListener listener = mock(FileChangeListener.class);
     FileMonitor monitor = new FileMonitor("", 0);
     monitor.addListener(listener);

     CollectionUtil.assertContains(monitor.getTask().getListeners(), listener);
 }
}

The design of the production code had to change based on my interest in better tests. A number of small, incremental refactoring steps led me to this solution:

import java.io.*;
import java.util.*;

public class FileMonitor {
 private final long duration;
 private Task task;

 public FileMonitor(String filename, long durationInMs) {
     task = new Task(filename);
     this.duration = durationInMs;
 }

 public void start() {
     long initialDelay = duration;
     createTimer().schedule(task, initialDelay, duration);
 }

 protected Timer createTimer() {
     final boolean isDaemon = true;
     return new Timer(isDaemon);
 }

 Task getTask() {
     return task;
 }

 public void addListener(FileChangeListener listener) {
     task.addListener(listener);
 }

 static class Task extends TimerTask {
     private final File file;
     private long lastModified;
     private Set<FileChangeListener> listeners = new HashSet<FileChangeListener>();

     Task(String filename) {
         this.file = new File(filename);
         lastModified = file.lastModified();
     }

     public void addListener(FileChangeListener listener) {
         listeners.add(listener);
     }

     Set<FileChangeListener> getListeners() {
         return listeners;
     }

     @Override
     public void run() {
         if (file.lastModified() > lastModified) {
             lastModified = file.lastModified();
             for (FileChangeListener listener: listeners) {
                 listener.modified();
             }
         }
     }
 }
}

Well, most interestingly, I no longer have any tests that must deal with additional threads. I test that the monitor delegates appropriately, and I test that the task works properly when run. I still want the integration tests, but I think they can be relegated to a higher, acceptance level test for the story:

reload properties at runtime

Otherwise, at this point, I have a high level of confidence in my code. I know the Timer class works, and I know my two classes work, and I've also demonstrated to myself that they all work together. I'm ready to move on.

Some additional relevant points:

• Threading is tough to get right, and it can be even tougher to test it.
• My final solution is a few more lines of code due to supporting my interest in more focused testing. I'm ok with the additional code because of the flexibility having the tests gives me, and more often than not, having them allows me to easily eliminate duplication elsewhere.
• It's hard to be overdo the SRP. Far more classes than not weigh in the wrong direction of too many responsibilities.
• The original solution, probably fine otherwise, is what you might expect from someone uninterested in TDD. It works (I think) but it represents rigid code, code that is more costly to change.

Labels: agile, Java, mockito, single responsibility principle, SRP, TDD, Timer, TimerTask