4

My DAO object

package com.myselect;

import java.sql.*;
import java.sql.DriverManager;

import java.util.logging.Level;
import java.util.logging.Logger;

public class DataAccess {

    Connection conn;
    PreparedStatement pst;
    ResultSet rs;

    public DataAccess() {
        try {
            Class.forName("com.mysql.jdbc.Driver");
            conn = (Connection) DriverManager.getConnection("jdbc:mysql://localhost/db", "root", "root");
        } catch (ClassNotFoundException ex) {
            Logger.getLogger(DataAccess.class.getName()).log(Level.SEVERE, null, ex);
        } catch (SQLException ex) {
            Logger.getLogger(DataAccess.class.getName()).log(Level.SEVERE, null, ex);
        }

    }

    public String getAge(String name) {
        String userage = null;
        try {
            pst = conn.prepareStatement("select age from mydb where name= ?");
            pst.setString(1, name);

            rs = pst.executeQuery();

            while (rs.next()) {
                userage = rs.getString("age");
                System.out.println(userage);
            }

        } catch (Exception ex) {
            Logger.getLogger(DataAccess.class.getName()).log(Level.SEVERE, null, ex);
        }

        return userage;
    }

    public int insertRecord(String name, int addage) {
        int b = 0;
        try {
            pst = conn.prepareStatement("insert into mydb values(?,?)");
            pst.setString(1, name);
            pst.setInt(2, addage);
            b = pst.executeUpdate();

        } catch (Exception ex) {
            Logger.getLogger(DataAccess.class.getName()).log(Level.SEVERE, null, ex);
        }

        return b;
    }


}

I want to close the connection. I have several servlets which calls insertRecord and getAge methods.What is the best way to close my connection? Create another method and call from insertRecord and getAge method or in constructor?

M A
  • 71,713
  • 13
  • 134
  • 174
  • Why not have 2 methods, first to open jdbc resource and second to close them? –  Apr 27 '15 at 09:25
  • You've by the way another major problem: suppressing exceptions and continuing as if nothing exceptional happened. This way the servlet is never aware if the DAO call succeeded or failed and thus unable to show an error page or message. See also this another answer for an example: http://stackoverflow.com/questions/5003142/jsp-using-mvc-and-jdbc – BalusC Apr 27 '15 at 11:08

2 Answers2

2

Connections should be opened and closed when needed. In your case, you are opening a connection at one point, and then, using it at another. This means that you open a connection and potentially not use it for some time.

What you would need to do, would be to make the methods self contained, meaning that the methods should open the connection, and when done, close it. The closing part should be done within a finally block, this will ensure that regardless what happens, the collection will always be closed.

Something like so:

public String getAge(String name) {
        String userage = null;
        Connection conn = null;
        try {
            Class.forName("com.mysql.jdbc.Driver");
            conn = (Connection) DriverManager.getConnection("jdbc:mysql://localhost/db", "root", "root");

            PreparedStatement pst = conn.prepareStatement("select age from mydb where name= ?");
            pst.setString(1, name);

            rs = pst.executeQuery();

            while (rs.next()) {
                userage = rs.getString("age");
                System.out.println(userage);
            }

        } catch (Exception ex) {
            Logger.getLogger(DataAccess.class.getName()).log(Level.SEVERE, null, ex);
        }
        finally {
            if(conn != null) {
                conn.close();
            }

            if(pst != null) {
                pst.close();
            }

            if(rs != null) {
                rs.close();
            }
        }

        return userage;
    }

EDIT: As per @virtualpathum suggestion, you can also take a look at 3rd party frameworks which provide you with means to manage your connection pool, usually through the implementation of the Singleton programming pattern.

npinti
  • 51,780
  • 5
  • 72
  • 96
2

Before considering the connection, you should also always close the ResultSet and Statement objects every time you use them. Don't keep them as state in your DataAccess object. Have them as local variables and close them after using them:

public String getAge(String name) {
    PreparedStatement pst = null;
    ResultSet rs = null;
    String userage = null;
    try {
        pst = conn.prepareStatement("select age from mydb where name= ?");
        pst.setString(1, name);

        rs = pst.executeQuery();

        while (rs.next()) {
            userage = rs.getString("age");
            System.out.println(userage);
        }

    } catch (Exception ex) {
        Logger.getLogger(DataAccess.class.getName()).log(Level.SEVERE, null, ex);
    } finally {
       // closing JDBC resources
       if(rs != null) {
          rs.close();
       }

      if(pst != null) {
          pst.close();
       }

    return userage;
}

Same for the connection: Ideally, every method should create a Connection object and close it after its use. Depending on how frequently you open and close a connection in a method, you may want to use a connection pool to share open connections for more efficiency.

For closing connections and its resources, you can create a method to do that:

public void closeConnection(Connection connection, Statement statement, ResultSet resultSet) {
     ... // close resources here   
}
M A
  • 71,713
  • 13
  • 134
  • 174
  • If i had more methods then DO I NEED TO add repetitive codes such as Connection c= null; etc again and again in all methods? Cant there be a solution where I can call a static method to close all the connection? – Chandler Chris Pratt Apr 27 '15 at 09:16
  • @ChandlerChrisPratt Of course you can group the closing in a method, and same for retrieving a connection from scratch or from a pool in case you used one. – M A Apr 27 '15 at 10:26