To split or not to split

When managing a resource such as a file or a database connection, it is necessary to free the resource once you're done with it. Of course, on any given execution of the code, you want to obtain the resource exactly once, and free it exactly once. There are a couple of ways you could go about doing this:

• You could obtain and free the resource in the same method. This way, you could guarantee that each time the resource is obtained, it is also freed.
  
  
• You could trace through each possible execution path of the code and ensure that the resource is eventually freed in each instance.

The second option can be problematic. Because your code base will inevitably grow, another programmer who is not familiar with your code may add another execution path in which the resource is not freed, resulting, of course, in resource leakage.

Back to top

The Split Cleaner pattern

I call bugs that fit this pattern split cleaners because the cleanup code for the resource is split along the various possible execution paths. Because the cleanup code along each path is likely to be identical, most split cleaners are also examples of rogue tiles. (Rogue tiles are what I call bugs resulting from first copying and pasting code, and then forgetting to appropriately modify all copies of the code when a change is made. For more on rogue tiles, see my article, "Bug patterns: An introduction.")

For example, suppose you are using JDBC to manipulate a table of employee names. Many of the operations you want to perform involve walking over this table and computing over the contained data. One thing you may end up doing is printing out the first names of all your employees, like so:

import java.sql.*;

public class Example {

    public static void main(String[] args) {

	String url = "your database url";
	
	try {
	    Connection con = DriverManager.getConnection(url);
	    new TablePrinter(con, "Names").walk();
	}
	catch (SQLException e) {
	    throw new RuntimeException(e.toString());
	}
    }
}

abstract class TableWalker {

    Connection con;
    String tableName;

    public TableWalker(Connection _con, String _tableName) {
	this.con = _con;
	this.tableName = _tableName;
    }

    public void walk() throws SQLException {
	String queryString =("SELECT * FROM " + tableName);
	Statement stmt = con.createStatement();
	ResultSet rs = stmt.executeQuery(queryString);
	
	while (rs.next()) {
	    execute(rs);	    
	}
	con.close();
    }

    public abstract void execute(ResultSet rs) throws SQLException;
}

class TablePrinter extends TableWalker {

    public TablePrinter(Connection _con, String _tableName) {
	super(_con, _tableName);
    }

    public void execute(ResultSet rs) throws SQLException {
	String s = rs.getString("FIRST_NAME");
	System.out.println(s);
    }

}

First, a short digression. Notice that I've pulled out the code to handle walking over the table into an abstract Walker class, allowing for new subclasses to easily walk over the rows of a table. Although it is often a waste of time to try to anticipate and code for all the ways in which a program will be extended, let's suppose that in this case there is absolutely no doubt whatsoever that exactly this kind of extension will be made to the code. (In fact, I can guarantee that just such an extension will be made before the end of this article).

Back to top

The symptoms

Now, notice that the database connection is passed into the constructor of the TableWalker. Once it is done walking over the table, it closes the connection.

So, in this case, we've used the second strategy for cleaning up the connection. We've attempted to close the connection separately along each execution path.

Let's suppose that in the context of our system it makes sense to close the connection after a single walk over the data (for example, perhaps this code is intended to be called from the command prompt). Even in that case, we haven't caught every possible execution path -- if an SQLException is thrown, the program may abort before ever closing the connection.

Because SQLExceptions are not so common in mature code, this bug may not demonstrate any symptoms for a long time, perhaps not until the original developer has already moved on. Naturally, this will make things harder to diagnose when the symptoms do appear.

But if the code is extended, there are ways in which symptoms might appear much more rapidly.

For instance, let's suppose that after the original code was written, it became clear that the bulk of the phone numbers on file were out of date. So management decides that all employee phone numbers should be replaced with "411". To perform this update, a new TableWalker would be written, as follows:

class TableFixer extends TableWalker {

    public TableFixer(Connection _con, String _tableName) {
	super(_con, _tableName);
    }

    public void execute(ResultSet rs) throws SQLException {
	String s = rs.getString("FIRST_NAME");
	String updateString = "UPDATE " + tableName +
                              "SET PHONE_NUMBER = " + "411" +
                              "WHERE FIRST_NAME LIKE '" + s + "'";
	Statement stmt = con.createStatement();
	stmt.executeUpdate(updateString);
    }
}	

Because TableFixer also extends TableWalker, calling walk on an instance of this class will close the connection to the database, just like TablePrinter. If a program were to try to make instances of both walkers with the same connection, it would find that the connection was closed as soon as the first walker finished.

It would be easy for an incoming programmer to make this mistake, especially if the invariant -- that only one walker could be constructed -- wasn't documented or tested.

Back to top

Cures and preventions

When you find an execution path that doesn't include the appropriate cleanup code, you might be tempted to simply add it to that path.

For example, you could wrap the body of the walk method in a try block, and put in a finally clause to guarantee that the connection will be closed. But such a solution would be a bad fix.

There is really no reason why our TableWalkers should need to worry about closing the connection at all. Even if each TableWalker did try to close the connection, we would run into the second way in which this bug pattern can manifest itself -- when we want to run multiple walkers, too many attempts are made to close the connection.

Worse still, if we were to put in two calls to con.close() (one in the try block, and one in the catch block, as opposed to a single call in a finally clause), we would have introduced a rogue tile into the code.

If many such rogue tiles were added, it would become quite difficult to ever refactor the code successfully. Some of the rogue tiles might handle rare execution paths that may not occur, even during testing.

A much better solution would be to refactor the code to use the first approach to managing such resources: put the code for obtaining and releasing a resource in the same method.

In their excellent book, The Pragmatic Programmer, Andrew Hunt and Dave Thomas advocate this idea through the catch-phrase "Finish What You Started." Each method should be responsible for cleaning up resources that it acquires. In our example, this would involve moving the call to con.close() into the main method of class Example, as follows:

public class Example {

    public static void main(String[] args) {

	String url = "your database url";
	
	// con must be declared and initialized here, so as to be in
	// the scope of both the try and finally clauses.
	Connection con = null;
	
	try {
	    con = DriverManager.getConnection(url, "Fernanda", "J8");
	    new TablePrinter(con, "Names").walk();
	}
	catch (SQLException e) {
	    throw new RuntimeException(e.toString());
	}
	finally {
	    try {
		con.close();
	    }
	    catch (Exception e) {
		throw new RuntimeException(e.toString());
	    }
	}
    }
}

Here, the call to con.close() is in a finally clause of the same try block that created the connection, preventing any possible execution path in which it's not called.

Back to top

Wrapup

We can summarize this week's bug pattern as follows:

• Pattern: Split Cleaner
• Symptoms: A program that improperly manages resources, either by leaking them or by freeing them too early.
• Cause: Some execution paths of the program do not free the resource exactly one time as they should.
• Cures and preventions: Move the code to handle cleanup into the same method that obtains the resource.

Enough said.

Resources

• The JUnit home page provides links to many interesting articles discussing program testing methods, as well as the last version of JUnit.
  
  
• If you like JUnit, check out the entire set of xUnit testing tools for many different languages.
  
  
• I'd be remiss if I didn't mention that the xUnit suite of tools is designed for use with Extreme Programming, a new and powerful way of developing clean, robust software quickly.
  
  
• "The UML Profile for Framework Architectures" (PDF slide show) highlights a detailed case study of JUnit.
  
  
• Take the Java debugging tutorial (developerWorks, February 2001) for help with general debugging techniques.
  
  
• The Pragmatic Programmer (Addison-Wesley, October 1999) by Andrew Hunt and David Thomas can help you improve your programming skills.
  
  
• New to Java development or looking to brush up on your Java programming skills? Take this comprehensive tutorial, "Introduction to Java programming" (developerWorks, November 2000).
  
  
• Read all of Eric's Diagnosing Java code articles, many of which focus on bug patterns.
  
  
• Find more Java technology resources on the developerWorks Java technology zone.
  
  

About the author

Comments (Undergoing maintenance)

Back to top

Trademarks  |  My developerWorks terms and conditions

Table of contents

• To split or not to split
• The Split Cleaner pattern
• The symptoms
• Cures and preventions
• Wrapup
• Resources
• About the author
• Comments

Next steps from IBM

Spring, JRuby, and Ajax development is easier with WebSphere Application Server - a smart Java 5 and J2EE Web services-based application server.

---

• Try: The no-charge WebSphere Application Server Community Edition is a pre-integrated, lightweight Java 5 application server built on Apache Tomcat and other best-of-breed open source software such as OpenEJB, Apache Axis, and Apache Derby.
• Article: The article leverage the Spring Framework and the WebSphere Application Server to improve your J2EE project productivity.
• Tutorial: See how the free WebSphere Application Server and XML can improve the efficiency of your JRuby on Rails and Ajax development.
• Buy: WebSphere Application Server - Express

My developerWorks community

Dig deeper into Java on developerWorks

Rate a product. Write a review.

Write a review for Rational products today!

Special offers