Exceptional Traps

Exceptions give you substantial power in your debugging and error-handling capabilities. However, they can lead to elusive corruption bugs, which can take hours to eradicate. See Example 5-7.

Example 5-7. A data table model with a problem
package oracle.hcj.finalstory;
public class ExceptionalTraps implements TableModel {
 /** Cache of the data fetched. */
 private ArrayList data; /** Holds the sql used to fetch the data. */
 private String sql;
 /** * Gets data from the the database and caches it for the table model.
 *
 * @param conn The database connection to use.
 * @param sql The SQL to use.
 *
 * @return The result value. *
 * @throws SQLException If there is a SQL error. */
 public int loadData(final Connection conn, final String sql)
 throws SQLException {
 int result = 0;
 Statement stmt = null;
 ResultSet rs = null;
 Object[] record = null;
 this.data = new ArrayList( );
 try {
 this.sql = sql;
 stmt = conn.createStatement( );
 rs = stmt.executeQuery(sql);
 int idx = 0;
 int columnCount = rs.getMetaData( ).getColumnCount( );
 while (rs.next( )) {
 record = new Object[columnCount];
 for (idx = 0; idx < columnCount; idx++) {
 record[idx] = rs.getObject(idx); }
 data.add(record);
 }
 } finally {
 if (rs != null) {
 rs.close( );
 }
 if (stmt != null) {
 stmt.close( );
 }
 }
 return result;
 }
 public void refresh(final Connection conn) throws SQLException {
 loadData(conn, this.sql);
 }
}


This snippet is from a JDBC-powered GUI. The code is supposed to provide a table model for a set of data from a database. When the user calls loadData( ), the given SQL is run against the database, and the data is stored in a cache, which will be used by the GUI. This SQL is also stored in a local data member so that the user can periodically refresh the data in the table model. Unfortunately, if this code throws an exception, it stops working in a catastrophic way. Suppose there is a transaction issue that causes the method to throw an exception. To understand what would happen, pretend you are the computer on which the method is being executed:

public int loadData(final Connection conn, final String sql)
 throws SQLException {
 int result = 0;
 Statement stmt = null;
 ResultSet rs = null;
 Object[] record = null;


Up to this point, you have set things up and initialized variables. The next thing you do is clear the old cache of data to prepare for the new records:

 this.data = new ArrayList( );


Now that you have a sparkling clean cache, you can change the sql member of the class to store the SQL that you will use in the upcoming database call:

 try {
 this.sql = sql;


Finally, you are ready to connect to the database and start loading the data into the table model:

 stmt = conn.createStatement( );
 rs = stmt.executeQuery(sql);
 int idx = 0;
 int columnCount = rs.getMetaData( ).getColumnCount( );
 while (rs.next( )) {
 record = new Object[columnCount];
 for (idx = 0; idx < columnCount; idx++) {
 record[idx] = rs.getObject(idx); 


After the seventh iteration of the loop, a transaction exception occurs, which causes this call to fail and subsequently throw a SQLException. The flow will now pass to the finally block:

 } finally {
 if (rs != null) {
 rs.close( );
 }
 if (stmt != null) {
 stmt.close( );
 }
 }
 return result;
 }


Now that you are done with the method call, let's examine the results. Hitting the exception on the rs.getObject(idx) call caused the method to end prematurely, and the data wasn't loaded completely. However, the sql member of the class now contains the SQL of the failed call. What's worse is that the data in the table is now partially loaded. The contents of the object are totally scrambled. Other objects that do not expect the data to be scrambled, such as listeners, will probably also throw exceptions. The result could be a chain reaction of hundreds of exceptions that eventually crash the program. The user, not knowing anything about databases or exceptions, fills out a bug report that says, "Using the `Find My Data' menu item sometimes causes the client to crash." Once you receive this bug report, you first try to replicate the error to figure out what is wrong with the method. However, you don't get the transaction problem the user did, so you can't reproduce the bug. You now have a transient bug that will be extremely difficult to find. There could be thousands of lines of code between the invocation of the menu item and this method. The phrase "needle in a haystack" was invented for times like this! The problem is not with the actual SQLException but with the programmer's use of exceptions within the method. In this case, the data in the method is hopelessly scrambled because the programmer did things in a problematic order. By setting instance members prior to the possible occurrence of exceptions in the class, he introduced the possibility of scrambled data. The good news is that you can block this problem with a coding standard. Here is another look at the method, rewritten to prevent corruption:

package oracle.hcj.exceptions;
public class ExceptionalTraps implements TableModel {
 public int loadData2(final Connection conn, final String sql)
 throws SQLException {
 int result = 0;
 Statement stmt = null;
 ResultSet rs = null;
 Object[] record = null;
 ArrayList temp = new ArrayList( );
 try {
 stmt = conn.createStatement( );
 rs = stmt.executeQuery(sql);
 int idx = 0;
 int columnCount = rs.getMetaData( ).getColumnCount( );
 while (rs.next( )) {
 record = new Object[columnCount];
 for (idx = 0; idx < columnCount; idx++) {
 record[idx] = rs.getObject(idx); }
 temp.add(record);
 }
 } finally {
 if (rs != null) {
 rs.close( );
 }
 if (stmt != null) {
 stmt.close( );
 }
 }
 this.data = temp;
 this.sql = sql;
 return result;
 }
}


In the new version of the loadData( ) method, you first try to do all of the operations that could cause an exception prior to setting the instance variables. To accomplish this, create a temporary list to hold the data and then add the records to that list. Once all the data is loaded and the connections are cleaned up, set the instance members of the class appropriately. The SQL is copied and the temporary list becomes the new data cache. This technique guarantees that the data can never become corrupted. If an exception is thrown in the method body, the virtual machine will execute the finally block and exit the method, and the instance members won't be set. Revisiting the example of a transaction problem, an error in processing the client's request results in old data being shown in the GUI table. Not bothering to look at the log showing the transaction exception, the user merely clicks the menu item again, and goes about his business. In the data table example, the effects of corruption are a rather minor GUI problem. In other cases, they can be much more critical. For example, a customer's order may not be completely processed, while the customer's account indicates that it has. This would result in an angry and potentially lost customer. In any app, remember that data is sacred, and data corruption is the devil. Being vigilant for signs of data corruption is not always easy because this particular devil likes to hide from you and attack when least expected. Try to flush out this devil in Example 5-8.

Example 5-8. A sneaky corruption issue
package oracle.hcj.exceptions;
public class ExceptionalTraps implements TableModel {
 public void processOrder(final Object order, final Customer customer) {
 this.processedOrders.add(order);
 doOrderProcessing( ); billCreditCard(customer); }
}


This example seems harmless enough. In reality, it's about as harmless as explosives attached to a ticking clock. The problem is that although the method does not declare any exceptions to be thrown, it isn't certain that the two called methods won't throw runtime exceptions. To illustrate, look at the billCreditCard( ) method, which is called by the processOrder( ) method:

package oracle.hcj.exceptions;
public class ExceptionalTraps implements TableModel {
 public void billCreditCard(final Customer customer) {
 if (customer == null) {
 throw new NullPointerException( );
 }
 System.err.println(customer);
 }
}


Since runtime exceptions aren't required to be declared in the method declaration, they could be lurking in these methods and you wouldn't know it. In this case, if the customer is null, a NullPointerException will be thrown. If these exceptions do occur, then the system would think you have processed the order because you have already added it to the processedOrders member, but you have actually failed to process the order. The best way to avoid this problem, and the main rule you should establish for all of your code, is, whenever possible, to make the setting of instance variables the last thing you do in any method. You can rewrite the method in Example 5-8 to reflect this policy:

 public void processOrder(final Object order, final Customer customer) {
 doOrderProcessing( ); billCreditCard(customer);  this.processedOrders.add(order);
 }


In the revised method, the order will be added to processedOrders only after all other operations have succeeded. In this manner, you can be sure that an order is processed properly and is not in a corrupted state. One example of where it isn't possible to set instance variables in the method after everything else has been completed is demonstrated by the a bound property of a Java bean:

 public void setSomeProperty(final String someProperty) {
 final String oldSomeProperty = this.someProperty;
 this.someProperty = someProperty;
 propertyChangeSupport.firePropertyChange("someProperty", 
 oldSomeProperty, 
 someProperty);
 }


In this case, the bound property has to actually change before the listeners are informed of the property change events. If you mix up the steps, then any class getting the event and reading the new values would actually be reading old data. The fact that you have no choice but to put the event-firing after the setting of the instance variable is the only excuse for setting instance variables before performing any dangerous work. However, there is still a problem with your exceptions. The fact that the firePropertyChange( ) and the methods called after it can cause runtime exceptions leaves you with a hole in your logic. For example, consider a GUI in which the event fired causes data to be reloaded and then other panels to be refreshed. In this case, the firePropertyChange( ) is the entry point in potentially hundreds of consecutive method calls, any of which could throw runtime exceptions and corrupt the object. This problem can be solved by using exception firewalls. The idea behind exception firewalls stems from the fact that the listeners of an object are passive in nature. The object firing the events is notifying the listeners of an event, but it couldn't care less if the listeners actually react because of the event. In this case, you could build a shield around the exceptions that allow your program to continue:

 public void setSomeProperty2(final String someProperty) {
 final String oldSomeProperty = this.someProperty;
 this.someProperty = someProperty;
 try {
 propertyChangeSupport.firePropertyChange("someProperty",
 oldSomeProperty,
 someProperty);
 } catch (final Exception ex) {
 LOGGER.error("Exception in Listener", ex);
 }
 }


In the revised version of your setter, the exceptions that occur as a result of notifying the listener of the event are caught and routed to a logging device and will not propagate through the setter. Not only does this technique protect the bean object from corruption, it also allows the actual position of the exception to be found much more efficiently. If your event had caused another event, which in turn had caused another event, the exception could have propagated through several methods before it was reported. Naturally, since this code is specific to the actual firing of the events and not the beans, the exception firewall code would be better off in the listener support classes, such as PropertyListenerSupport. Note that this technique is appropriate only for passive listeners. In the case of a PropertyVetoListener, this technique would not be appropriate because the setter has to abort the change if the change is vetoed. In all other cases, you should wait until the last lines of the method to set values of instance members. By using temporary variables to hold intermediate results in long methods, this is easily accomplished.

      
Comments