Efficient Connection

[chicago1985]

I created a Java class that inserts and updates an Oracle 9i database. I am running single insert and single update statements.
The below is what I have been using and was wondering if it is efficient or anyway to improve it:

public class DbWork
{

    private Connection connection = new ConnectionMgr().getConnection();
    private PreparedStatement stat;

     public void cityInserter(FormBean city) throws SQLException  {
        stat = connection.prepareStatement("Insert into City (street, school) values (?,?)");
        stat.setString(1, city.getStreet());
        stat.setString(2, city.getSchool());
        stat.executeUpdate();
    }

    public void cityUpdater(FormBean city) throws SQLException  {
        stat = connection.prepareStatement("update Person set PersonId = ? where name LIKE ? ");
        stat.setInt(1, city.getPersonId());
        stat.setString(2, city.getName());
        stat.executeUpdate():
    }


   public void dbMethod(FormBean city)
   {
        try 
       {
               cityInserter(city);
               cityUpdater(city);
       }
       catch(SQLException ex)
       {
              System.out.println(ex);
       }
       finally
       {
             connection.close();
       }

}

[jwenting]

that's the worst thing you could have done.
Creating a Connection as a class member and initialising it. Then using it in one public method without closing it while closing it in another.
And in neither case checking whether it's open...

You're not closing your PreparedStatement, which you should always do.

1) NEVER have JDBC resources as class fields.
2) NEVER leave JDBC resources open longer than strictly required.
3) NEVER rely on the user to close those resources for you.

[chicago1985]

that's the worst thing you could have done.
Creating a Connection as a class member and initialising it. Then using it in one public method without closing it while closing it in another.
And in neither case checking whether it's open...

You're not closing your PreparedStatement, which you should always do.

1) NEVER have JDBC resources as class fields.
2) NEVER leave JDBC resources open longer than strictly required.
3) NEVER rely on the user to close those resources for you.

Thanks for the info. I made some changes and I hope this is okay?

public class DbWork
{

    private Connection connection;

        public DbWork() {
           connection = new ConnectionMgr().getConnection();
        }


        public void cityInserter(FormBean city) throws SQLException  {
        PreparedStatement stat = connection.prepareStatement("Insert into City (street, school) values (?,?)");
        stat.setString(1, city.getStreet());
        stat.setString(2, city.getSchool());
        stat.executeUpdate();
        stat.close();
    }

     public void cityUpdater(FormBean city) throws SQLException  {        
          PreparedStatement stat = connection.prepareStatement("update Person set PersonId = ? where name LIKE ? ");        
           stat.setInt(1, city.getPersonId());        
           stat.setString(2, city.getName());              
           stat.executeUpdate():   
           stat.close(); 
     }


   public void dbMethod(FormBean city)
   {
        try 
       {
               if(connection != null)
               {
                  cityInserter(city);
               }
               if(connection != null)
               {
                  cityUpdater(city);
               }
       }
       catch(SQLException ex)
       {
              System.out.println(ex);
       }
       finally
       {
             connection.close();
       }

}

[jwenting]

somewhat. Move the creation of the database connection into the dbMethod as well though and delete it as a class field (instead making it local to the method).