Question: I'm trying to set up a server-side cache properly and I'm looking for constructive criticism on the setup I have currently. The cache is loaded when the Servlet starts and never changed again, so in effect it's a read-only cache. It obviously needs to stay in memory for the lifetime of the Servlet. Here's how I have it set-up 

```
private static List<ProductData> _cache;
private static ProductManager productManager;

private ProductManager() {
    try {
        lookup();
    } catch (Exception ex) {
        _cache = null;
    }
}

public synchronized static ProductManager getInstance() {
    if (productManager== null) {
        productManager= new ProductManager();
    }
    return productManager;
}

```

The cache is setup by the Servlet as below:

```
private ProductManager productManager;

public void init(ServletConfig config) throws ServletException {
    productManager = ProductManager.getInstance();
}

```

And finally, this is how I access it:

```
public static ProductData lookup(long id) throws Exception {
    if (_cache != null) {
        for (int i = 0; i < _cache.size(); i++) {
            if (_cache.get(i).id == id) {
                return _cache.get(i);
            }
        }
    }

    // Look it up in the DB.
}

public static List<ProductData> lookup() throws Exception {
    if (_cache != null) {
        return _cache;
    }

    // Read it from the DB.

    _cache = list;
    return list;
}

```

Answer: 
You're doing it the hard way. A singleton-flavored pattern is completely unnecessary. Just implement a [`ServletContextListener`](http://download.oracle.com/javaee/5/api/javax/servlet/ServletContext.html) to have a hook on the webapp startup (and shutdown), so that you can just load and store the data in the **application scope** during the webapp startup.

```
public class Config implements ServletContextListener {

    private static final String ATTRIBUTE_NAME = "com.example.Config";
    private Map<Long, Product> products;

    @Override
    public void contextInitialized(ServletContextEvent event) {
        ServletContext context = event.getServletContext();
        context.setAttribute(ATTRIBUTE_NAME, this);
        String dbname = context.getInitParameter("dbname");
        products = Database.getInstance(dbname).getProductDAO().map();
    }

    @Override
    public void contextDestroyed(ServletContextEvent event) {
        // NOOP.
    }

    public static Config getInstance(ServletContext context) {
        return (Config) context.getAttribute(ATTRIBUTE_NAME);
    }

    public Map<Long, Product> getProducts() {
        return products;
    }

}

```

Which you register in `web.xml` as follows:

```
<listener>
    <listener-class>com.example.Config</listener-class>
</listener>

```

This way you can get it in any servlet as follows:

```
Config config = Config.getInstance(getServletContext());
Map<Long, Product> products = config.getProducts();
// ...

```
A few things that spring to mind:

* Store your cached `ProductData` instances in a map so that you can look up a cached instance in constant time rather than having to search through a list.
* The `lookup` methods aren't thread-safe.
* Only `lookup()` will actually load the values from the database: don't you want the other `lookup` method to also load the cache (if not already loaded) to speed up retrieve of single `ProductData` instances?