0

I'm little confused about closing connection by jdbc.

package Login;
public class LoginFrame {
    private JFrame loginFrame;
    Connection conn = null;
    /**
     * Launch the application.
     */
    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                try {
                    LoginFrame window = new LoginFrame();
                    window.loginFrame.setVisible(true);
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }
        });
    }

    /**
     * Create the application.
     */
    public LoginFrame() {
        initialize();
        conn = DBConnect.connectDB();
    }

    /**
     * Initialize the contents of the frame.
     */
    private void initialize() {
        loginFrame = new JFrame();
        loginFrame.setResizable(false);
        loginFrame.setTitle("XXX");
        loginFrame.setBounds(100, 100, 350, 300);
        loginFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        loginFrame.getContentPane().setLayout(null);

        panel = new JPanel();
        panel.setBorder(new TitledBorder(UIManager.getBorder("TitledBorder.border"), "Login", TitledBorder.LEADING, TitledBorder.TOP, null, SystemColor.inactiveCaptionText));
        panel.setBounds(96, 140, 139, 99);
        loginFrame.getContentPane().add(panel);
        panel.setLayout(null);

        loginField = new JTextField();
        loginField.setBounds(47, 16, 86, 20);
        panel.add(loginField);
        loginField.setColumns(10);

        passwordField = new JPasswordField();
        passwordField.setBounds(47, 37, 86, 20);
        panel.add(passwordField);

        JButton loginButton = new JButton("Login");
        loginButton.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent arg0) {

                String sql = "select * from employees where login=? and password=?";
                try{
                    PreparedStatement pst = conn.prepareStatement(sql);
                    pst.setString(1, loginField.getText());
                    pst.setString(2, passwordField.getText());
                    ResultSet rs = pst.executeQuery();
                    int countUsr = 0;
                    while(rs.next()){
                        countUsr++;
                    }
                    if(countUsr == 1){
                        loginFrame.dispose();
                        AdminFrame adminFrame = new AdminFrame();
                        adminFrame.setVisible(true);
                    }else  if(countUsr > 1){
                        JOptionPane.showMessageDialog(null, "ERR");
                    }else{
                        JOptionPane.showMessageDialog(null, "ERR");
                        passwordField.setText("");
                    }
                rs.close();
                pst.close();
                }catch(Exception e){
                    JOptionPane.showMessageDialog(null, "ERR: "+e.getMessage());
                }
            }
        });
        loginButton.setBounds(25, 65, 89, 23);
        panel.add(loginButton);
    }
}   

I'm not sure which metod is better to use to close connection:

@Override
protected void finalize() throws Throwable {
    conn.close();
    super.finalize();
}

or

finally {
    conn.close();
}

after a try catch block in button ActionListener.

In some examples people say finally block is better but what when I have many methods(4example Action Listeners) and in every of them I do some operations on DB. Should I open and close connection in all methods or just use finalize method?

sebix
  • 2,475
  • 2
  • 25
  • 35
Wahuu
  • 1
  • 7
  • You should open and close a connection in the shortest possible scope. – Luiggi Mendoza Dec 02 '14 at 20:06
  • If you are reusing the connection then you can leave it open and close it when you are completely done like your first example. No point in creating a new connection for each function. – brso05 Dec 02 '14 at 20:08
  • @brso05 if the connection times out then there's no way to re use the connection. Even if you use a connection from a database connection pool, you should call the `close` method. – Luiggi Mendoza Dec 02 '14 at 20:10
  • I'm reusing this one connection in 10 more ActionListeners. @Luiggi Mendoza so better way is to use finally block in each ActionListener method? Isnt that reduce performance? – Wahuu Dec 02 '14 at 20:11
  • @LuiggiMendoza you should check if the connection is timed out and if it is create a new connection. You actually don't even need to explicitly call the `close` method it will be closed automatically. – brso05 Dec 02 '14 at 20:11
  • @brso05 And perform lot's of `SELECT 1` statements to your database every X seconds like a heartbeat? That's not a good idea. I would prefer to use a database connection pool that can truly reuse the same physical connection rather than doing manually. – Luiggi Mendoza Dec 02 '14 at 20:14
  • @LuiggiMendoza yes you can use a database connection pool that would be fine. But just answering his question he should reuse the connection and not create a new connection each time. Also you don't have to perform every X seconds just check if the connection is open before you try to use if its not get new connection you can create a method to do this. – brso05 Dec 02 '14 at 20:16
  • @brso05 and how will you reuse the physical database connection if not by using a database connection pool? Again, doing this manually is not recommended, and declaring the connection as a `static` field for *reuse* and keep it it open is a naive approach. – Luiggi Mendoza Dec 02 '14 at 20:17

6 Answers6

5

When working with databases, there are some concepts to take into account:

  • Do not keep a connection open too much time. To accomplish this, you should open and close it in the shortest possible scope. This will also help you from using the same connection to perform multiple actions in the same transaction, specially in multi thread environments.
  • Reuse the physical connections only. This means, you open the connection once, send it to SLEEP state and reuse it in future times. To accomplish this, do not reinvent the wheel, instead use a database connection pool which automatically handles this for you.

Steps to do this:

  • At the beginning of your application, create the database connection pool.
  • In every method when you need to perform some action against the database, obtain the connection from the database connection pool, perform your actions against database, then close the connection.
  • At the end of the application, close the database connection pool.

Here's an example to do this using HikariCP:

Define the data source that will use the database connection pool:

public final class DataSourceFactory {

    private static final Logger LOG = LoggerFactory.getLogger(DataSourceFactory.class);

    private static DataSource mySQLDataSource;

    private DataSourceFactory() { }

    private static DataSource getDataSource(String configurationProperties) {
        Properties conf = new Properties();
        try {
            conf.load(DataSourceFactory.class.getClassLoader().getResourceAsStream(configurationProperties));
        } catch (IOException e) {
            LOG.error("Can't locate database configuration", e);
        }
        HikariConfig config = new HikariConfig(conf);
        HikariDataSource dataSource = new HikariDataSource(config);
        return dataSource;
    }

    public static DataSource getMySQLDataSource() {
        LOG.debug("Retrieving data source for MySQL");
        if (mySQLDataSource == null) {
            synchronized(DataSourceFactory.class) {
                if (mySQLDataSource == null) {
                    LOG.debug("Creating data source for MySQL");
                    mySQLDataSource = getDataSource("mysql-connection.properties");
                }
            }
        }
        return mySQLDataSource;
    }
}

Use the connection in the shortest possible scope.

public class LoginFrame {
    private JFrame loginFrame;
    //remove the Connection from here, this is not the shortest possible scope
    //Connection conn = null;
    /**
     * Launch the application.
     */
    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                try {
                    LoginFrame window = new LoginFrame();
                    window.loginFrame.setVisible(true);
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }
        });
    }

    /**
     * Create the application.
     */
    public LoginFrame() {
        initialize();
        conn = DBConnect.connectDB();
    }

    /**
     * Initialize the contents of the frame.
     */
    private void initialize() {
        //...
        loginButton.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent arg0) {
                //This is the shortest possible scope for the connection
                //Declare it here and use it
                Connection conn = DataSourceFactory.getMySQLDataSource().getConnection();
                String sql = "select * from employees where login=? and password=?";
                try{
                    PreparedStatement pst = conn.prepareStatement(sql);
                    pst.setString(1, loginField.getText());
                    pst.setString(2, passwordField.getText());
                    ResultSet rs = pst.executeQuery();
                    int countUsr = 0;
                    while(rs.next()){
                        countUsr++;
                    }
                    if(countUsr == 1){
                        loginFrame.dispose();
                        AdminFrame adminFrame = new AdminFrame();
                        adminFrame.setVisible(true);
                    }else  if(countUsr > 1){
                        JOptionPane.showMessageDialog(null, "ERR");
                    }else{
                        JOptionPane.showMessageDialog(null, "ERR");
                        passwordField.setText("");
                    }
                } catch(Exception e) {
                    //ALWAYS log the exception, don't just show a message
                    e.printStackTrace();
                    JOptionPane.showMessageDialog(null, "ERR: "+e.getMessage());
                } finally {
                    try {
                        rs.close();
                        pst.close();
                        con.close();
                    } catch (SQLException silent) {
                        //do nothing
                    }
                }
            }
        });
        loginButton.setBounds(25, 65, 89, 23);
        panel.add(loginButton);
    }
}

If you're working with Java 7 or superior, then use try-with-resources (which is syntactic sugar, after all):

loginButton.addActionListener(new ActionListener() {
    public void actionPerformed(ActionEvent arg0) {
        //This is the shortest possible scope for the connection
        //Declare it here and use it
        Connection conn = ;
        String sql = "select * from employees where login=? and password=?";
        try(Connection conn = DataSourceFactory.getMySQLDataSource().getConnection();
            PreparedStatement pst = conn.prepareStatement(sql);) {
            pst.setString(1, loginField.getText());
            pst.setString(2, passwordField.getText());
            try (ResultSet rs = pst.executeQuery();) {
                int countUsr = 0;
                while(rs.next()){
                    countUsr++;
                }
                if(countUsr == 1){
                    loginFrame.dispose();
                    AdminFrame adminFrame = new AdminFrame();
                    adminFrame.setVisible(true);
                } else if(countUsr > 1){
                    JOptionPane.showMessageDialog(null, "ERR");
                } else {
                    JOptionPane.showMessageDialog(null, "ERR");
                    passwordField.setText("");
                }
            }
        } catch(Exception e) {
            //ALWAYS log the exception, don't just show a message
            e.printStackTrace();
            JOptionPane.showMessageDialog(null, "ERR: "+e.getMessage());
        }
    }
});
Luiggi Mendoza
  • 81,685
  • 14
  • 140
  • 306
2

If you are using Java 7 and above, I would recommend using try with resources.

The try-with-resources statement ensures that each resource is closed at the end of the statement. Any object that implements java.lang.AutoCloseable, which includes all objects which implement java.io.Closeable, can be used as a resource.

In your case:

        try (PreparedStatement pst = conn.prepareStatement(sql))//use try with resources
            {
            pst.setString(1, loginField.getText());
            pst.setString(2, passwordField.getText());
            ResultSet rs = pst.executeQuery();
            int countUsr = 0;
            while(rs.next()){
                countUsr++;
            }
            if(countUsr == 1){
                loginFrame.dispose();
                AdminFrame adminFrame = new AdminFrame();
                adminFrame.setVisible(true);
            }else  if(countUsr > 1){
                JOptionPane.showMessageDialog(null, "ERR");
            }else{
                JOptionPane.showMessageDialog(null, "ERR");
                passwordField.setText("");
            }
        //removed rst closing, no need to close if your PreparedStatement is being closed.
        //No need to explicitly close our PreparedStatement since we are using try with resources
        }catch(Exception e){
            JOptionPane.showMessageDialog(null, "ERR: "+e.getMessage());
        }
    }

You should also note that you don't need to close your ResultSet if you are closing your PreparedStatement. (See this answer)

Community
  • 1
  • 1
Alexander Kohler
  • 1,657
  • 14
  • 21
  • So in every ActionListener method in try block i should open connection and after all job in finaly block close connection? Isnt that reduce performance if i have lots of ActionListeners methods? – Wahuu Dec 02 '14 at 20:25
  • I don't think you'll notice much change in performance from this. It's better off to open and close as needed as opposed to worrying about a "global connection". You should strive to make your scope as small as possible. – Alexander Kohler Dec 02 '14 at 20:29
1

The nice thing about Java is all of PreparedStatement, Connection, and ResultSet use AutoCloseable. If you want to create a method that will close all of these instances in one shot:

public static void close(AutoCloseable... closeables) {
    for (AutoCloseable c : closeables) {
        try {
            if (c != null) {
                c.close();
            }
        } catch (Exception ex) {
            //do something, Logger or or your own message
        }
    }

}

You can then call this method and toss in any instances you have created that use AutoCloseable with no fixed length of parameters.

It is best to use close() calls in a finally block because if an Exception is thrown, the finally block will execute anyway. Otherwise you may run into issues with multiple connections being left open.

Drew Kennedy
  • 3,950
  • 4
  • 20
  • 33
0

The finally is more appropriate is always called. Remember any check that conn is not null

Adam111p
  • 2,529
  • 1
  • 20
  • 18
0

The standard way is to close it in the finally block to save DB resources and avoid leakage. The best results can be obtained using connection pooling with idle timeout: http://www.mchange.com/projects/c3p0/index.html.

Simon Sadetsky
  • 524
  • 2
  • 5
0

I would use a try/catch/finally for all of the methods/transactions. And you should check if the connection, result set and prepared statements are not null before closing them. Something like this for the finally block of a method:

finally {
    try {
    if (rset != null) {
        rset.close();
    }
    if (st != null) {
        st.close();
    }
    } catch (Exception e) {
    // do something
    }
}

When you are finished using the database, I would close the connection with a method:

public void close() {
if (!isOpen) return;
try {
    if (conn != null) {
    conn.close();
    }
} catch (Exception e) {
    // do something
}
isOpen = false;
conn = null;
}

Hope it helps :-)

jferris92
  • 11
  • 4