Ttykuu

Is below code a valid Singleton design pattern for Mongodb in Java?

Please suggest any improvements.

public class Mongo extends MongoClient 

 private static Mongo mongoClient = null;
 private static MongoDatabase mogoDB = null;

 private Mongo(ServerAddress server, List<MongoCredential> mongoCredentialList) 
 super(server, mongoCredentialList);
 

 public static Mongo getMongoClient() 
 return getMongoClient("userName", "password", "127.0.0.1", 27017, "SampleDB");
 

 public static Mongo getMongoClient(String userName, String pwd, String ip, int port, String dbName) 

 if (mongoClient == null) 

 logger.debug("******* Trying to connect to Mongo Server *******");

 ServerAddress server = new ServerAddress(ip, port);
 List<MongoCredential> mongoCredentialList = new ArrayList();
 char[] password = pwd.toCharArray();
 mongoCredentialList.add(MongoCredential.createCredential(userName, dbName, password));

 mongoClient = new Mongo(server, mongoCredentialList);

 logger.debug("******* Connected to the Mongo server Successfully *******");
 else 
 logger.debug("Already connected to Mongo server...");
 
 return mongoClient;
 

 public static MongoDatabase getMongoDatabase() 

 if (mogoDB == null) 
 logger.debug("******* Trying to get Mongo DB - SampleDB - connection. *******");
 mogoDB = getMongoClient().getDatabase("SampleDB");
 logger.debug("******* Successfully Connected to - SampleDB - Mongo DB *******");
 
 return mogoDB;

 

 public static MongoDatabase getMongoDatabase(String dbName) 

 if (mogoDB == null) 
 logger.debug("******* Trying to get Mongo DB - " + dbName + " - connection. *******");
 mogoDB = getMongoClient().getDatabase(dbName);
 logger.debug("******* Successfully Connected to - " + dbName + " - Mongo DB *******");
 
 return mogoDB;

 

Simple answer: No. Unless if your application only ever runs on 1 thread.

The easiest way to lazy initialise a singleton thread safe would be with a holder pattern. Example from wikipedia:

public class Something 
 private Something() 

 private static class LazyHolder 
 static final Something INSTANCE = new Something();
 

 public static Something getInstance() 
 return LazyHolder.INSTANCE;
 

This is thread safe because the java class loader ensures that the INSTANCE is initialised exactly once, when the LazyHolder class is used for the first time.

Since the database you initialise isn't known at compile time in this class you cannot use this pattern. What you can do is a pattern called double-checked locking:

public final class Singleton 
 private static volatile Singleton instance = null;

 private Singleton() 

 public static Singleton getInstance() 
 if (instance == null) 
 synchronized(Singleton.class) 
 if (instance == null) 
 instance = new Singleton();
 
 
 
 return instance;
 

Important here are the synchronized(...) and volatile keywords.
The synchronised part makes sure only 1 thread can execute the construction code. And the volatile ensures that all threads always see all changes to the variable.