3
\$\begingroup\$

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.

\$\endgroup\$
3
\$\begingroup\$
  1. 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.
  2. 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/
  3. Your counter int might return the same int for two concurrent calls at the same time. Use AtomicInteger / AtomicLong instead.
  4. 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!
  5. You might want to implement Closeable to close all connections at once when your application shuts down and clear the instance.
  6. You might want to throw if initializeConnections fails, because what are you supposed to do with a broken object?
  7. Hardcoding URLs that might change is not considered good practice
\$\endgroup\$
1
\$\begingroup\$

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.

\$\endgroup\$
  • 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 May 5 '16 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 Sep 17 '18 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. Sep 19 '18 at 14:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.