Christopher Schultz wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Neil,
>
> Just a few comments on your connection acquisition code:
>
> On 7/14/2009 12:45 PM, Neil Youngman wrote:
>> private static final String CONTEXT_NAME = "java:/comp/env";
>> private static final String DB_NAME = "jdbc/AppDatabase";
>
> If you have the same class in two applications (except for these two
> constants), why not use the same class plus a configuration file?
It's not the same class, but they share the code that connects to the database.
It might be advantageous to factor it out, but they are currently small self
contained apps.
>> private static Connection getConnection ()
>> throws Exception, SQLException
>> {
>> if( DATA_SOURCE == null )
>
> Hmm... a DataSource object is not guaranteed to be threadsafe. I would
> not bother to cache the DataSource object in your class: at least not in
> a static field.
OK, I'm happy to drop that.
> The "performance hit" you experience from performing repeated JNDI
> lookups is negligible, and your code has the benefit of always getting
> the "current" DataSource from the directory.
>
>> // Get the data source (from context.xml)
>> try
>> {
>> Class.forName("oracle.jdbc.OracleDriver");
>
> This line of code is not necessary at all, for several reasons:
>
> 1. The connection pool will already make sure this class is loaded
>
> 2. Standard JDBC practice is to call Class.forName(driver).newInstance()
> to make sure that the driver is, in fact, registered with the DriverManager.
>
> 3. A driver used with a DataSource does not bother to register itself
> with the DriverManager (see javadoc for javax.sql.DataSource).
>
> This is all you need:
>
>> Context initContext = new InitialContext();
>> Context envContext = (Context)
>> initContext.lookup(CONTEXT_NAME);
>> DATA_SOURCE = (DataSource)envContext.lookup(DB_NAME);
>
> That's it.
>
>> catch (Exception ex)
>> {
>> System.err.println( ex );
>> ex.printStackTrace();
>> throw ex;
>> }
>
>
> This exception handler doesn't add much: the caller has the opportunity
> to catch this exception, so why log it at this level?
I prefer not to rely on the caller logging exceptions. I want to be sure that I
get a full stack trace and I can't always rely on the caller to provide it.
> If I were you, I'd try these steps to resolve your problem:
>
> 1. Remove the 'factory' attribute from your <Resource> declaration
> 2. Remove commons-dbcp-*.jar from WEB-INF/lib
I've already done both of these.
> This has a fair chance of working. Any particular reason you want to use
> a webapp-provided connection pool factory?
I think that was a hangover from tomcat 5. I've removed that, to no obvious
effect.
Thanks for the pointers. I've still got a long way to go before I'm an expert
and all the feedback helps.
Neil Youngman
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
