59.

About This Bug Pattern

As we said earlier, since the clean up code along each path is likely to be identical, most Split Cleaners are also examples of Rogue Tiles.

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:

Listing 16-1: Code That Walks over a Table of Employees

 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 we'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 chapter.)

Tip 

It is often a waste of time to try to anticipate and code for all the ways in which a program will be extended. Instead, try to find the simplest design that will solve the problem at hand, and cover your code with unit tests.When you need to extend or modify the program's functionality, your unit tests will allow you to do so with confidence.

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 is no longer available. 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 could be written, as follows:

Listing 16-2: Code Walker to Update Old Data

 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 its walk.

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.

The Cause

Some execution paths of the program do not free the resource exactly one time as they should.

Cures and Preventions

When you find an execution path that doesn't include the appropriate clean up 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. But 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 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 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 with the phrase "Finish What You Started."

Each method should be responsible for cleaning up the 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:

Listing 16-3: Code Refactored So That Resources Are Obtained and Released in the Same Method

 class Example2 {   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; thus, an execution path in which it's not called is impossible.