0
\$\begingroup\$

I needed to create a basic JDBC helper class for our JDBC connection and wanted to know if my design is appropriate or needs cleans up?


Just a basic JDBC utility class which helps reduce the boilerplate code that JDBC has.

public class JDBCUtils {

    /**
     * Retrieves a connection from the given pool.
     *
     * @param reference Name of the pool from which the connection should be retrieved.
     *
     * @return the connection
     *
     * @throws SQLException on error
     */
    public static Connection getConnection(final String reference) throws SQLException {
        ComboPooledDataSource cpds = (ComboPooledDataSource) setConnection(reference);
        return cpds.getConnection();
    }

    /**
    * Sets up the connection by accessing an xml file that has all the necessary configurations for connecting to a specific database.
    */    
    private static DataSource setConnection(final String reference) {
        Properties properties = new Properties();
        try(InputStream is = new FileInputStream(reference)) {
            properties.loadFromXML(is);
        } catch (InvalidPropertiesFormatException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        }
        ComboPooledDataSource cpds = new ComboPooledDataSource();
        try {
            cpds.setDriverClass(properties.getProperty("driverClassName"));
        } catch (PropertyVetoException e) {
            e.printStackTrace();
        }
        cpds.setJdbcUrl(properties.getProperty("jdbcUrl"));
        cpds.setUser(properties.getProperty("user"));
        cpds.setPassword(properties.getProperty("password"));

        return cpds;
    }

    /**
     * Closes this database Connection, ResultSet & Statement
     *
     * @param conn Connection
     * @param stmt Statement
     * @param rs ResultSet
     */
    public static void clearDBResources(Connection conn, Statement stmt, ResultSet rs) {
        if (rs != null) {
            try {
                rs.close();
            } catch (Exception ex) {
                LogUtils.warn("Unable to clear database resources", ex.getMessage());
            }
        }

        if (stmt != null) {
            try {
                stmt.close();
            } catch (Exception ex) {
                LogUtils.warn("Unable to clear database resources", ex.getMessage());
            }
        }

        if (conn != null) {
            try {
                if (!conn.isClosed()) {
                    conn.close();
                }
            } catch (Exception ex) {
                LogUtils.warn("Unable to clear database resources", ex.getMessage());
            }
        }
    }
}

This class is the generic data access object. It defines the basic interactions between all data objects and the data sources.

public abstract class GenericDAO {

    /**
     * This method returns a connection.
     *
     * @return the connection
     *
     * @throws SQLException on error
     */
    protected final Connection getConnection(String baseUrl) throws SQLException {
        if (!baseUrl.isEmpty() && baseUrl.contains("www") || baseUrl.contains("web")) {
            return JDBCUtils.getConnection(getPreProdDataSourceName());
        } else {
            return JDBCUtils.getConnection(getQam1DataSourceName());
        }
    }

    /**
     * This method returns the data source name.
     *
     * @return data source name String
     */
    protected abstract String getQam1DataSourceName();

    /**
     * This method returns the data source name.
     *
     * @return data source name String
     */
    protected abstract String getPreProdDataSourceName();
}

This class is the class that points to a specific database, in this case it points to the QA database. There are more classes like this but they follow the same pattern.

public class QaDAO extends GenericDAO {

    /**
     * This method returns the data source name.
     *
     * @return data source name String
     */
    protected String getQam1DataSourceName() {
        return DataSourceConfig.getQam1DataSource();
    }

    /**
     * This method returns the data source name.
     *
     * @return data source name String
     */
    protected String getPreProdDataSourceName() {
        return DataSourceConfig.getQac1DataSource();
    }
}

This class points to the path to the xml that has all the necessary configurations needed to establish a connection.

public class DataSourceConfig {    

    private static final String QAM1_DATA_SOURCE = "src/main/java/com/qam1.xml";

    private static final String QAC1_DATA_SOURCE = "src/main/java/com/qas1.xml";

    public static String getQac1DataSource() {
        return QAC1_DATA_SOURCE;
    }

    public static String getQam1DataSource() {
        return QAM1_DATA_SOURCE;
    }
}
\$\endgroup\$
1
\$\begingroup\$
  1. e.printStackTrace();

Catching exception just to do that is plain wrong. If you cannot handle it then just do not catch it. Also e.printStackTrace(); is not suitable for logging exceptions. If you just want to log it use a proper logger.

  1. ComboPooledDataSource cpds = (ComboPooledDataSource) setConnection(reference);

There is absolutely no need to downcast to a specific subclass. What will happen if if setConnection() does not return a ComboPooledDataSource ?

  1. clearDBResources()

this method seems pretty useless to me. One should use a try-finally or try-with-resources to guarantee that the resources will be closed.

  1. The DAO itself is not a dao. A true dao hides the implementation details, but yours just exposes everything.

  2. It's not clear if this is a webapp, but if it is you should let the container manage the datasource, not the application.

\$\endgroup\$
  • \$\begingroup\$ For 1. I will handle it better. I just did a e.printStackTrace() just for POC purposes. 2. I did that because the setConnection() method returns a datasource so I had to cast the c3p0 datasource. 3. K, will implement try with resources. 4. Yea, my mistake. I changed the title to say helper JDBC class. Not really DAO pattern. 5. It is not an application. I needed to create this for running automated test scripts via selenium. \$\endgroup\$ – Robben Oct 7 '16 at 15:40
  • \$\begingroup\$ You still don;t need to downcast to ComboPooledDataSource. You wan't to work with the abstraction, not the concrete implementation. But if you want the concrete implementation, then make it explicit by changing the method signature to return ComboPooledDataSource instead of the abstraction \$\endgroup\$ – Svetlin Zarev Oct 7 '16 at 16:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.