I'm studying design patterns, and to demonstrate a singleton, I've implemented a primitive database connection pool.
ConnectionPool.java
package com.levent.connpool;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;
public class ConnectionPool {
private final static int MAX_CONNECTIONS = 8;
private static ConnectionPool instance = null; // lazy loading
private static Connection[] connections = new Connection[MAX_CONNECTIONS];
private static String dbUrl = "jdbc:derby://localhost:1527/memory:levoDB/singletonDemo;create=true";
private static int counter;
private ConnectionPool() { }
public static ConnectionPool getInstance() {
if(instance == null) {
synchronized(ConnectionPool.class) {
if(instance == null) {
instance = new ConnectionPool();
initializeConnections();
counter = 0;
}
}
}
return instance;
}
private static void initializeConnections() {
for(int i = 0; i < MAX_CONNECTIONS; i++) {
try {
connections[i] = DriverManager.getConnection(dbUrl);
} catch (SQLException e) {
e.printStackTrace();
}
}
}
public static Connection getConnection() {
counter++;
if(counter == Integer.MAX_VALUE)
counter = 0;
return connections[counter%MAX_CONNECTIONS];
}
}
ConnectionPoolDemo.java
package com.levent.connpool;
import java.sql.Connection;
import java.sql.SQLException;
public class ConnectionPoolDemo {
public static void main(String[] args) {
ConnectionPool pool = ConnectionPool.getInstance();
Connection[] connections = new Connection[17];
for(int i = 0; i < connections.length; i++)
connections[i] = pool.getConnection();
int preIndex = connections[0].getClass().getCanonicalName().length();
for(int i = 0; i < connections.length; i++)
System.out.printf("Connection %2d : %s\n", i+1, connections[i].toString().substring(preIndex));
for(int i = 0; i < connections.length; i++)
try {
connections[i].close();
} catch (SQLException e) {
e.printStackTrace();
}
}
}
The output is as follows:
Connection 1 : @27f8302d Connection 2 : @6438a396 Connection 3 : @e2144e4 Connection 4 : @6477463f Connection 5 : @3d71d552 Connection 6 : @1cf4f579 Connection 7 : @18769467 Connection 8 : @46ee7fe8 Connection 9 : @27f8302d Connection 10 : @6438a396 Connection 11 : @e2144e4 Connection 12 : @6477463f Connection 13 : @3d71d552 Connection 14 : @1cf4f579 Connection 15 : @18769467 Connection 16 : @46ee7fe8 Connection 17 : @27f8302d
Explanation
The aims is to have distributed connections that's why on each getConnection()
method call, on of the each connections are returned with the use of mod. The maximum connection size is determined by the MAX_CONNECTIONS
which is set to 8 in this example and all 17 connections retrieved from the singleton pool class are recurring instances of the 8 connections stored in the ConnectionPool
class.
I know that this code is not a big shot, but my aim was to demonstrate a simple singleton in a real world example. I'm waiting for your ideas and your criticism.
2 Answers 2
- Your connection pool is not threadsafe / concurrently usable, if you are sure you do not need it to be ignore my points 1. to 3.
- Your pool does not offer connection locking (mutual exclusive usage of one Connection object) which is usually implemented by making the connection inaccessible after getConnection() and making it eligible for takeout after calling its close() method. You close the raw connections manually, leaving the pool in a broken state.
This is problematic: https://stackoverflow.com/questions/9428573/ - Your counter int might return the same int for two concurrent calls at the same time. Use AtomicInteger / AtomicLong instead.
- Your pool does not implement connection testing: Your connections could expire at any time. You need to replace broken Connections with new Connections or your program might receive dead Connection objects!
- You might want to implement Closeable to close all connections at once when your application shuts down and clear the instance.
- You might want to throw if initializeConnections fails, because what are you supposed to do with a broken object?
- Hardcoding URLs that might change is not considered good practice
A more defensive implementation of creating a singleton is known as the enum
singleton pattern (I can't really find a definitive source, hence the copping-out with a Google search link). That means instead of rolling your own public class
(which really should be a public final class
so that nothing else can sub-class it), you use an enum
:
public enum ConnectionPool {
INSTANCE;
// some internal fields here
private ConnectionPool() {
// do required initialization here
}
public static ConnectionPool getInstance() {
return INSTANCE;
}
}
You may also want to look into standardizing your braces approach for all code blocks, including one-liners. This will make the scoping more apparent.
-
1\$\begingroup\$ I've just noticed this useful answer, I'll take a look up to enum singleton pattern. Thanks for your useful comment. \$\endgroup\$Levent Divilioglu– Levent Divilioglu2016年05月05日 07:31:54 +00:00Commented May 5, 2016 at 7:31
-
\$\begingroup\$ But how do you pass the connection params. for a less likely command line application that takes connection params from stdin \$\endgroup\$Somasundaram Sekar– Somasundaram Sekar2018年09月17日 12:26:19 +00:00Commented Sep 17, 2018 at 12:26
-
\$\begingroup\$ @SomasundaramSekar I'll suggest you can put that as a new question over here on Code Review. :) \$\endgroup\$h.j.k.– h.j.k.2018年09月19日 14:53:57 +00:00Commented Sep 19, 2018 at 14:53
Explore related questions
See similar questions with these tags.