1
\$\begingroup\$

A Java service that connects to MongoDB in production, but opened connection count is too much. It affects on MongoDb performance, so I have come up with a solution that provides only one MongoClient instance so which will reduce the connections.

Here is my code, is it a good way to provide a singleton object from 3rd party library?

import com.mongodb.client.MongoClient;
import com.mongodb.client.MongoClients;

public class MongoClientFactory {

    private static volatile MongoClient instance = null;

    private MongoClientFactory() {}

    public static MongoClient getInstance(String connectionString) {
        if (instance == null) {
            synchronized(MongoClientFactory.class) {
                if (instance == null) {
                    instance = MongoClients.create(connectionString);
                }
            }
        }
        return instance;
    }
}

\$\endgroup\$
3
2
\$\begingroup\$

There are already links on this page to reasons why singletons should not be used. I will not repeat those.

is it a good way to provide a singleton object from 3rd party library?

No. Your singleton has an undocumented internal state which affects the processing of the input parameters. On the first invocation, the getInstance(String connectionString) methods returns a connection to the database requested by the caller. On subsequent invocations it returns a connection to the database requested by the first caller, regardless of what connection string is provided to it. You could document it but it doesn't fix the issue of this being a very error prone solution: your method does something unexpected if something that cannot be verified during run time happened before it was called.

\$\endgroup\$
1
\$\begingroup\$

I am not expert in singleton but does the connectionString really change over the life of the application? Usually (I might be wrong) this string is declared with the final keyword.

\$\endgroup\$
3
  • \$\begingroup\$ because of different environment like test, prod and local, for different connection string \$\endgroup\$
    – fuat
    Mar 16 '21 at 14:46
  • \$\begingroup\$ Yes but once the application is launch you are probably not going to change the URL ? For me the url should be inside a porperties file, which is different for every env and so you can change your singleton to be build inside static block. \$\endgroup\$
    – Quentin M
    Mar 16 '21 at 14:55
  • \$\begingroup\$ yes, just change the function signature \$\endgroup\$
    – fuat
    Mar 16 '21 at 15:46
0
\$\begingroup\$

With your code you'll get getInstance(database1) == getInstance(database2) which is obviously wrong, this points to a problem with your API. Using a singleton (poorly) is the likely cause here. There's good reason why singletons are discouraged.

For your problem you really should have one instance created in your main that's then passed through to all objects that need it and the instance is then kept alive. In general what you're looking for is called Dependency Injection and there are several frameworks to make it easier, for example dagger and guice.

\$\endgroup\$

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.