Simple code, subtle bugs: write unit tests!

We developed an applet some years ago that was supposed to run 24/7. Basically, it fetches some values periodically and plots them in a chart. It always shows the last 24 hours. Everything seemed to work fine but every few weeks it crashed. The problems were clouded by hardware instabilities and the use of some obscure JVM. After quite a bit of analysis it became clear that it was a bug in our applet: a memory leak. Here is the troublemaker code:

for (int i = 0; i < dataSeries.getItemCount(); i++) {
    XYDataItem dataItem = dataSeries.getDataItem(i);
    if (dataItem.getX().longValue() < startDate.getTime().getTime()) {
        dataSeries.remove(i);
    } else {
        break;
    }
}
dataSeries.add(newDataPoint);

This simple and innocently looking piece of code essentially removes old items from a list by index. Many of you may already have spotted the main problem leading to the leak. Removing items by index means that following items shift one place towards the head of the list. As the index is incremented one element is skipped by the next iteration. This added up over time and lead to an OutOfMemoryError after some weeks.

Now, even if the code is not great it does relatively look straightforward on first sight, yet it is not and is contains errors. Making things worse, the code was buried somewhere in some tangled logic. This leads me to the point of my post:

Write unit tests even for relatively simple code.

Most likely writing a unit test for this cleanup work would have led to a class that manages the dataSeries and nothing more. Quite easy to understand and test in isolation. The problem would never have slipped into production and caused months of  investigating different stability problems.

The programmer may not have been aware of iterators but he should have made sure that this block works as intended. The best way to do this is automated unit tests. They make sure that your building blocks are as solid as you expect them to be. Use acceptance testing to make sure you have put your building blocks together the right way. Together unit and acceptance tests will save countless hours hunting regressions.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s