Connection to a database, do I need to create a new one and close it everytime?

Question

I currenctly have a class which does several requests to a database with several methods (called from main method) sharing this pattern :

public int getSomething(String id, File file) {
    Connection conn = null;
    PreparedStatement ps = null;
    ResultSet rs = null;
    int lines = 0;

    try {
        conn = openConnection();
        ps = conn.prepareStatement(SQL_QUERY.replace("?", id));
        ps.setFetchSize(1500);
        rs = ps.executeQuery();

        lines += nnpw.writeInFile(file, rs);
    } catch (Exception e) {
        e.printStackTrace();
    } finally {
        close(rs, ps, conn);
    }
    return lines;
}

The method openConnection() is in the main class and looks like this :

protected Connection openConnection() throws SQLException, ClassNotFoundException {
    Class.forName(jdbcParams.getString("jdbc.driver"));
    Connection connect;
    connect = DriverManager.getConnection(jdbcParams.getString("jdbc.url"), //$NON-NLS-1$ 
            jdbcParams.getString("jdbc.username"), //$NON-NLS-1$ 
            jdbcParams.getString("jdbc.password")); //$NON-NLS-1$
    return connect;
}

Would it be better if I had a Connection parameter in the main class, which I would open and close like this, and modify the close(rs, ps, conn) from above to close(rs, ps) ?

public MainClass {
    Connection conn;

    public static void main(String[] args) {
        try {
            int lines = 0;
            DBExtract ex = new DBExtract();
            ...
            conn = openConnection()
            lines += ex.getSomething("1", file, conn);
            lines += ex.getSomethingElse("1", file, conn);
            lines += ex.getSomethingDifferent("1", file, conn);
            conn.close()
        } catch(Exception e) {
            e.printStackTrace();
        }
    }
}

Answer 1

I have to agree with Praveen's comment that you should open a connection when you need it and close it when your are done with the Database operations.

But first I have to make a serious objection to this statement:

ps = conn.prepareStatement(SQL_QUERY.replace("?", id));

That is not the way you should use prepared statements!

By using that, you are opening yourself to SQL Injection. Consider what would happen if an id something like 'random-useless-name' OR 1 == 1; DROP TABLE sometablename; -- was given. Catastrophic failure!

Instead, use this:

ps = conn.prepareStatement(SQL_QUERY);
ps.setString(1, id);

You might want to take a look at the PreparedStatement tutorial to see how PreparedStatements are normally used. If you are using parameters in the query, use the setXXX methods!

---

I would say that yes, it is better if you extract the Connection parameter to the main method and provide it to your getSomething method(s). I would use it as a local variable if possible though. Also make sure to put the conn.close(); inside a finally statement and not at the end of the try.

public static void main(String[] args) {
    Connection conn = null;
    try {
        int lines = 0;
        DBExtract ex = new DBExtract();
        ...
        conn = openConnection();
        lines += ex.getSomething("1", file, conn);
        lines += ex.getSomethingElse("1", file, conn);
        lines += ex.getSomethingDifferent("1", file, conn);
    } catch(Exception e) {
        e.printStackTrace();
    } finally {
        if (conn != null)
            conn.close();
    }
}

In this way the caller could also control the transactional state of bulk operations, that is it can decide if and when to commit or to rollback.

Finally, using local variables in place of member variables reduces the chance of having connection leaks, because you really need to be sure to call the "close" method.

---

Future suggestions:

If you have an application that requires several database connections that should be able to be open simultaneously I recommend using c3p0 Connection Pooling

If you are working on a mid-size to large-size project and want some assistance with the database stuff, I recommend learning Hibernate

C3P0 and Hibernate can be used at the same time, but they require some configuration. They take a while to configure for the first time, but once you got it running they do a lot of things for you. (They're just suggestions though, you should prioritize the other things first!)