Home | Contact Us | FAQ | Search & Site Map | Link to Us
Sign In | Join | Other 45 Sites in Network
HomeAnnouncementsWhite Papers
Discussion GroupsFirst AidDatabasesJavaBeansGUIJava 3DVirtual MachineCORBASecurityToolsGeneral
Java DirectoryOpen Source ProjectsSample Book ChaptersUser GroupsWeb Resources
Related Topics
Databases.NETMore Topics ...

Java Forum / General / November 2007

Tip: Looking for answers? Try searching our database.

Database helper class with PreparedStatements

Thread view: 
teser3@hotmail.com - 09 Nov 2007 00:27 GMT
I have a JDBC working with Oracle 9i where database is inserted/
updated maybe 10 times at most during a week with very little usage in
my Tomcat 4.1.3 Container.

The Database classes I have used for the past year are working great
but I wonder if I should be closing my connection in my database
helper class with Prepared statements:

public class DbInsert
{
   private PreparedStatement stat;
   private Connection connection;

   public DbInsert(Connection connection)
   {
        this.connection = connection;
   }

   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();
   }

   //more Methods with preparedstatements here....
}

....

public class DbWork
{

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

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

  //more db methods using prepared statements here

.....
}

When I experiment and put a close statement in the [b]DbInsert[/b]
class method then my database insert wont work because it would be
closed when it is called in the [b]DbWork[/b] class?

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();
      connection.close();
   }

Please advise.
Arne Vajhøj - 09 Nov 2007 00:38 GMT
> I have a JDBC working with Oracle 9i where database is inserted/
> updated maybe 10 times at most during a week with very little usage in
[quoted text clipped - 58 lines]
> class method then my database insert wont work because it would be
> closed when it is called in the [b]DbWork[/b] class?

I am surprised that it even work with a connection.close in dbMethod.

For very small single desktop apps you can consider a single
permanent open database connection.

For a complex app or any web app, then you should use a connection
pool and only keep the connection open when you need it.

Something like:

     public void cityInserter(FormBean city) throws SQLException
     {
         Connection connection = new ConnectionMgr().getConnection();
         PreparedStatement stat = connection.prepareStatement("Insert
into City (street, school) values (?,?)");
         stat.setString(1, city.getStreet());
         stat.setString(2, city.getSchool());
         stat.executeUpdate();
         connection.close();
     }
Martin Gregorie - 09 Nov 2007 10:52 GMT
> The Database classes I have used for the past year are working great
> but I wonder if I should be closing my connection in my database
> helper class with Prepared statements:

I prefer to close a connection in the same class that opens it and at
the same logical level within the class, i.e. if you open it in a method
then close it in another method belonging to the same class and balance
the logical level of the method calls within the invoking class. I don't
open connections in a constructor because I don't like constructors to
throw exceptions, but ymmv.

As you say this is a rarely used function and is used to run a single
INSERT statement, I'd put the connection open and close into a single
invoking method so the connection exists for the minimum time.

Signature

martin@   | Martin Gregorie
gregorie. | Essex, UK
org       |

Lew - 09 Nov 2007 13:38 GMT
> I don't
> open connections in a constructor because I don't like constructors to
> throw exceptions, but ymmv.

You shouldn't open connections in a constructor because it can cause bugs.
Opening a connection is not part of object construction so it doesn't belong
in a constructor.

You shouldn't do real work from an unconstructed object.  Once the object is
completely built it's safe to use.

Signature

Lew

Arne Vajhøj - 12 Nov 2007 02:38 GMT
> You shouldn't open connections in a constructor because it can cause
> bugs. Opening a connection is not part of object construction so it
> doesn't belong in a constructor.

If the connection is a field then opening the connection
is part of the construction.

Arne
Lew - 12 Nov 2007 05:01 GMT
>> You shouldn't open connections in a constructor because it can cause
>> bugs. Opening a connection is not part of object construction so it
>> doesn't belong in a constructor.
>
> If the connection is a field then opening the connection
> is part of the construction.

Except that the OP's code didn't treat the connection that way, but closed it
prior to disposal of the object.

I should have said, "Either open the connection in the constructor and *keep
it for the object's entire lifetime*, or do not open the connection in the
constructor and go ahead and close it before the object dies."

And even if the connection is a field doesn't mean that it *must* open in
construction.  It depends on the lifetime of the connection relative to the
object's lifetime.  If the connection lives during the entire life of the
object, fine, but that wasn't the case here.

It is not uncommon to have a field represent the connection that is re-used
for new connections during the object's lifetime, and therefore not opened
during construction.

Signature

Lew

Arne Vajhøj - 12 Nov 2007 02:37 GMT
> As you say this is a rarely used function and is used to run a single
> INSERT statement, I'd put the connection open and close into a single
> invoking method so the connection exists for the minimum time.

You could do that for a frequently used method as well.

Allocate a connection from pool and deallocate is nothing
compared to the database operation itself.

Arne
Are Nybakk - 09 Nov 2007 14:54 GMT
> I have a JDBC working with Oracle 9i where database is inserted/
*snip*

I started replying to this post, but I gave it up. I don't quite get
what you want answered here. I'll give you some tips on how to organize
things.

Make a class that takes care of the database connection:

public class DatabaseConnector {

   private Connection con;

   public DatabaseConnector(user, pass, ...) {
      con = ...;
   }

   public Connection getConnection() {
      return con;
   }

   public void close() {
      con.close();
   }

   //...

}

And a database handler class:

public class DatabaseHandler {

   private DatabaseConnector dbCon;
   private Connection con;
   private PreparedStatement stmt1 = new PreparedStatement("...");

   public DatabaseHandler(user, pass, ...) {
      dbCon = new DatabaseConnector(user, pass, ...);
      con = dbCon.getConnection();
   }

   public void insertSomeObject(SomeObject obj) {
      stmt.setXxx(...);
      //...

      stmt.executeXxx();
   }

   /...

   public void closeConnection() {
      dbCon.close();
   }

}

Remember that the point of PreparedStatements is a way to declare
statements that are frequently used. To declare such a statement for
every method call removes the point entirely.

About closing connections, simply close them when you won't be using it
for a while. I don't think open connections would make any problem
unless the database is accessed by a lot of clients simultaneously.
Are Nybakk - 09 Nov 2007 15:00 GMT
>> I have a JDBC working with Oracle 9i where database is inserted/

*snip*

> unless the database is accessed by a lot of clients simultaneously.

Oh and by the way; there's a group called comp.lang.java.databases that
would fit better than this group.
Lew - 09 Nov 2007 15:10 GMT
> Remember that the point of PreparedStatements is a way to declare
> statements that are frequently used. To declare such a statement for
> every method call removes the point entirely.

Not entirely.  PreparedStatements also provide a kind of type safety for query
parameters, and protect against injection attacks.  There's really no reason
not to use PreparedStatements routinely.

Signature

Lew

teser3@hotmail.com - 10 Nov 2007 22:21 GMT
Thanks for info. I combined the classes and was wondering if this is
more efficient?

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 dbMethod(FormBean city)
  {
       try
      {
              cityInserter(city);
      }
      catch(SQLException ex)
      {
             System.out.println(ex);
      }
      finally
      {
            connection.close();
      }
  }
.....
}
Lew - 10 Nov 2007 23:04 GMT
> Thanks for info. I combined the classes and was wondering if this is
> more efficient?
[quoted text clipped - 5 lines]
> ConnectionMgr().getConnection();
>     private PreparedStatement stat;

There are consequences to making the connection happen at construction time
and making 'connection' and 'stat' instance variables.

>      public void cityInserter(FormBean city) throws SQLException
>     {
[quoted text clipped - 5 lines]
>     }
>    .......

It's better to leave the sample code compilable than to throw in this kind of
ellipsis.

>    public dbMethod(FormBean city)

You need a return type such as 'void' for a method.

>    {
>         try
[quoted text clipped - 8 lines]
>        {
>              connection.close();

The problem here is that connection is part of the instance, and created
during construction, but you close() it before the object is disposed.  This
leaves the possibility of client code attempting to re-use the object after
the connection has been closed.

>        }
>    }
> ......

<http://www.physci.org/codes/sscce.html>

> }

Signature

Lew

teser3@hotmail.com - 10 Nov 2007 23:21 GMT
> tes...@hotmail.com wrote:
> > Thanks for info. I combined the classes and was wondering if this is
[quoted text clipped - 55 lines]
> --
> Lew

Thanks for your quick response.

Would this be better where I put the Connection and PreparedStatement
instances in the method??

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

   public void dbMethod(FormBean city)
   {
        try
       {
               cityInserter(city);
       }
       catch(SQLException ex)
       {
              System.out.println(ex);
       }
      finally
       {
             connection.close();
//not sure if I corrected this issue here or not??
> The problem here is that connection is part of the instance, and created
> during construction, but you close() it before the object is disposed.  This
> leaves the possibility of client code attempting to re-use the object after
> the connection has been closed.
       }
   }
teser3@hotmail.com - 10 Nov 2007 23:26 GMT
Thanks for your quick response.

Would this be better where I put the Connection and PreparedStatement
instances in the method??

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

   public void dbMethod(FormBean city)
   {
        try
       {
               cityInserter(city);
       }
       catch(SQLException ex)
       {
              System.out.println(ex);
       }
      finally
       {
             connection.close();
//not sure if I corrected this issue here or not??

> The problem here is that connection is part of the instance, and created
> during construction, but you close() it before the object is disposed.  This
> leaves the possibility of client code attempting to re-use the object after
> the connection has been closed.

       }
   }
teser3@hotmail.com - 10 Nov 2007 23:28 GMT
Thanks for your quick response.

Would this be better where I put the Connection and PreparedStatement
instances in the method??

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

   public void dbMethod(FormBean city)
   {
        try
       {
               cityInserter(city);
       }
       catch(SQLException ex)
       {
              System.out.println(ex);
       }
      finally
       {
             connection.close();
//not sure if I corrected this issue here or not??
> The problem here is that connection is part of the instance, and created
> during construction, but you close() it before the object is disposed.  This
> leaves the possibility of client code attempting to re-use the object after
> the connection has been closed.
       }
   }
Lew - 11 Nov 2007 01:24 GMT
> Thanks for your quick response.
>
[quoted text clipped - 13 lines]
>         {
>               connection.close();

Now the problem is that this variable 'connection' has not been declared.
Remember, a variable loses scope with the closing brace.  That means that the
local variable 'connection' from method cityInserter() is not available any more.

The good new is that you can put the finally block in cityInserter() and get
rid of dbMethod().

Try it like this (some class names changed to better reflect their purpose):

 public class CityDb
 {
  private static final String INSERT_SQL =
    "INSERT INTO city (street, school) VALUES (?,?)";

  private final Logger logger = Logger.getLogger( getClass() );

  /** Insert the city into the DB.
   * @param city <code>City</code> to insert.
   * @return boolean <code>true</code> iff insert succeeded.
   */
  public City insert( City city )
  {
    List<City> batch = new ArrayList<City>();
    batch.add( city );
    return insert( batch );
  }

  /** Insert a batch of cities into the DB.
   * Uses one <code>PreparedStatement</code> for the whole batch.
   * @param city <code>City</code> to insert.
   * @return boolean <code>true</code> iff all inserts succeeded.
   */
  public boolean insert( Iterable <City> batch )
  {
    if ( batch == null )
    {
      throw new IllegalArgumentException( "Batch cannot be null" );
    }
    Connection connection = ConnectionMgr().getConnection();
    if ( connection == null )
    {
      return false;
    }
    try
    {
      return insert( batch, connection );
    }
    finally
    {
      try
      {
        connection.close();
      }
      catch ( SQLException exc )
      {
        logger.error( "Cannot close. "+ exc.getMessage();
        logger.debug( exc );
      }
    }
  }

  /** Insert a batch of cities into the DB.
   * Assumes a live <code>Connection</code> for the whole batch.
   * @param city <code>City</code> to insert.
   * @param connection <code>Connection</code> through which to insert.
   * @return boolean <code>true</code> iff all inserts succeeded.
   */
  boolean insert( Iterable <City> batch, Connection connection )
  {
    try
    {
      connection.setAutoCommit( false );

      PreparedStatement stat = connection.prepareStatement( INSERT_SQL );
      for ( City city : batch )
      {
        if ( city == null )
        {
          throw new IllegalArgumentException( "City cannot be null" );
        }
        insert( city, stat );
      }
      connection.commit();
      return true;
    }
    catch ( SQLException ex )
    {
      logger.error( "Whoops! "+ ex.getMessage();
      logger.debug( ex );
      try
      {
        connection.rollback();
      }
      catch ( SQLException exc )
      {
        logger.error( "Cannot rollback. "+ exc.getMessage();
        logger.debug( exc );
      }
      return false;
    }
  }

  private void insert( City city, PreparedStatement stat )
    throws SQLException
  {
    stat.setString( 1, city.getStreet() );
    stat.setString( 2, city.getSchool() );
    stat.executeUpdate();
  }

 }

Signature

Lew

teser3@hotmail.com - 11 Nov 2007 02:03 GMT
> tes...@hotmail.com wrote:
> > Thanks for your quick response.
[quoted text clipped - 131 lines]
>
> - Show quoted text -

Lew, Thanks for all your time and help on this!
Lew - 11 Nov 2007 02:17 GMT
> Lew, Thanks for all your time and help on this!

Bear in mind that the code I presented is itself untested.  Assuming I made no
errors, it still is not the only way to go about things.

The decomposition of the methods was designed to allow PreparedStatement reuse
where possible.  It also allows sharing a Connection between helper classes in
some putative expansion of the concept.  That's why all the overloads.

If you parametrize the class on the entity type rather than just City, you can
come up with a pretty decent Data Access superclass.  You still need to
override methods in child classes that are smart about
object<->PreparedStatement and object<->ResultSet specifics.

When I've done this I wind up with a fairly complex framework, one really big
AbstractDataAccessor class and a lot of very easy, quick child classes that
correspond to each entity.  Once the framework is in place it's easy to adapt
to new entities.

That said, the new Java Persistence API (JPA) (see also Hibernate) looks to be
a much more robust, portable way to handle object persistence to RDBMSes.

Signature

Lew

Arne Vajhøj - 12 Nov 2007 02:48 GMT
> I started replying to this post, but I gave it up. I don't quite get
> what you want answered here. I'll give you some tips on how to organize
[quoted text clipped - 21 lines]
>
> }

What does that class provide that Connection does not ?

> And a database handler class:
>
[quoted text clipped - 23 lines]
>
> }

That DatabaseConnector is not needed is emphasized by the fact
that you have both the DatabaseConnector wrapper and the underlying
Connection as fields here.

Furthermore you should really open and close connection in the
insert method. If you keep the DatabaseHandler around then you
collect way to many database connections. If you construct and
close DatabaseHandler for every insert call, then you could
just as well do everything in the insert method.

> Remember that the point of PreparedStatements is a way to declare
> statements that are frequently used. To declare such a statement for
> every method call removes the point entirely.

PreparedStatement provides other useful functionality including:
  - handling of special characters like ' (both relevant for
    scottish names and malicious SQL injection)
  - date format handling

> About closing connections, simply close them when you won't be using it
> for a while. I don't think open connections would make any problem
> unless the database is accessed by a lot of clients simultaneously.

If it does not cost anything to make the code scalable, then why'
not do it ?

You never know how long time some code will live and in how many
different contexts it will be used !

Arne
Are Nybakk - 12 Nov 2007 14:12 GMT
>> I started replying to this post, but I gave it up. I don't quite get
>> what you want answered here. I'll give you some tips on how to
[quoted text clipped - 23 lines]
>
> What does that class provide that Connection does not ?

I see your point. The idea is to isolate what has got to do with the
actual initialization and maintainance of the server connection. Only
this class needs to know about JDBC driver and such. That way it can
also be reused.

>> And a database handler class:
>>
[quoted text clipped - 27 lines]
> that you have both the DatabaseConnector wrapper and the underlying
> Connection as fields here.

Alright let me revise. I guess I wrote this in a hurry.

(Error catching left out)

public class DatabaseConnector {

   private String serverURL = "...";
   private String driverName = "...";
   private Connection con;
   private PreparedStatement stmt1 = new PreparedStatement("...");
   //...

   public DatabaseConnector(user, pass) {
      Class.forName(driverName);
      con = DriverManager.getConnection(serverURL, user, pass);
   }

   public void insertSomeObject(SomeObject obj) {
      stmt.setXxx(obj.getZzz);
      //...

      stmt.executeWww();
   }

   public boolean isActive() {
      return !con.isClosed();
   }

   public void close() {
      stmt.close();
      con.close();
   }

   //...

}

public class DatabaseHandler {

   private DatabaseConnector dbCon;

   public DatabaseHandler(user, pass, ...) {
      dbCon = new DatabaseConnector(user, pass, ...);
   }

   public insert(SomeObject obj) {
      //dbCon.insertSomeObj(...)
   }

   public insert(Collection<SomeObject> obj) {
      //loop: dbCon.insertSomeObj(...)
   }

   //...

   public void end() {
      if(dbCon.isActive()) {
         dbCon.close();
      }
      //Any other close operataions.
   }

}

> Furthermore you should really open and close connection in the
> insert method. If you keep the DatabaseHandler around then you
[quoted text clipped - 20 lines]
> You never know how long time some code will live and in how many
> different contexts it will be used !

Point taken.

> Arne
Lew - 12 Nov 2007 14:24 GMT
>>> I started replying to this post, but I gave it up. I don't quite get
>>> what you want answered here. I'll give you some tips on how to
[quoted text clipped - 68 lines]
>
>    private String serverURL = "...";

If you're going to hard-code these values, make them static final Strings (and
therefore use all upper case in their names).

>    private String driverName = "...";
>    private Connection con;
[quoted text clipped - 5 lines]
>       con = DriverManager.getConnection(serverURL, user, pass);
>    }

You don't need to load the driver class but once, not every time you make an
object.  You could do it in a static initializer for the class (assuming you
followed my advice to make the driver name a static variable, too).

Signature

Lew

Are Nybakk - 13 Nov 2007 19:59 GMT
>>>> I started replying to this post, but I gave it up. I don't quite get
*snip*
>>    private String serverURL = "...";
>
[quoted text clipped - 15 lines]
> (assuming you followed my advice to make the driver name a static
> variable, too).

Hmm, good idea.
Are Nybakk - 12 Nov 2007 14:28 GMT
>>> I started replying to this post, but I gave it up. I don't quite get
>>> what you want answered here. I'll give you some tips on how to
[quoted text clipped - 84 lines]
>       stmt.executeWww();
>    }

Commenting myself again =)
This makes it hard to reuse the class. It would be better to have a
getPreparedStatement and let DatabaseHandler take care of the actual
queries.

>    public boolean isActive() {
>       return !con.isClosed();
[quoted text clipped - 64 lines]
>
>> Arne
Are Nybakk - 14 Nov 2007 11:43 GMT
>> I started replying to this post, but I gave it up. I don't quite get
>> what you want answered here. I'll give you some tips on how to
[quoted text clipped - 23 lines]
>
> What does that class provide that Connection does not ?

Let me quote from a thread in comp.lang.java.databases:

"Lew wrote:
>> Which is worse, to take a while to construct a connection, or to run
out of connections because your objects never let them go?

David Harper wrote:
> Better *not* to let each object create its own connection.  That way,
madness lies.  As you say, this argues strongly for a connection pool.

The application code doesn't change when you switch to a connection
pool.  The semantics of the close() call change to "release to the pool"
instead of "destroy"."

Now I better understand why I've learned to do it this way. This wrapper
class is a connection pool of sorts. One with only one connection.

The pool could contain more connections if needed (several databases
perhaps) and a time-out for closing unused, open connections. It should
also have a way of "locking" the connection.

>> And a database handler class:
>>
[quoted text clipped - 33 lines]
> close DatabaseHandler for every insert call, then you could
> just as well do everything in the insert method.

Then rather, get a reference to a connection from the pool and "free" it
afterwards.

>> Remember that the point of PreparedStatements is a way to declare
>> statements that are frequently used. To declare such a statement for
[quoted text clipped - 16 lines]
>
> Arne


Free Magazines

Get these publications absolutely FREE for up to 12 months. There are no hidden fees and no obligation. Simply choose a title, complete the application form and submit it. Read more ...

Oracle MagazineNetwork ComputingComputer WorldBio-IT WorldeWeekInformation WeekInfosecurity
 
Sign In
Join
My Latest Posts
My Monitored Threads
My Blog
My Photo Gallery
My Profile
My Homepage

Start New Thread
Enable EMail Alerts
Rate this Thread



©2009 Advenet LLC   Privacy Policy - Terms of Use
This website includes both content owned or controlled by Advenet as well as content owned or controlled by third parties.