Back To Basics: Connection Pooling and DBCP

While working on a very small project, it has given me an opportunity to get back to some very basic JDBC programming since using any type of framework is really just overkill.

I am trying to implement connection pooling using jakarta commons-DBCP. There aren't really any good tutorials for doing such a thing and I wanted to do something that wasn't dependent on a web container and that I could port to a desktop app if I so chose to.

Below is the code I have mustered up:

public class SimplePoolingDataSource {
    static PoolingDataSource dataSource = null;
 
    public static DataSource getDataSource() throws Exception {
        if (dataSource == null) {
            BasicDataSource ds = new BasicDataSource();
            ds.setUrl("jdbc:mysql://localhost:3306/test");
            ds.setDriverClassName("com.mysql.jdbc.Driver");
            ds.setUsername("admin");
            ds.setPassword("admin");
            ds.setMaxActive(15);
 
            ObjectPool connectionPool = new GenericObjectPool(null);
            ConnectionFactory connectionFactory = new DataSourceConnectionFactory(ds);
            PoolableObjectFactory poolableConnectionFactory = new PoolableConnectionFactory(
                    connectionFactory, connectionPool, null, null, false, true);
            dataSource = new PoolingDataSource(connectionPool);
        }
        return dataSource;
    }
 
    public static Connection getConnection() throws Exception {
        return getDataSource().getConnection();
    }
}

As you can see, I've tried to implement a singleton DataSource object so that it only gets created once. And I have a few questions:

1. In terms of simplicity is there anything I could do to make this class simpler or does it need more?
2. Is this a good way to do this?
3. Do I need to do anything special, like have my own closeConnection method, so that the connection gets returned to the pool or is issuing close() on the retrieved connection good enough?

Tips, suggestions, improvements, mockery, and/or flattery are all welcome.

Thanks.

[inserted line break in overlong line - Jim]
[ May 01, 2006: Message edited by: Jim Yingst ]

A few minor comments.

1) Why is getDataSource public?
2) Assuming the answer to 1 is well it could be private then I might put that in a static initializer block. If getDataSource fails it might as well fail on load.
3) I like loading the Strings from *something* (Properties file).
4) Closing the connection will do what is appropriate on it's own. You don't need to do anything special. So in your case it will release it back into the pool.
5) synchronization?

Originally posted by Maximilian Xavier Stocker:
A few minor comments.

1) Why is getDataSource public?
2) Assuming the answer to 1 is well it could be private then I might put that in a static initializer block. If getDataSource fails it might as well fail on load.
3) I like loading the Strings from *something* (Properties file).
4) Closing the connection will do what is appropriate on it's own. You don't need to do anything special. So in your case it will release it back into the pool.
5) synchronization?

1) Because my first draft didn't have a getConnection method. So I needed it public. So yes, it needs to be private.
2) Ok
3) I agree. But this is such a simple proof of conecpt class, I didn't bother messing with that.
4) cool, thanks.
5) care to elaborate?

5) care to elaborate?

Well if you have a need for a connection pool then that
kind of suggests you have multiple threads accessing this
class. Which would mean that getConnection should be
synchronized.

If as you said though this is a simple PoC to demonstrate
the use of pooling or whatnot and it isn't actually going
to have multiple threads then not.

[edited post to fix formatting]
[ April 30, 2006: Message edited by: Maximilian Xavier Stocker ]

Originally posted by Maximilian Xavier Stocker:

Well if you have a need for a connection pool then that kind of suggests you have multiple threads accessing this class. Which would mean that getConnection should be synchronized.

If as you said though this is a simple PoC to demonstrate the use of pooling or whatnot and it isn't actually going to have multiple threads then not.

Thanks. I'll be sure and keep that in mind when I create the next version. I have another question.

When doing the research to create this class, I saw code like:

DataSource ds = new BasicDataSource();
ds.setUrl();
ds.set...
ds.set...

Currently, the javax.sql.DataSource interface does not contain those methods. Did an earlier version of it have those methods? Is my code ok in the sense of:

BasicDataSource ds = new BasicDataSource()

or is there an interface I could use instead?
[ April 30, 2006: Message edited by: Gregg Bolinger ]

Well you see...

The intializing of the DataSource depends on the DataSource
implementation. Hopefully though as in your case this is in just the one
place in the code. So it's not a huge hassle.

In some ways you could say "Isn't DriverManager getConnection more dynamic?
Because I can use URL with it".

Well yes in theory. But of course it didn't quite work out that smoothly. I
think while the original intent was that more or less you could just slap
in the correct protocol for your driver and bob's your uncle it's not
really that simple because the rest of the URL often needs modfification
depending on your database and driver.

At the end of the day the rest of JDBC you can get away with just
interfaces[1] configuring your DataSource or connection URL is the fugly
part that is not so generic.

But by using a DataSource like you are you are hiding the fugliness from
the rest of your app so that's good.

[1] - Oracle Blobs for example being an exception