java - Do this implimentation of C3P0 in my web application is correct? What are the pros and cons of this implimentation? -

i need implement connection pooling web application uses servlet , jsp pages. don't want connection pooling across application in server, want implement connection pool across servlets.

i have working implementation of c3p0, don't know correct way , gives expected result.

following implementation have datasource file c3p0 implementation return connection. connection retrieved on servelet init method , closed on destroy method.

• do type of implementation give expected result?
• do have close connection in destroy?
• what other ways of implementing connection pooling servlet app?
• i move db related actions separate classes, in case things have take care of , have connection in servlet , pass db methods in case ?
• please give pros , cons of current implementation

please find connection pooling implementation below

datasource file returns connection , servelt uses connection

datasource file

import java.beans.propertyvetoexception; import java.io.ioexception; import java.sql.connection; import java.sql.sqlexception;  import com.mchange.v2.c3p0.combopooleddatasource;   public class datasource {      // jdbc driver name , database url     static final string jdbc_driver = "com.mysql.jdbc.driver";     static final string db_url = "jdbc:mysql://dburl/dbname";      // database credentials     static final string user = "username";     static final string pass = "password";        private static datasource     datasource;     private combopooleddatasource cpds;      private datasource() throws ioexception, sqlexception, propertyvetoexception {         cpds = new combopooleddatasource();         cpds.setdriverclass("com.mysql.jdbc.driver"); //loads jdbc driver          cpds.setjdbcurl(db_url);         cpds.setuser(user);         cpds.setpassword(pass);          // settings below optional -- c3p0 can work defaults         cpds.setminpoolsize(5);         cpds.setacquireincrement(5);         cpds.setmaxpoolsize(20);         cpds.setmaxstatements(180);      }      public static datasource getinstance() throws ioexception, sqlexception, propertyvetoexception {         if (datasource == null) {             datasource = new datasource();             return datasource;         } else {             return datasource;         }     }      public connection getconnection() throws sqlexception {         return this.cpds.getconnection();     }  } 

servlet

import java.beans.propertyvetoexception; import java.io.ioexception; import java.io.printwriter; import java.sql.connection; import java.sql.resultset; import java.sql.sqlexception; import java.sql.statement;  import javax.servlet.servletexception; import javax.servlet.annotation.webservlet; import javax.servlet.http.httpservlet; import javax.servlet.http.httpservletrequest; import javax.servlet.http.httpservletresponse;  /**  * servlet implementation class connectiontest  */ @webservlet("/connectiontest") public class connectiontest extends httpservlet {   connection connection = null; statement statement = null;  @override public void init() throws servletexception {     super.init();      try {         connection = datasource.getinstance().getconnection();      } catch (propertyvetoexception e) {         e.printstacktrace();     } catch (sqlexception e) {         e.printstacktrace();     } catch (ioexception e) {         e.printstacktrace();     }  }  /**  * @see httpservlet#doget(httpservletrequest request, httpservletresponse  *      response)  */ protected void doget(httpservletrequest request,         httpservletresponse response) throws servletexception, ioexception {      printwriter out = response.getwriter();     out.println("<h1> hi</h1>");      resultset resultset = null;     try {          statement = connection.createstatement();         resultset = statement                 .executequery("select vehicleregistration  registration");         while (resultset.next()) {              string first = resultset.getstring("vehicleregistration");              // display values             out.println("<h1> " + first + "</h1>");          }     } catch (sqlexception e) {         e.printstacktrace();     } {         if (resultset != null)             try {                 resultset.close();             } catch (sqlexception e) {                 e.printstacktrace();             }         if (statement != null)             try {                 statement.close();             } catch (sqlexception e) {                 e.printstacktrace();             }     } }  @override public void destroy() {     if (connection != null) {         try {              connection.close();          } catch (sqlexception e) {             e.printstacktrace();         }     }     super.destroy(); }  } 

i welcomes suggestions small or large helps me improve code

advance all

so, improved.

your servlet should acquire reference datasource in init(...), , not hold connection object outstanding life of servlet. connection objects should acquired connection-pooling datasource on as-needed basis, not held open long periods of time. doget(...) method should acquire connection (which fast, because in pool!), , take care call close() on connection in block.

it's important change this. besides ickiness , poor resource use of holding connection open forever, under current architecture, simultaneous requests servlet operate concurrently on same connection, can lead unexpected , incorrect behavior, particular if there servlets in webapp make use of transactions (as nontrivial applications must).

your datasource class doesn't seem particularly useful. why not construct c3p0 datasource instead of defining new kind of object getconnection() method. think motivation may have been have static member datasource 1 true datasource. if want store datasource static memberm can anywhere, there's no need wrap actual datasource.

the best place hold connection-pool backed datasource in javaee web application not static member, though. better approach construct datasource in servletcontextlistener, in contextinitialized(...), , store datasource in application scope. in contextdestroyed(...) call close() c3p0 combopooleddatasource, , remove application scope.