2

For insert record in database with JDBC, there are two approaches :

  1. Nested try.. finally : In this approach, nested try with finally use for close prepare statement and connection

    public void performInsert(String insertSQL) {
        try {
            Connection connection = dataSource.getConnection();
            try {
                PreparedStatement insertStmt = connection
                        .prepareStatement(insertSQL);
                try {
                    // bind value to prepare statements
                    insertStmt.executeUpdate();
                } finally {
                    insertStmt.close();
                }
            } finally {
                connection.close();
            }
        } catch (SQLException e) {
            // TODO: handle exception
        }
    }
    
  2. single try with if condition in finally block : In this approach single try use and in finally block use if condition for close statement and connection:

    public void performInsertIF(String insertSQL) {
        Connection connection = null;
        PreparedStatement insertStmt = null;
        try {
            connection = dataSource.getConnection();
            insertStmt = connection.prepareStatement(insertSQL);
            // bind value to prepare statements
            insertStmt.executeUpdate();
        }catch (SQLException e) {
            // TODO: handle exception
        } finally {
            if( insertStmt != null) {
                try {
                    insertStmt.close();
                } catch (SQLException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }
    
            if( connection != null) {
                try {
                    connection.close();
                } catch (SQLException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }
        }
    }
    

Both above approach is working fine, but which approach is better to use and why ?

Pshemo
  • 122,468
  • 25
  • 185
  • 269
Punit Patel
  • 901
  • 1
  • 12
  • 25
  • 5
    If you have Java 7+, you can use try-with-resources as a superior third alternative: http://stackoverflow.com/questions/8066501/how-should-i-use-try-with-resources-with-jdbc – Duncan Jones May 29 '14 at 11:10

4 Answers4

2

In the above code both approaches work fine but second is better in a way because you check for a null object before calling close() on it. Calling close() on a null object will throw a NullPointerException.

In Java 7+ now, however, you can use a better alternative called try-with-resources.

Abdul Fatir
  • 6,159
  • 5
  • 31
  • 58
  • In the first approach both `connection` and `insertStmt` will not be null when `close()` is called. – Mark Rotteveel May 29 '14 at 11:41
  • @MarkRotteveel **In the above code both approaches work fine** – Abdul Fatir May 29 '14 at 11:42
  • That is not my concern, you say _"but second is better ... because you check for a null object ... Calling close() on a null object will throw a NullPointerException"_. That doesn't apply to the first approach as neither `connection` or `insertStmt` can be `null` when `close()` is called in the code shown, invalidating your argument that the second approach is better because of that check – Mark Rotteveel May 29 '14 at 11:45
  • @MarkRotteveel You've failed to understand my argument. I meant that the second approach is better **in general**. I was not being specific to this example when I said __but second is better__. – Abdul Fatir May 29 '14 at 11:49
  • I disagree - but that is mostly a matter of taste - the first approach is IMHO better (if you are unable to use try-with-resources that is). It requires less checks (like the null checks) at the cost of additional nested try-finally blocks. – Mark Rotteveel May 29 '14 at 11:51
  • I agree with you as you said, "but that is mostly a matter of taste." – Abdul Fatir May 29 '14 at 11:52
1

The best option (assuming Java 7 or higher) is to use try-with-resources:

public void performInsert(String insertSQL) {
    try (
        Connection connection = dataSource.getConnection();
        PreparedStatement insertStmt = connection
                    .prepareStatement(insertSQL);
    ) {
        // bind value to prepare statements
        insertStmt.executeUpdate();
    } catch (SQLException e) {
        // TODO: handle exception
    }
}

After the try block ends, both connection and insertStmt will be closed automatically (in the reverse order of creation). The connection will even be closed if the statement prepare fails, or close of insertStmt fails.

Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
-1

You should got with second approach. Make a Utility with static methods, which close the resources with NULL checks.

public class DBUtils{
public static void closeConnection(Connection  con){
    if(con != null){
           // close the connection
        }
    }
}
Nitul
  • 997
  • 12
  • 35
-2

I generally use a structure that is quite similar to your second example...

public void performInsertIF(String insertSQL) {
Connection connection = null;
PreparedStatement insertStmt = null;
try {
    connection = dataSource.getConnection();
    insertStmt = connection.prepareStatement(insertSQL);
    // bind value to prepare statements
    insertStmt.executeUpdate();
}
catch (SQLException e) {
    // TODO: handle exception
}
finally {

    try {
        insertStmt.close();
    }
    catch(Exception ignore) {
    }

    try {
        connection.close();
    }
    catch(Exception ignore) {
    }
}

}

However, if you are really wanting to print the stacktrace for SQLExceptions in the finally block, I would personally go with this variation as I think it looks a bit tidier/cleaner...

public void performInsertIF(String insertSQL) {
Connection connection = null;
PreparedStatement insertStmt = null;
try {
    connection = dataSource.getConnection();
    insertStmt = connection.prepareStatement(insertSQL);
    // bind value to prepare statements
    insertStmt.executeUpdate();
}
catch (SQLException e) {
    // TODO: handle exception
}
finally {

    try {
        insertStmt.close();
    }
    catch(NullPointerException ignore) {
    }
    catch (SQLException e) {
        e.printStackTrace();
    }

    try {
        connection.close();
    }
    catch(NullPointerException ignore) {
    }
    catch (SQLException e) {
        e.printStackTrace();
    }
}

}

ban-geoengineering
  • 18,324
  • 27
  • 171
  • 253
  • You simply don't catch a `NullPointerException`. A `NullPointerException` simply proves that the code is buggy. – Abdul Fatir May 29 '14 at 11:44
  • Personally, I wouldn't catch the NPE as I would use the first example. I disagree, though, that the code is buggy as the two objects in the finally block may be null through no fault of the preceding code. – ban-geoengineering May 29 '14 at 11:51
  • You've misunderstood me brother. I didn't say your code is buggy. I said if a program throws a NPE it is in most cases buggy and has no proper checks. Hence, catching a NPE is a bad practice. – Abdul Fatir May 29 '14 at 11:54
  • But, surely, it depends on the context. Sure, catching/absorbing an NPE in the main flow is bad practice, but I personally don't believe that rule should apply in a finally block that is being used only to clean up. – ban-geoengineering May 29 '14 at 11:59
  • ...especially as we know the objects may be null and we're not interested if they are. – ban-geoengineering May 29 '14 at 12:07
  • But as you can see a simple null check is better and cleaner than a try-catch. – Abdul Fatir May 29 '14 at 12:08
  • Not in this case, as we already have a catch block for the SQLException anyway - so in my example we have less code indentation. I think my code looks cleaner, but maybe it's just personal choice. – ban-geoengineering May 29 '14 at 12:13