Is it Ok to Pass ResultSet?

Question

In my situation, I am querying a database for a specific return (in this case registration information based on a username).

            //Build SQL String and Query Database.
        if(formValid){
            try {
                SQL = "SELECT * FROM users WHERE username=? AND email=?";
                Collections.addAll(fields, username, email);
                results = services.DataService.getData(SQL, fields);
                if (!results.next()){
                    errMessages.add("User account not found.");
                } else {
                    user = new User();
                    user.fillUser(results); //Is it ok to pass ResultSet Around?
                }
            } catch (SQLException e) {
                e.printStackTrace();
            } finally {
                services.DataService.closeDataObjects(); //Does this close the ResultSet I passed to fillUser?
            }
        }

So once I query the database, if a result is found I create a new User object and populate it with the data I received from the database. I used to do all of this directly in the method that I was pulling the resultset into, but I realized I was doing a lot of redundant coding throughout my project so I moved it all into one central method that lives in the actual User bean.

    public void fillUser(ResultSet data) throws SQLException{
    setUserId(data.getInt("id"));
    setFirstName(data.getString("first_name"));
    setLastName(data.getString("last_name"));
    setUsername(data.getString("username"));
    setType(data.getString("type"));
    setEmail(data.getString("email"));
}

I have done a few tests and from what I can determine, because I close the original resultset in the finally block of the query, the resultset that I pass into the fillUser method also gets closed. Or am I wrong and am I seriously leaking data? This is actually the second time I pass a resultset (so its two instances of one) because the block I use to query my database is

    public static ResultSet getData(String SQL, ArrayList fields) throws SQLException {
    try{
        connection = Database.getConnection();
        preparedStatement = connection.prepareStatement(SQL);

        for(int i=0; i<fields.size(); i++){
            Integer num = i + 1;
            Object item = fields.get(i);

            if(item instanceof String){
                preparedStatement.setString(num, (String) item); //Array item is String.
            } else if (item instanceof Integer){
                preparedStatement.setInt(num, (Integer) item); //Array item is Integer.
            }
        }

        resultSet = preparedStatement.executeQuery();
        return resultSet;
    }finally{

    }
}

All of these code snippets live in separate classes and are reused multiple times throughout my project. Is it ok to pass a resultset around like this, or should I be attempting another method? My goal is to reduce the codes redundancy, but i'm not sure if i'm going about it in a legal manner.

Answer 1

Technically, it's OK to pass result sets, as long as you are not serializing and passing it to a different JVM, and your JDBC connection and statement are still open.

However, it's probably a better software engineer and programming practice to have DB access layer that returns you the result set in a Java encoded way (a list of User in your example). That way, your code would be cleaner and you won't have to worry if the ResultSet is already opened, or you have to scroll it to the top, you name it...

Answer 2

As everyone before me said its a bad idea to pass the result set. If you are using Connection pool library like c3p0 then you can safely user CachedRowSet and its implementation CachedRowSetImpl. Using this you can close the connection. It will only use connection when required. Here is snippet from the java doc:

A CachedRowSet object is a disconnected rowset, which means that it makes use of a connection to its data source only briefly. It connects to its data source while it is reading data to populate itself with rows and again while it is propagating changes back to its underlying data source. The rest of the time, a CachedRowSet object is disconnected, including while its data is being modified. Being disconnected makes a RowSet object much leaner and therefore much easier to pass to another component. For example, a disconnected RowSet object can be serialized and passed over the wire to a thin client such as a personal digital assistant (PDA).

Here is the code snippet for querying and returning ResultSet:

public ResultSet getContent(String queryStr) {
    Connection conn = null;
    Statement stmt = null;
    ResultSet resultSet = null;
    CachedRowSetImpl crs = null;
    try {
        Connection conn = dataSource.getConnection();
        stmt = conn.createStatement();
        resultSet = stmt.executeQuery(queryStr);

        crs = new CachedRowSetImpl();
        crs.populate(resultSet);
    } catch (SQLException e) {
        throw new IllegalStateException("Unable to execute query: " + queryStr, e);
    }finally {
        try {
            if (resultSet != null) {
                resultSet.close();
            }
            if (stmt != null) {
                stmt.close();
            }
            if (conn != null) {
                conn.close();
            }
        } catch (SQLException e) {
            LOGGER.error("Ignored", e);
        }
    }

    return crs;
}

Here is the snippet for creating data source using c3p0:

 ComboPooledDataSource cpds = new ComboPooledDataSource();
            try {
                cpds.setDriverClass("<driver class>"); //loads the jdbc driver
            } catch (PropertyVetoException e) {
                e.printStackTrace();
                return;
            }
            cpds.setJdbcUrl("jdbc:<url>");
            cpds.setMinPoolSize(5);
            cpds.setAcquireIncrement(5);
            cpds.setMaxPoolSize(20);

 javax.sql.DataSource dataSource = cpds;