1

I have a Java application with a Swing GUI that uses a swing worker to pull data out of a database (e.g., SQLite or MySQL) to fill a JTable. The swing worker uses JDBC and puts chunks of rows at a time into the table.

To do this, I adapted code found here to my purposes. The code contains a JDBCModel class, which extends an AbstractTableModel to store the data for the JTable. The code also contains a JDBCWorker class, which extends SwingWorker, to access the database and add the rows to the table model.

The constructor of the JDBCModel starts out by establishing a connection, executing a query, and creating a ResultSet:

try {
    Statement s = conn.createStatement();
    rs = s.executeQuery(query);
    meta = rs.getMetaData();
    JDBCWorker worker = new JDBCWorker();
    jpb.setIndeterminate(true);
    worker.execute();
} catch (SQLException e) {
    e.printStackTrace(System.err);
}

The JDBCWorker then simply iterates through the result set and creates rows for the table. The JDBCWorker is defined as a private class within the JDBCModel. This is how the JDBCWorker iterates through the result set:

protected List<Row> doInBackground() {
    try {
        while (rs.next()) {
            Row r = new Row();
            // omitting some additional computations for brevity...
            publish(r);
        }
    } catch (SQLException e) {
        e.printStackTrace(System.err);
    }
    return data;
}

In my own code, I use connection pooling instead of keeping the same connection alive. I have modified the code as follows in order to be able to request a new connection from the data source I defined in a separate Sql class. I also moved the private JDBCWorker class out of the JDBCModel class. Each time the table needs to be re-populated, a new worker is created. This is what the worker now looks like with the connection pooling; it uses try-with-resources to close the connection, statement, and result set automatically after use:

protected List<Row> doInBackground() {
    try (Connection conn = sql.getDataSource().getConnection();
            PreparedStatement statement = conn.prepareStatement(query)) {
        ResultSet rs = statement.executeQuery();
        while (rs.next()) {
            Row r = new Row();
            // omitting some additional computations for brevity...
            publish(r);
        }
    } catch (SQLException e) {
        e.printStackTrace(System.err);
    }
    return null;
}

It seems to work fine, but I am now concerned about properly separating my code. I have the following three interrelated questions:

  1. Is this reasonably efficient, or is there a strong recommendation to keep a connection alive for hours in the background just for the worker, as is done in the original code?
  2. Ideally I would like to move all the SQL- and JDBC-related code into my Sql class for writing more cleanly separated code. But it looks like the publish method of the worker must be nested inside the try-with-resources block of the result set because the set would already be closed if it wasn't inside the block. How do I separate the two tasks into separate classes/methods without keeping the connection alive forever, losing track of the connection, or mixing SQL and GUI/table model code?
  3. Is there a way to auto-close the connection I create in the Sql class as soon as the returned ResultSet object is destroyed or has reached the end of the while loop?
Philip Leifeld
  • 2,253
  • 15
  • 24

1 Answers1

0

After seeing trashgod's comment, I have come up with a solution that separates the two concerns as much as possible and, following the helpful comment, closes the connection later. I am posting it as an answer despite knowing that it is not perfect, hoping somebody can come up with something better.

In my Sql class, I have created a method that does the query and saves the result set along with the statement and connection and returns everything:

public SqlStuff getSqlStuff() {
    String query = "SELECT * FROM MyTable;";
    ResultSet rs = null;
    Connection conn = null;
    PreparedStatement statement = null;
    try {
        conn = getDataSource().getConnection();
        statement = conn.prepareStatement(query);
        rs = statement.executeQuery();
    } catch (SQLException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    } finally {
        // nothing to close here because the result set is still needed
    }
    SqlStuff s = new SqlStuff(rs, tableStatement, conn);
    return s;
}

The connection, statement, and result set need to be closed by the swing worker once the data have been extracted, hence they cannot be closed yet and all need to be stored in an SqlStuff object and returned for this purpose. This container object is defined as follows:

public class SqlStuff implements AutoCloseable {
    ResultSet rs;
    PreparedStatement ps;
    Connection c;
    
    public SqlStuff(ResultSet rs, PreparedStatement ps, Connection c) {
        this.rs = rs;
        this.ps = ps;
        this.c = c;
    }
    
    public void close() {
        rs.close();
        ps.close();
        c.close();
    }
    
    public ResultSet getResultSet() {
        return rs;
    }
}

TheSqlStuff class implements AutoCloseable and a close method. Thus the swing worker can use it in a try-with-resources block, and it will automatically close the connection, statement, and result set when done. The swing worker processes the results as follows:

protected List<TableDocument> doInBackground() {
    
    try (SqlStuff s = sql.getSqlStuff();
            ResultSet rs = s.getResultSet()) {
        while (rs.next()) {
            Row r = new Row(rs.getString(1));
            // omitting additional getString and getInt calls for brevity
            publish(r);
        }
    } catch (SQLException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
    return null;
}

The doInBackground function closes the connection, statement, and result set after the result set has been processed because of the AutoCloseable interface. There does not seem to be a noticeable performance difference compared to the version I had before, and the separation into the Sql class and the GUI/model concerns is stricter, which makes it more maintainable by experts on each subject.

However, there are still some downsides to this: The code is much longer. If somebody else starts using the getResultSet method, they may not realize that the connection still needs to be closed and may fail to employ try-with-resources or call close manually. And there is still not a perfect separation of concerns because the swing worker still has to close the connection and work through the result set, which is JDBC code. But I fail to see how one could achieve better separation (comments are welcome). At least with this code I have all the SQL and some of the JDBC stuff together in the Sql class, even if there is some overhead.

I guess as an alternative I could have moved the whole doInBackground function into the separate Sql class and referenced it in the swing worker to do the work. But I would not know how to tell the function then where to get the publish method from.

Philip Leifeld
  • 2,253
  • 15
  • 24
  • @trashgod I don't think it's necessary in this case because I implemented `AutoCloseable` and used it as a resource. See [here](https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html). – Philip Leifeld Aug 25 '21 at 22:59